Batch updating UI component through observable collection with large amounts of data WinRT

Question

I have an WinRT application that fires notifications everytime it recieves data from a device. I also have a UI control that is databound to an observable collection which I wish to add the new data to

While I have made it capable of updating the observable collection, it causes the UI to become very laggy, as the amount of data generated it fast. It would therefore be better to batch the update, maybe every few hundred milliseconds.

Below shows a snippet of the code. First I create the periodic timer

 TimerElapsedHandler f = new TimerElapsedHandler(batchUpdate);
        CreatePeriodicTimer(f, new TimeSpan(0, 0, 3));

Below is my event handler for when new data comes in, along with the temporary list that stores the information

List<FinancialStuff> lst = new List<FinancialStuff>();
    async void myData_ValueChanged(GattCharacteristic sender, GattValueChangedEventArgs args)
    {
        var data = new byte[args.CharacteristicValue.Length];
        DataReader.FromBuffer(args.CharacteristicValue).ReadBytes(data);
        lst.Add(new FinancialStuff() { Time = "DateTime.UtcNow.ToString("mm:ss.ffffff")", Amount = data[0] });

    }

Then my batch update, which is called peroidically

    private void batchUpdate(ThreadPoolTimer source)
    {

            AddItem<FinancialStuff>(financialStuffList, lst);                        

    }

Then finally, for testing I want to clear the observable collection and items.

    public async void AddItem<T>(ObservableCollection<T> oc, List<T> items)
    {

        lock (items)
        {

            if (Dispatcher.HasThreadAccess)
            {
                foreach (T item in items)
                    oc.Add(item);

            }
            else
            {
                Dispatcher.RunAsync(CoreDispatcherPriority.Low, () =>
                {

                    oc.Clear();
                    for (int i = 0; i < items.Count; i++)
                    {
                        items.Count());
                        oc.Add(items[i]);
                    }
                    lst.Clear();

                });
            }
        }

    }

While this seems to work, after a few updates the UI locks up and it updates very slowly/if not at all. For testing, it's only getting a few hundred items in the list by the time the timer is fired.

Can anybody enlighten me as to why as to why this is happening - I'm presuming my design is very poor.

Thanks

Answer 1

You're not locking your list in the event handler

// "lst" is never locked in your event handler
List<FinancialStuff> lst = new List<FinancialStuff>();
lst.Add(new FinancialStuff() { Time = "DateTime.UtcNow.ToString("mm:ss.ffffff")", Amount = data[0] });

Passing "lst" above to your async method

AddItem<FinancialStuff>(financialStuffList, lst);    

You're locking "items" below, which is really "lst" above. However, you're adding to the list while your processing it. I assume the event handler has a higher priority so your processing is slower than your add. This can lead to "i < items.Count" being true forever.

public async void AddItem<T>(ObservableCollection<T> oc, List<T> items)
{
    // "lst" reference is locked here, but it wasn't locked in the event handler 
    lock (items)
    {
        if (Dispatcher.HasThreadAccess)
        {
            foreach (T item in items)
                oc.Add(item);
        }
        else
        {
            Dispatcher.RunAsync(CoreDispatcherPriority.Low, () =>
            {
                oc.Clear();

                // This may never exit the for loop
                for (int i = 0; i < items.Count; i++)
                {
                    items.Count());
                    oc.Add(items[i]);
                }
                lst.Clear();
            });
        }
    }
}

EDIT: Do you need to view every piece of data? There is going to be some overhead when using a lock. If you're getting data quicker than the speed of how fast you can render it, you'll eventually be backed up and/or have a very large collection to render, which might also cause some problems. I suggest you do some filtering to only draw the last x number of items (say 100). Also, I'm not sure why you need the if (Dispatcher.HasThreadAccess) condition either.

Try the following:

public async void AddItem<T>(ObservableCollection<T> oc, List<T> items)
{
    // "lst" reference is locked here, but it wasn't locked in the event handler 
    lock (items)
    {
        // Change this to what you want
        const int maxSize = 100;

        // Make sure it doesn't index out of bounds
        int startIndex = Math.Max(0, items.Count - maxSize);
        int length = items.Count - startIndex;

        List<T> itemsToRender = items.GetRange(startIndex, length);

        // You can clear it here in your background thread.  The references to the objects
        // are now in the itemsToRender list.
        lst.Clear();

        // Dispatcher.RunAsync(CoreDispatcherPriority.Low, () =>
        // Please verify this is the correct syntax
        Dispatcher.Run(() =>
        {
            // At second look, this might need to be locked too
            // EDIT: This probably will just add overhead now that it's not running async.
            // You can probably remove this lock
            lock(oc)
            {
                oc.Clear();

                for (int i = 0; i < itemsToRender.Count; i++)
                {
                    // I didn't notice it before, but why are you checking the count again?
                    // items.Count());
                    oc.Add(itemsToRender[i]);
                }
             }
        });
    }
}

EDIT2: Since your AddItem method is already on a background thread, I don't think you need to run the Dispatcher.RunAsync. Instead, I think it might be desirable for it to block so you don't end up with multiple calls to that section of code. Try using Dispatcher.Run instead. I've updated the code example above to show the changes. You shouldn't need the lock on the oc anymore since the lock on the items is good enough. Also, verify the syntax for Dispatcher.Run is correct.