A multithreading "best practice" question!
-
Hello all. I've got a common multithreading predicament that might look easy to solve, but I'm looking for best practice. I got some sloppy ideas myself, but I'd like to hear what should be best used in this situation. 1- I'm fetching data from the network server -No problem. Takes about 3-7 secs-. 2- Then I'm populating/refreshing a TreeView from that DataTable which could contain up to 30,000+ records. This is the problem as this takes about 2-3 mins. I can't allow step two to hold off the user for 2-3 mins until the list is filled. I decided to make another thread to fill up the TreeView while the user still can interact with the rest of the UI. Two questions: 1- Is it normal that a TreeView takes 2-3 mins on a modern Centrino 1.83 laptop with 1.5 GB RAM to add 30,00 nodes? How to speed it up? 2- I got a refresh button that should cancel that active thread, clear the TreeView, then refresh it with node. If I press it twice quickly with my current code, the UI freezes. Any ideas? If you need to take a look at my sloppy code -Here! Have a nice laugh at it!-, here it is:
private bool StopPopulating;
private delegate void ClearDel();
private delegate TreeNode AddDel(string text);internal void PopulateList() { if (!IsPopulating && StopPopulating) return; IsPopulating = false; StopPopulating = true; while (Populator != null && Populator.IsAlive) Thread.Sleep(1); Populator = new Thread(new ThreadStart(PopulateMethod)); Populator.IsBackground = true; Populator.Priority = ThreadPriority.Lowest; IsPopulating = true; StopPopulating = false; Populator.Start(); } private void PopulateMethod() { lock ("Populating") { try { ClearDel clearing = new ClearDel(Patients.Nodes.Clear); AddDel adding = new AddDel(Patients.Nodes.Add); Patients.BeginInvoke(clearing); CathProxy PatientsProxy = (CathProxy)Activator.GetObject(typeof(CathProxy), CathLab.Url); DataTable mvps = new DataTable(); DataTable Pts = PatientsProxy.GetList(out mvps); TreeNode CurrentNode; foreach (DataRow r in Pts.Rows) { if (StopPopulating)
-
Hello all. I've got a common multithreading predicament that might look easy to solve, but I'm looking for best practice. I got some sloppy ideas myself, but I'd like to hear what should be best used in this situation. 1- I'm fetching data from the network server -No problem. Takes about 3-7 secs-. 2- Then I'm populating/refreshing a TreeView from that DataTable which could contain up to 30,000+ records. This is the problem as this takes about 2-3 mins. I can't allow step two to hold off the user for 2-3 mins until the list is filled. I decided to make another thread to fill up the TreeView while the user still can interact with the rest of the UI. Two questions: 1- Is it normal that a TreeView takes 2-3 mins on a modern Centrino 1.83 laptop with 1.5 GB RAM to add 30,00 nodes? How to speed it up? 2- I got a refresh button that should cancel that active thread, clear the TreeView, then refresh it with node. If I press it twice quickly with my current code, the UI freezes. Any ideas? If you need to take a look at my sloppy code -Here! Have a nice laugh at it!-, here it is:
private bool StopPopulating;
private delegate void ClearDel();
private delegate TreeNode AddDel(string text);internal void PopulateList() { if (!IsPopulating && StopPopulating) return; IsPopulating = false; StopPopulating = true; while (Populator != null && Populator.IsAlive) Thread.Sleep(1); Populator = new Thread(new ThreadStart(PopulateMethod)); Populator.IsBackground = true; Populator.Priority = ThreadPriority.Lowest; IsPopulating = true; StopPopulating = false; Populator.Start(); } private void PopulateMethod() { lock ("Populating") { try { ClearDel clearing = new ClearDel(Patients.Nodes.Clear); AddDel adding = new AddDel(Patients.Nodes.Add); Patients.BeginInvoke(clearing); CathProxy PatientsProxy = (CathProxy)Activator.GetObject(typeof(CathProxy), CathLab.Url); DataTable mvps = new DataTable(); DataTable Pts = PatientsProxy.GetList(out mvps); TreeNode CurrentNode; foreach (DataRow r in Pts.Rows) { if (StopPopulating)
1. I don't have a direct comparison, but 2-3 minutes to create a 30,000 node TreeView seems slow. I think your performance problem is due to two things: making individual TreeView updates and making those updates via individual BeginInvoke() calls. On my 1.63GHz Core2 Duo, it takes about 45 seconds when updates are performed individually from within a loop on the UI thread. When batching the updates, that time was cut down to 20 seconds. Using Control.Invoke() or Control.BeginInvoke() is a relatively expensive operation due to all of the coordination necessary to shift to and execute the delegate on the UI thread. While it's necessary in order to update the UI from a worker thread, it should not be used in "tight loop" situations. Instead, one should make a single Invoke()/BeginInvoke() call and make *all* the updates from within that single delegate. This model also allows one to make use of the Control.BeginUpdate() and Control.EndUpdate() methods. These disable and reenable drawing of the control. By adding your new nodes in between those method calls, the system will add them without repeatedly refreshing the control display, and therefore, will add them more quickly. 2. You're likely seeing the UI freeze due to your Thread.Sleep(1) in your PopulateList() method. If this is being called on the UI thread, it effectivly stops the Windows message pump until the thread exits. Blocking on the UI thread is strongly discouraged. If your thread is also interacting with the message pump (e.g. by making Control.Invoke() calls), you could also be setting up a deadlock situation. One trick to prevent the "impatient user multiple-click" is to disable the button on the first click and only reenable it once the resulting action is complete. -Phil
-
1. I don't have a direct comparison, but 2-3 minutes to create a 30,000 node TreeView seems slow. I think your performance problem is due to two things: making individual TreeView updates and making those updates via individual BeginInvoke() calls. On my 1.63GHz Core2 Duo, it takes about 45 seconds when updates are performed individually from within a loop on the UI thread. When batching the updates, that time was cut down to 20 seconds. Using Control.Invoke() or Control.BeginInvoke() is a relatively expensive operation due to all of the coordination necessary to shift to and execute the delegate on the UI thread. While it's necessary in order to update the UI from a worker thread, it should not be used in "tight loop" situations. Instead, one should make a single Invoke()/BeginInvoke() call and make *all* the updates from within that single delegate. This model also allows one to make use of the Control.BeginUpdate() and Control.EndUpdate() methods. These disable and reenable drawing of the control. By adding your new nodes in between those method calls, the system will add them without repeatedly refreshing the control display, and therefore, will add them more quickly. 2. You're likely seeing the UI freeze due to your Thread.Sleep(1) in your PopulateList() method. If this is being called on the UI thread, it effectivly stops the Windows message pump until the thread exits. Blocking on the UI thread is strongly discouraged. If your thread is also interacting with the message pump (e.g. by making Control.Invoke() calls), you could also be setting up a deadlock situation. One trick to prevent the "impatient user multiple-click" is to disable the button on the first click and only reenable it once the resulting action is complete. -Phil
Thanks for your very valuable help. I really appreciate it. Actually about the "impatient user multiple-click" issue. The list is updated by several events, so I can't really have control on the refreshing. I need that when the user triggers an event that requires refreshment that the current thread is gracefully terminated, and a new thread started again. I've made a few adjustment to the code. Now My main thread freezes even though I use BeginInvoke(). Any idea way.
internal void PopulateList()
{
/*if (!IsPopulating && StopPopulating)
return;
IsPopulating = false;
StopPopulating = true;
while (Populator != null && Populator.IsAlive)
Application.DoEvents();*/
Populator = new Thread(new ThreadStart(PopulateMethod));
Populator.IsBackground = true;
Populator.Priority = ThreadPriority.BelowNormal;
IsPopulating = true;
StopPopulating = false;
Populator.Start();
}private void PopulateMethod() { lock ("Populating") { try { CathProxy PatientsProxy = (CathProxy)Activator.GetObject(typeof(CathProxy), CathLab.Url); DataTable mvps = new DataTable(); DataTable Pts = PatientsProxy.GetList(out mvps); updateDel update = new updateDel(UpdateList); this.BeginInvoke(update, new object\[\] { Pts, mvps }); } catch (SystemException ex) { MessageBox.Show(ex.Message, "Error populating patients list"); } } } private void UpdateList(DataTable Pts, DataTable mvps) { Patients.Nodes.Clear(); TreeNode PleaseWait = Patients.Nodes.Add("Refreshing list. Please Wait"); Patients.BeginUpdate(); foreach (DataRow r in Pts.Rows) { if (StopPopulating) return; TreeNode CurrentNode = Patients.Nodes.Add('(' + r\["Hospital\_Number"\].ToString() + ") " + r\["Patient\_Name"\].ToString()); foreach (DataRow ro in mvps.Rows) { if (r\["Hospital\_Number"\].ToString() == ro\["Hospital\_Number"\].ToString()) CurrentNode.Nodes.Add("MVP at " + DateTime.Parse(ro\["Procedure\_Date"\].ToStrin
-
Thanks for your very valuable help. I really appreciate it. Actually about the "impatient user multiple-click" issue. The list is updated by several events, so I can't really have control on the refreshing. I need that when the user triggers an event that requires refreshment that the current thread is gracefully terminated, and a new thread started again. I've made a few adjustment to the code. Now My main thread freezes even though I use BeginInvoke(). Any idea way.
internal void PopulateList()
{
/*if (!IsPopulating && StopPopulating)
return;
IsPopulating = false;
StopPopulating = true;
while (Populator != null && Populator.IsAlive)
Application.DoEvents();*/
Populator = new Thread(new ThreadStart(PopulateMethod));
Populator.IsBackground = true;
Populator.Priority = ThreadPriority.BelowNormal;
IsPopulating = true;
StopPopulating = false;
Populator.Start();
}private void PopulateMethod() { lock ("Populating") { try { CathProxy PatientsProxy = (CathProxy)Activator.GetObject(typeof(CathProxy), CathLab.Url); DataTable mvps = new DataTable(); DataTable Pts = PatientsProxy.GetList(out mvps); updateDel update = new updateDel(UpdateList); this.BeginInvoke(update, new object\[\] { Pts, mvps }); } catch (SystemException ex) { MessageBox.Show(ex.Message, "Error populating patients list"); } } } private void UpdateList(DataTable Pts, DataTable mvps) { Patients.Nodes.Clear(); TreeNode PleaseWait = Patients.Nodes.Add("Refreshing list. Please Wait"); Patients.BeginUpdate(); foreach (DataRow r in Pts.Rows) { if (StopPopulating) return; TreeNode CurrentNode = Patients.Nodes.Add('(' + r\["Hospital\_Number"\].ToString() + ") " + r\["Patient\_Name"\].ToString()); foreach (DataRow ro in mvps.Rows) { if (r\["Hospital\_Number"\].ToString() == ro\["Hospital\_Number"\].ToString()) CurrentNode.Nodes.Add("MVP at " + DateTime.Parse(ro\["Procedure\_Date"\].ToStrin
Under what conditions does the UI freeze? Is it a complete freeze, or does the UI respond after some period of time? I noticed in your UpdateList() method that you do not call Patients.EndUpdate() if your update is interrupted; you should probably put that in a finally statement so that it is always called. -Phil
-
Under what conditions does the UI freeze? Is it a complete freeze, or does the UI respond after some period of time? I noticed in your UpdateList() method that you do not call Patients.EndUpdate() if your update is interrupted; you should probably put that in a finally statement so that it is always called. -Phil
Thanks to your help I managed to reduce the time to 17 seconds. My UI freezes these 17 seconds when it adds the nodes between the BeginUpdate() and EndUpdate() calls. I discovered that adding the nodes to the TreeView can not be done except on the main thread, so I called a DoEvents() in the loop and the freeze is gone. I can't have gone this far without your help. I really appreciate it. Thanks again.
Regards:rose: