dancolewye
dancolewye

Reputation: 11

WPF Application UI freezing despite collection view source being updated by a background worker

I have an app that retrieves data from a database and displays it in data grid on the main window. The maximum number of items being displayed is ~5000.

I don't mind a time delay in display the results, but i'd like to display a loading animation whilst this is happening. However, even when using a background worker to update the collection view source the UI freezes before displaying the rows.

Is it possible to add all these rows without freezing the UI? Apply filters to the collection view source also seems to freeze the UI which i'd like to avoid also if possible.

Thanks in advance!

UPDATE 06.01.2023

Updated as per the suggestions from BionicCode and Andy and now everything is running very smoothly - thank you for the help!

XAML for the data grid:

<DataGrid Grid.Column="1" Name="documentDisplay" ItemsSource="{Binding Source={StaticResource cvsDocuments}, UpdateSourceTrigger=PropertyChanged, IsAsync=True}" AutoGenerateColumns="False" 
                      Style="{StaticResource DataGridDefault}" ScrollViewer.CanContentScroll="True"
                      HorizontalAlignment="Stretch" HorizontalContentAlignment="Stretch" ColumnWidth="*">

XAML for collection view source:

<Window.Resources>
        <local:Documents x:Key="documents" />
        <CollectionViewSource x:Key="cvsDocuments" Source="{StaticResource documents}"
                                Filter="DocumentFilter">

Code within function being called after retrieving data from database:

Documents _documents = (Documents)this.Resources["documents"];
            
            BindingOperations.EnableCollectionSynchronization(_documents, _itemsLock);


            if (!populateDocumentWorker.IsBusy)
            {
                progressBar.Visibility = Visibility.Visible;
                populateDocumentWorker.RunWorkerAsync(jobId);
            }

Code within worker:

Documents _documents = (Documents)this.Resources["documents"];

                lock (_itemsLock)
                {
                    _documents.Clear();
                    _documents.AddRange(documentResult.documents);
                }

Observable collection class:

public class Documents : ObservableCollection<Document>, INotifyPropertyChanged
    {
        private bool _surpressNotification = false;

        protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
        {
            if (!_surpressNotification)
            {
                base.OnCollectionChanged(e);
            }
        }

        public void AddRange(IEnumerable<Document> list)
        {
            if(list == null)
            {
                throw new ArgumentNullException("list");

                _surpressNotification = true;
            }
            
            foreach(Document[] batch in list.Chunk(25))
            {
                foreach (Document item in batch)
                {
                    Add(item);
                }
                _surpressNotification = false;
            }
 
            OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
        }
    }

Base class for observable collection:

public class Document : INotifyPropertyChanged, IEditableObject
    {
        public int Id { get; set; }
        public string Number { get; set; }
        public string Title { get; set; }
        public string Revision { get; set; }
        public string Discipline { get; set; }
        public string Type { get; set;  }
        public string Status { get; set; }
        public DateTime Date { get; set; }
        public string IssueDescription { get; set; }
        public string Path { get; set; }
        public string Extension { get; set; }

        // Implement INotifyPropertyChanged interface.
        public event PropertyChangedEventHandler PropertyChanged;

        public void OnPropertyChanged(String info)
        {
            if(PropertyChanged != null)
            {
                PropertyChanged(this, new PropertyChangedEventArgs(info));
            }
        }

        private void NotifyPropertyChanged(string propertyName)
        {

        }

        // Implement IEditableObject interface.
        public void BeginEdit()
        {

        }

        public void CancelEdit()
        {

        }

        public void EndEdit()
        {

        }
    }

Filter Function:

private void DocumentFilter(object sender, FilterEventArgs e)
        {
            //Create list of all selected disciplines
            List<string> selectedDisciplines = new List<string>();
            
            foreach(var item in disciplineFilters.SelectedItems)
            {
                selectedDisciplines.Add(item.ToString());
            }

            //Create list of all select document types
            List<string> selectedDocumentTypes = new List<string>();

            foreach(var item in docTypeFilters.SelectedItems)
            {
                selectedDocumentTypes.Add(item.ToString());
            }

            // Create list of all selected file tpyes
            List<string> selectedFileTypes = new List<string>();

            foreach(var item in fileTypeFilters.SelectedItems)
            {
                selectedFileTypes.Add(item.ToString());
            }

            //Cast event item as document object
            Document doc = e.Item as Document;

            //Apply filter to select discplines and document types
            if( doc != null)
            {
                if (selectedDisciplines.Contains(doc.Discipline) && selectedDocumentTypes.Contains(doc.Type) && selectedFileTypes.Contains(doc.Extension))
                {
                    e.Accepted = true;
                } else
                {
                    e.Accepted = false;
                }
            }
        }

Upvotes: 1

Views: 826

Answers (2)

BionicCode
BionicCode

Reputation: 29018

The problem is your Filter callback. Currently you iterate over three lists inside the event handler (in order to create the filter predicate collections for lookup).
Since the event handler is invoked per item in the filtered collection, this introduces excessive work load for each filtered item.

For example, if each of the three iterations involves 50 items and the filtered collection contains 5,000 items, you execute a total of 5035000 = 750,000 iterations (150 for each event handler invocation).

I recommend to maintain the collections of selected items outside the Filter event handler, so that it doesn't have to be created for each individual item (event handler invocation). The three collections are only updated when a related SelectedItems property has changed.

To further speed up the lookup in the Filter event handler I also recommend to replace the List<T> with a HashSet<T>.
While List.Contains is an O(n) operation, HashSet.Containsis O(1), which can make a huge difference.

You need to track the SelectedItems that are the source for those collections separately to update them.

The following example should speed up your filtering significantly.

/* Define fast O(1) lookup collections */
private HashSet<string> SelectedDisciplines { get; set; }
private HashSet<string> SelectedDocumentTypes { get; set; }
private HashSet<string> SelectedFileTypes { get; set; }

// Could be invoked from a SelectionChanged event handler
// (what ever the source 'disciplineFilters.SelectedItems' is)
private void OnDisciplineSelectedItemsChanged()
  => this.SelectedDisciplines = new HashSet<string>(this.disciplineFilters.SelectedItems.Select(item => item.ToString()));

// Could be invoked from a SelectionChanged event handler
// (what ever the source 'docTypeFilters.SelectedItems' is)
private void OnDocTypeSelectedItemsChanged()
  => this.SelectedDocumentTypes = new HashSet<string>(this.docTypeFilters.SelectedItems.Select(item => item.ToString()));

// Could be invoked from a SelectionChanged event handler
// (what ever the source 'fileTypeFilters.SelectedItems' is)
private void OnFileTypeSelectedItemsChanged()
  => this.SelectedFileTypes = new HashSet<string>(this.fileTypeFilters.SelectedItems.Select(item => item.ToString()));
  
private void FilterDocuments(object sender, FilterEventArgs e)
{
  // Cast event item as document object
  if (e.Item is not Document doc) //if (!(e.Item is Document doc))
  {
    return;
  }

  // Apply filter to select discplines and document types
  e.Accepted = this.SelectedDisciplines.Contains(doc.Discipline) 
    && this.SelectedDocumentTypes.Contains(doc.Type) 
    && this.SelectedFileTypes.Contains(doc.Extension);
}

Remarks

You should fix your Documents.AddRange method.
It should use the NotifyCollectionChangedAction.Add. NotifyCollectionChangedAction.Replace will trigger the binding target to completely update itself, which is what you want to avoid.

Use the appropriate NotifyCollectionChangedEventArgs constructor overload to send the complete range of added items with the event:

OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, list as IList ?? list.ToList()));

Further considerations

  • Since you said you read the data from a database, you should consider to let the database filter the data for you. Given the correct query, the database will provide far better performance as it is highly optimized for filtering (search queries).
  • Using the filter feature of the ICollectionView will always block the UI until the collection is filtered. This is because the procedure is not asynchronous. This means you can't display a progress bar as it won't update in real time. Consider to prefilter the items when fetching them from the database. It doesn't make sense to load 5k items when the user can only view 10-50 of them.
  • If you want to display a progress bar, you better filter the collection directly. This requires a dedicated binding source collection. Since you have already implemented a custom ObservableCollection that exposes a AddRange method, you are good to go (don't forget to fix the CollectionChanged event data).

To add grouping you have to take into consideration that a) grouping disables row virtualization b) grouping actually takes place in the UI. The control creates a GroupItem for each group.

To fix a) you need to explicitly enable virtualization while grouping by setting the attached VirtualizingPanel.IsVirtualizingWhenGrouping to true:

<DataGrid VirtualizingPanel.IsVirtualizingWhenGrouping="True" />

To fix b) you could use LINQ grouping, which you could execute on a background thread if necessary:

IEnumerable<IGrouping<string, Document>> groupedDocuments = FilteredItemsSource.DataGridItems.GroupBy(document => document.Author);
dataGrid.ItemsSource = groupedDocuments;

The problem is that DataGrid doesn't know how to display IGrouping. You have to get creative here. Probably extending DataGrid to add this feature would be the best.

If this is not an option, then the only solution I think that is reasonable is to implement data virtualization.

I generally believe that it doesn't make sense to show 5k items at once while the user can only view a fraction.

Just imagine you have 5k items in two groups each of 2.5k items. When the user opens the first, he needs to scroll down 2.5k items before he can see the second group. The UX couldn't get any worse at this point.

If this was my problem to solve, I would reduce the number of items to load. Additionally I would ask myself if the data structure is the correct form to display the data. For example you could create top-down filtering: like first let user select an author, then a created date, etc. Use this filter information to query the database. This should significantly reduce the number of items to display/handle.

Alternatively create an index or use an indexing service like Elastic Search. Such service come with a very advanced query syntax that allow to search/filter the indexed documents more comfortable.

What you are currently doing is not efficient at all and provides a really bad UX.

The following example extends the basic example from above.
You need to bind your DataGrid to the FilteredItemsSource property while you populate the UnfilteredDocuments property with the data from the database.

The example also shows how to replace the CollectionViewSource in XAML with a ICollectionView that is more convenient to handle from C# (code-behind).

it also shows how to gracefully toggle a ProgressBar using the .NET BooleanToVisibilityConverter.

MainWindow.xaml

<Window>
  <Window.Resources>

    <!-- Use the existing .NET value converter -->
    <BooleanToVisibilityConverter x:Key="BooleanToVisibilityConverter" />
  </Window.Resources>

  <StackPanel>
    <ProgressBar IsIndeterminate="True"
                 Height="4"
                 Visibility="{Binding ElementName=Window, Path=IsFilterInProgress, Converter={StaticResource BooleanToVisibilityConverter}}"  />

    <DataGrid ItemsSource="{Binding FilteredItemsSource, Mode=OneTine}" 
              VirtualizingPanel.IsVirtualizingWhenGrouping="True"
              AutoGenerateColumns="False">
    </DataGrid>
  </StackPanel>
</Window>

*MainWindow.xaml.cs

// The binding source for the ProgressBar. 
// Can be bound to Visibility or used as predicate for a Trigger
// This property must be implemented as dependency property!
public bool IsFilterInProgress { get; private set; }

// Binding source for the ItemsControl
public Documents FilteredItemsSource { get; } = new Documents();

// Structure for the database data
private List<Document> UnfilteredDocuments { get; } = new List<Document>();


/* Define fast O(1) lookup collections */
private HashSet<string> SelectedDisciplines { get; set; }
private HashSet<string> SelectedDocumentTypes { get; set; }
private HashSet<string> SelectedFileTypes { get; set; }
private object SyncLock { get; } = new object();

// Constructor
public MainWindow()
{
  InitializeComponent();

  // Enable CollectionChanged propagation to the UI thread
  // when updating a INotifyCollectionChanged collection from a background thread
  BindingOperations.EnableCollectionSynchronization(this.FilteredItemsSource, this.SyncLock);
} 

// Could be invoked from a SelectionChanged event handler
// (what ever the source 'disciplineFilters.SelectedItems' is)
private async void OnDisciplineSelectedItemsChanged(object sender, EventArgs e)
{
  this.SelectedDisciplines = new HashSet<string>(this.disciplineFilters.SelectedItems.Select(item => item.ToString()));
  await ApplyDocumentFilterAsync();
}

// Could be invoked from a SelectionChanged event handler
// (what ever the source 'docTypeFilters.SelectedItems' is)
private async void OnDocTypeSelectedItemsChanged(object sender, EventArgs e)
{
  this.SelectedDocumentTypes = new HashSet<string>(this.docTypeFilters.SelectedItems.Select(item => item.ToString()));
  await ApplyDocumentFilterAsync();
}

// Could be invoked from a SelectionChanged event handler
// (what ever the source 'fileTypeFilters.SelectedItems' is)
private async void OnFileTypeSelectedItemsChanged(object sender, EventArgs e)
{
  this.SelectedFileTypes = new HashSet<string>(this.fileTypeFilters.SelectedItems.Select(item => item.ToString()));
  await ApplyDocumentFilterAsync();
}

private async Task ApplyDocumentFilterAsync()
{
  // Show the ProgressBar
  this.IsFilterInProgress = true;

  // Allow displaying of a progress bar (prevent the UI from freezing)
  await Task.Run(FilterAndSortDocuments);

  // Because grouping is actually happening in the UI (by creating GroupItems)
  // we can't group on a background thread.
  GroupDocuments();

  // Hide the ProgressBar
  this.IsFilterInProgress = false;
}

// Improve performance by filtering and sorting in one step.
// Use FilterDocuments() if filtering alone (no sorting) is required.
private void FilterAndSortDocuments()
{
  IEnumerable<Document> filteredDocuments = GetFilteredDocuments();

  // For example sort descending by the property Document.Id
  IOrderedEnumerable<Document> filteredAndSortedDocuments = filteredDocuments
    .OrderByDescending(document => document.Id);
  
  this.FilteredItemsSource.AddRange(filteredAndSortedDocuments);
}

private void FilterDocuments()
{
  this.FilteredItemsSource.Clear();
  IEnumerable<Document> filteredDocuments = GetFilteredDocuments();
  this.FilteredItemsSource.AddRange(filteredDocuments);
}

private void GroupDocuments()
{
  ICollectionView filteredItemsSourceCollectionView = CollectionViewSource.GetDefaultView(this.FilteredItemsSource);

  // Allow multiple GroupDescription.Add() and Clear() 
  // without raising change notifications every time. 
  // A single change notification is raised after leaving the using scope.
  using (var deferredRefreshContext = filteredItemsSourceCollectionView.DeferResfresh())
  {
    GroupDescriptions groupDescriptions = filteredItemsSourceCollectionView.GroupDescriptions;
    groupDescriptions.Clear();
    groupDescriptions.Add(new PropertyGroupDescription(nameof(Document.Author)));
  }
}

private IEnumerable<Document> GetFilteredDocuments()
{
  IEnumerable<Document> filteredDocuments = this.UnfilteredDocuments.Where(IsDocumentAccepted);  
  return filteredDocuments;
}

private bool IsDocumentAccepted(Document document)
  => this.SelectedDisciplines.Contains(doc.Discipline)     
    && this.SelectedDocumentTypes.Contains(doc.Type) 
    && this.SelectedFileTypes.Contains(doc.Extension);

Upvotes: 0

Andy
Andy

Reputation: 12276

There are a couple of problems with your design here.

The way the filter of a collectionview works is it iterates through the collection one by one and returns true/false.

EDIT: Experimentation seems to confirm this statement is true. AFAIK virtualisation is purely in creation of UI from the collection. Collectionviewsource > Collectionview > Itemssource. Creation of UI rows is virtualised by the virtualising stackpanel but the whole collection will be read into itemssource.

Your filter is complicated and will take a while per item.

It's running 5000 times.

You should not use that approach to filter.

A rethink and fairly substantial refactor is advisable.

Do all your processing and filtering in a Task you run as a background thread.

Forget all that synchronisation context stuff.

Once you've done your processing, return a List of your finalised data back to the UI thread

  async Task<List<Document>> GetMyDocumentsAsync
  {
     // processing filtering and really expensive stuff.
     return myListOfDocuments;
  }

If that doesn't get edited or sorted then set a List property your itemssource is bound to. If it does either then new up an observablecollection

  YourDocuments = new Observablecollection<Document>(yourReturnedList);

passing your list as a constructor paremeter and set a observablecollection property your itemssource is bound to.

Hence you do ALL your expensive processing on a background thread.

That is returned to the UI thread as a collection.

You set itemssource to that via binding.

The custom observablecollection is a bad idea. You should just use List or Observablecollection where t is a viewmodel. Any viewmodel should implement inotifypropertychanged. Always.

Two caveats.

Minimise the number of rows you present to the UI.

If it's more than a couple of hundred then consider paging and maybe an intermediate cache.

Remove this out your binding

    , UpdateSourceTrigger=PropertyChanged

And never use it again until you know what it does.

Some generic datagrid advice:

Avoid column virtualisation.

Minimise the number of columns you bind.

If you can, have fixed column widths.

Consider the simpler listview rather than datagrid.

Upvotes: 1

Related Questions