CODEWITHSUNDEEP
c#multithreadingui-thread
Douglas Gaskell
Douglas Gaskell

Reputation: 10030

Updating my ObservableCollection on a worker thread still hangs my UI

I have a log window in my application, when I have a few thousand logs, filtering them to include or exclude different log levels makes the UI unresponsive for a period of time from the work load. So I have tried to move the heavy lifting to a worker thread, and am having the same issue still.

I am using an ObservableCollection to hold the log information in my model, and I just reference that directly with my ViewModel. I have used BindingOperations.EnableCollectionSynchronization() to let my worker thread update my observable collection without Dispatching it to the UI thread.

I run the following to update the collection on a worker thread with a 

Task.Run(new Action(() => FilterList()));

Methods:

    private void FilterList()
    {
        //Necessary even with EnableCollectionSynchronization
        App.Current.Dispatcher.Invoke(new Action(() =>
        {
            FilteredLogEvents.Clear();
        }));

        foreach (LogEvent log in FilterLogEvents())
        {
            FilteredLogEvents.Add(log);
        }

        RaisePropertyChanged("FilteredLogEvents");
        FinishedFilteringLogs();
    }

    //Filters and returns a list of filtered log events
    private List<LogEvent> FilterLogEvents()
    {
        List<LogEvent> selectedEvents = (from x in LogEvents
                                         where ((ViewDebugLogs == true) ? x.Level == "Debug" : false)
                                         || ((ViewErrorLogs == true) ? x.Level == "Error" : false)
                                         || ((ViewInfoLogs == true) ? x.Level == "Info" : false)
                                         select x).ToList();
        return selectedEvents;
    }

This causes the UI to freeze on the foreach. I also tried just newing up an ObservableCollection and then assigning FilteredLogEvents to it with FilteredLogEvents = myNewCollection; this also causes the UI to freeze for a short while during that process.

If I use Thread.Sleep(1) within the foreach loop the UI remains responsive, though this seems like an inelegant and hacky solution.

What do I need to do to make this work?

Edit: A bit more code context from this class (LogEntries) The callback for FinishedFilteringLogsEventHandler goes back to the ViewModel to change a bool that enables a couple checkboxes when the filtering is complete.

    //Constructor
    public LogEntries()
    {
        foreach(NlogViewerTarget target in NLog.LogManager.Configuration.AllTargets.Where(t=>t is NlogViewerTarget).Cast<NlogViewerTarget>())
        {
            target.RecieveLog += RecieveLog;
        }

        FilteredLogEvents = new ObservableCollection<LogEvent>();
        BindingOperations.EnableCollectionSynchronization(FilteredLogEvents, filteredLogEventsLock);
    }

    public delegate void FinishedFilteringLogsEvent();
    public FinishedFilteringLogsEvent FinishedFilteringLogsEventHandler;

    private object filteredLogEventsLock = new object();

    public ObservableCollection<LogEvent> FilteredLogEvents { get; set; }

Upvotes: 0

Views: 614

Answers (1)

Harald Coppoolse
Harald Coppoolse

Reputation: 30464

Some thoughts to consider to improve the speed and responsiveness of your code

A few days ago I asked a similar question and someone advised me not to use the threadpool for long running Tasks. The thread pool is a collection of available threads, that can be started swiftly in comparison to starting a traditional thread like System.ComponentModel.BackGroundWorker.

Although it takes more time to create and start a real thread, this is no problem for long running tasks.

The number of threads in the thread pool is limited, so better not use it for longer running tasks.

If you run a task, it is only scheduled to run in the near future when a thread is available. If all threads are busy it will take some time before the thread starts.

The change from Task to Backgroundworker is limited. If you really want to stick to tasks, consider creating an async function:

async void FilteredLogEvents.AddRangeAsync(IEnumerable<LogEvent> logEvents)

or maybe better:

async void FilteredLogEvents.SetAsync(IEnumerable<LogEvent> logEvents)

which does the clear and add in one async call.

Make your own function async:

private async void FilterList()
{
    var filteredLogEvents = FilterLogEvents();
    var myTask = Task.Run( () => FilteredLogEvents.SetAsync(filteredLogEvents);
    // if desired do other things.
    // wait until ready:
    await myTask();
    RaisePropertyChanged("FilteredLogEvents");
    FinishedFilteringLogs();
}

By the way: Are you sure that your sequence of logEvents does not change while you are filtering it?

  • If so, why do you use ToList() instead of returning an IEnumerable and use deferred execution?
  • If you are not certain: what happens if during the FilterLogEvents your sequence of logEvents changes?

Upvotes: 1

Related Questions