3

I have the following code for population a ListView from a background thread (DoWork calls the PopulateThread method):

delegate void PopulateThreadCallBack(DoWorkEventArgs e);
private void PopulateThread(DoWorkEventArgs e)
{

    if (this.InvokeRequired)
    {
        PopulateThreadCallBack d = new PopulateThreadCallBack(this.PopulateThread);
        this.Invoke(d, new object[] { e });
    }
    else
    {

        // Ensure there is some data
        if (this.DataCollection == null)
        {
            return;
        }

        this.Hide();

        // Filter the collection based on the filters
        List<ServiceCallEntity> resultCollection = this.ApplyFilter();

        // Get the current Ids
        List<Guid> previousIdList = this.GetUniqueIdList(listView);
        List<Guid> usedIdList = new List<Guid>();

        foreach (ServiceCallEntity record in resultCollection)
        {

            if (e.Cancel)
            {
                this.Show();
                return;
            }
            else
            {

                // Get the top level entities
                UserEntity userEntity = IvdSession.Instance.Collection.GetEngineerEntity(record.UserId);
                AssetEntity assetEntity = IvdSession.Instance.Collection.GetAssetEntity(record.AssetId);
                SiteEntity siteEntity = IvdSession.Instance.Collection.GetSiteEntity(record.SiteId);
                FaultEntity faultEntity = IvdSession.Instance.Collection.GetFaultEntity(record.FaultId);

                if (siteEntity == null || userEntity == null || faultEntity == null)
                {
                    continue;
                }
                else
                {

                    // Get the linked entities
                    RegionEntity regionEntity = IvdSession.Instance.Collection.GetRegionEntity(siteEntity.RegionId);
                    StatusEntity statusEntity = IvdSession.Instance.Collection.GetStatusEntity(record.ServiceCallStatus.StatusId);

                    ListViewItem item = new ListViewItem(siteEntity.SiteName);
                    item.SubItems.Add(siteEntity.Address);
                    item.Tag = record;

                    item.SubItems.Add(regionEntity.Description);

                    // Handle if an Asset is involved
                    if (record.AssetId > 0)
                        item.SubItems.Add(assetEntity.AssetDisplay);
                    else
                        item.SubItems.Add("N/A");

                    item.SubItems.Add(faultEntity.Description);
                    item.SubItems.Add(userEntity.UserDisplay);

                    item.SubItems.Add("TODO: Claimed By");
                    item.SubItems.Add(record.DateTimeStamp.ToString());

                    IvdColourHelper.SetListViewItemColour(item, false);
                    this.PopulateItem(item, ref usedIdList);

                }

            }

        }

        // Clean up the grid
        this.CleanListView(previousIdList, usedIdList);

        // Only autosize when allowed and when there are some items in the ListView
        if (this.AllowAutoSize && listView.Items.Count > 0)
        {
            rsListView.AutoSizeColumns(listView);
            this.AllowAutoSize = false;
        }

        this.Show();

    }

}

Unfortunately, this causes the UI to freeze whilst in the foreach... is there any way to update/populate the ListView without it freezing the main UI?

djdd87
  • 67,346
  • 27
  • 156
  • 195
  • Just to add - this is the completely wrong way around doing what I was trying to achieve. THis code has since been nuked and fixed. The accepted answer is a bodge. – djdd87 Jan 19 '10 at 13:02

3 Answers3

11

A) You probably don't need to use this.Invoke and instead use this.BeginInvoke. Invoke blocks the current thread.

B) You don't need to define your own delegates you can use MethodInvoker

if(this.InvokeRequired) {
  this.BeginInvoke(new MethodInvoker(() => PopulateThread(e)));
  return;
}

It's much cleaner :)

Rich Schuler
  • 41,814
  • 6
  • 72
  • 59
5

You are using Control.Invoke to execute just about everything, meaning this code isn't multithreaded at all.

The proper way (involving a Backgroundworker) would be to use the UpdateProgress event to add elements. It is already synchronized.

But since you're hiding the control (or is it the Form ?) during this process you might as well build a List and on completion add it to the Listview. That piece of code shouldn't take long.

Or some sort of combination, adding small lists in an update event. And I wonder about the wisdom of Hide/Show, I expect this to just make the UI flicker. Leave them out or replace with SuspendLayout/Resumelayout.

H H
  • 263,252
  • 30
  • 330
  • 514
  • 1
    Should also add that if you have a SortComparer assigned to the list, add/remove is MUCH slower. When making modifications it's a good idea to temporarily disable sorting and then re-enable it after all adds and removes are complete. – Jeff Yates Jun 12 '09 at 18:58
0

Pump the events manually with

Application.DoEvents();
Jared Updike
  • 7,165
  • 8
  • 46
  • 72
  • 5
    -1, Sorry, but BgWorker->Invoke->DoEvents? Looks like patching one hole with another. – H H Jun 12 '09 at 18:45
  • 1
    Good point, though I will say: you can get pretty far with Application.DoEvents instead of multiple threads if all you want is a moderately responsive GUI during computationally intensive tasks, just pump the message queue yourself. A lot of this machinery is just extra complexity with little gain (unless of course you really need the threads). For example, with multiple threads, debugging + exceptions = FAIL. But as you said in your answer this isn't multithreaded code at all. Concurrency in imperative languages is a PITA. Stay away unless you really need it. – Jared Updike Jun 12 '09 at 21:38
  • Jared, if you meant DoEvents instead of a BgWorker then I missed that. DoEvents is tricky though. – H H Jun 13 '09 at 11:58