Why my program lags upon selecting strip

Question

I have this function that calls the function:

private void liguaneaRxToolStripMenuItem_Click(object sender, EventArgs e)
{
    FillLiguanea();
}

This is the function it invokes:

private void FillLiguanea()
{
    this.liguanea_LaneTableAdapter1.Fill(this.pharmaciesDataSet1.Liguanea_Lane);
    try
    {
        string connectionString = "Data Source=LPMSW09000012JD\\SQLEXPRESS;Initial Catalog=Pharmacies;Integrated Security=True";
        SqlConnection con = new SqlConnection(connectionString);
        con.Open();
        string query = "SELECT * FROM dbo.Liguanea_Lane2";
        SqlCommand cmd = new SqlCommand(query, con);

        SqlDataReader dr = cmd.ExecuteReader();
        while (dr.Read())
        {
            string scode = dr.GetString(dr.GetOrdinal("code"));
            comboBox2.Items.Add(scode);
        }
    }
    catch (Exception ex)
    {

        MessageBox.Show(ex.ToString());
    }
}

It's over 5000+ data reading in from the SQL database, but a 20 second lag each time before it reads in will not be satisfying to the end user.

My question is why does this happen and is there a way to speed it up?

It would appear that it happens because that's how long it takes to execute the query and process the results. Unless there's some easy optimization I'm missing, you may have to do some threading/async coding to avoid tying up the UI thread. — adv12, Nov 04 '16 at 14:22
[Load data asynchronously into my DataTable in Windows Forms](http://stackoverflow.com/a/38427392/3110834) — Reza Aghaei, Nov 04 '16 at 14:29
Have you run that query in sql server managment studio ? how does it performs there ? If it also takes 20 seconds there than you have your answer. — GuidoG, Nov 04 '16 at 14:32
It takes less than a second to run in MSSQL so I don't believe that is the issue — Javy26, Nov 04 '16 at 14:33
Small thing: You are not closing your connection after it's done. You might want to do that to avoid memory leaks! I do not think it's the source of your problems. You might want to wrap your sql connection in a using statement so you don't have to write that logic — Dudemanword, Nov 04 '16 at 14:36
Also as another tuning, you should call `comboBox2.BeginUpdate()` before while and `comboBox2.EndUpdate()` after loop or use `Items.AddRange` instead [like this](http://stackoverflow.com/a/39338415/3110834). — Reza Aghaei, Nov 04 '16 at 14:36
ok so you are running this `string query = "SELECT * FROM dbo.Liguanea_Lane2";` but you are only loading the comboBox2 with `code` then either change your query to `select on Code from Liguanea` or bind the code to a `List` — MethodMan, Nov 04 '16 at 14:36
@MethodMan did that before and didn't improve performance. Binding it into a List that is — Javy26, Nov 04 '16 at 14:41
Correcting the way of adding items to combo box will have an impressive impact on performance. See the [linked post](https://stackoverflow.com/questions/39338237/populate-listbox-with-a-ienumrable-on-another-thread-winforms) — Reza Aghaei, Nov 04 '16 at 14:44
what about selecting only the Code to speed up the performance..? have you tied that as well @Jevon ? — MethodMan, Nov 04 '16 at 14:45
It might also be because the ArithAbort property on your database is off. See also http://www.sommarskog.se/query-plan-mysteries.html — GuidoG, Nov 04 '16 at 15:12
@GuidoG thanks for the suggestion but unfortunately it doesn't work — Javy26, Nov 04 '16 at 15:15
@Jevon So you are saying that executing the query in your code also takes less than a second ? — GuidoG, Nov 04 '16 at 15:17
@GuidoG in my code it takes 20 seconds. In MSSQL it takes less than a second. Am guessing it has something to do with reading so much data from the MSSQL table am dealing with. Just need to speed that up. Wondering if adding a "loading,please wait" could be a work around — Javy26, Nov 04 '16 at 15:20
@Jevon Wat exactly is taking 20 seconds ? You should check if the code cmd.ExecuteReader(); takes 20 seconds or the loop after it. Put a breakpoint on SqlDataReader dr = cmd.ExecuteReader(); and than measure how long it takes when you press F10. If this takes 20 seconds than the link in my prior comment is your solution, if not than there is something in your c# code not correct. Also check with a profiler what command is send to the database. — GuidoG, Nov 04 '16 at 15:28
No, the lag is expected because it is reading in 10k + records, so am trying to implement a background worker. I just posted my attempt. your input on it would be good — Javy26, Nov 07 '16 at 19:35
I really will not use a background worker if I write code in .NET 4.5. Surely using async/await pattern is preferred. — Reza Aghaei, Nov 07 '16 at 19:36
oh am not familiar with it. Your input would be great though on my new question with the async/await. I don't really fancy background worker — Javy26, Nov 07 '16 at 19:37
@RezaAghaei could you help me out please? no one seems to have any solution — Javy26, Nov 07 '16 at 20:05
I honestly didn't see that you asked that. My apologies. Am looking at your current response now — Javy26, Nov 07 '16 at 20:16
@AlexanderPetrov it has to because based on the selection from that it populates the textBox that follows with values from the SQL Database. It's basically an inventory program — Javy26, Nov 07 '16 at 20:24

score 2 · Accepted Answer · edited May 23 '17 at 11:53

2

You should consider some notes to make the form run without being frozen.

You should load data in another thread. You can create a Thread, or use BackgroundWorker or using async/await pattern like this post.
You are adding items to ListBox one by one using listBox.Items.Add(). If you want to do so, at least you should call listBox.BeginUpdate() before adding the loop and call listBox.EndUpdate() after the loop. It makes an impressive impact on your UI. See this post for more information. You can call AddRange or set the DataSource. Setting data source will work great.

Example

public async Task<DataTable> GetDataAsync()
{
    var dt = new DataTable();
    var cn = @"Your Connection String";
    var cmd = @"SELECT Field1 FROM Table1";
    var da = new SqlDataAdapter(cmd, cn);
    await Task.Run(() => { da.Fill(dt); });
    return dt;
}   
private async void Form1_Load(object sender, EventArgs e)
{
    var data = await GetDataAsync();
    this.ListBox1.DataSource = data;
    this.ListBox1.DisplayMember= "Field1";
}

edited May 23 '17 at 11:53

Community

1
1

answered Nov 07 '16 at 20:11

Reza Aghaei

120,393
18
203
398

I saw it. The answer which I posted is not based on backgroundworker and it's totally different. Try it and let us the result. – Reza Aghaei Nov 07 '16 at 20:35
Am not quite understanding the part. Those arguments inside should be replaced by my "fillLiguanea()" function parameters? – Javy26 Nov 07 '16 at 20:51
No it shouldn't be replaced. Just replace your query and your connection string. Try to learn something new. It's really better to load data this way. `var cn = @"Data Source=LPMSW09000012JD\\SQLEXPRESS;Initial Catalog=Pharmacies;Integrated Security=True""` – Reza Aghaei Nov 07 '16 at 20:52
`var cmd = @"SELECT Code FROM dbo.Liguanea_Lane2"` and `this.ListBox1.DisplayMember= "Code";` – Reza Aghaei Nov 07 '16 at 20:54
These are all changes you need to make the code working for yourself. – Reza Aghaei Nov 07 '16 at 20:54
Use only one \ since we used `@` behind the string. Also you don't need to select `*`, code would be enough. – Reza Aghaei Nov 07 '16 at 21:08
It's in general OK. By the way, don't edit the question with updated code, it makes the answers nonsense :) – Reza Aghaei Nov 07 '16 at 21:09
Ok I will remember tha in the future. An exception is being thrown by "da.Fill(dt))" – Javy26 Nov 07 '16 at 21:12
What's the exception? did you corrected the connection string as I said use one \? – Reza Aghaei Nov 07 '16 at 21:14
Yes I did correct it. It works now but this isn't what am looking for. It is doing what it has been doing from the beginning. What I want is for it to load the data based on the dropdown selected in comboBox4. – Javy26 Nov 07 '16 at 21:17
eg: if(comboBox4.selectedIndex==0{ //do stuff } – Javy26 Nov 07 '16 at 21:17
Surely it's better than what you had at first. Now it doesn't freeze. – Reza Aghaei Nov 07 '16 at 21:19
But it wasn't freezing before. it only froze when conditional operators were introduced. Like the strip menu or a comboBox. It always loaded fast. it just froze and move slow cause conditions were introduced. – Javy26 Nov 07 '16 at 21:21
No problem. It seems you need to read more about How to ask. We can't predict and read what is in your mind. Currently the question talks about a freezing issue which can be solved by the answer – Reza Aghaei Nov 07 '16 at 21:23
I'm interested to help. I think it's better to ask a new question based on the current code and clearly say what is the problem now (which is not freezing at all). I think you need to fill the data table using parameters, for example *SELECT * FROM Table1 WHERE Id = 1* and 1 would be a value fromm the combo box. Any way, it's just a guess. Your description about the problem and the codes will be required :) – Reza Aghaei Nov 07 '16 at 21:33
Also if you want to fill a DataTable using a TableAdapter asynchronously use `var ds = new DataSet1(); await Task.Run(() =>{ tableAdapter1.Fill(ds.Table1);});` The same way which is used in `GetDataAsync` for a data adapter. – Reza Aghaei Nov 07 '16 at 21:37
If you learn more about `async/await` life would be easier. As you see also others guide you to use `async/await` instead of `backgroundworker` :) – Reza Aghaei Nov 08 '16 at 16:18
I understand. Is there a way to have more than one sql table populate in data grid based on an if condition? – Javy26 Nov 08 '16 at 17:09
You can load any count of DataTable of a DataSet and return the DataSet. Also you can Load data tables one by one asynchronously. The core line of code is here `await Task.Run(() => { da.Fill(dt); });`. – Reza Aghaei Nov 08 '16 at 17:12
1

Then if you specifically asking about `DataGridView` it's enough to set its `DataSource` to a `DataTable` based on a criteria. – Reza Aghaei Nov 08 '16 at 17:13
OHHH brilliant. Thanks. You are an awesome tutor – Javy26 Nov 08 '16 at 17:18
@Jevon By the way, it seems the post answers your question and it would be great if you accept/upvote the post :) It's not compulsory at all. – Reza Aghaei Nov 17 '16 at 22:08

Why my program lags upon selecting strip

1 Answers1

Linked