9

I have a custom BindingList that I want create a custom AddRange method for.

public class MyBindingList<I> : BindingList<I>
{
    ...

    public void AddRange(IEnumerable<I> vals)
    {
        foreach (I v in vals)
            Add(v);
    }
}

My problem with this is performance is terrible with large collections. The case I am debugging now is trying to add roughly 30,000 records, and taking an unacceptable amount of time.

After looking into this issue online, it seems like the problem is that the use of Add is resizing the array with each addition. This answer I think summarizes it as :

If you are using Add, it is resizing the inner array gradually as needed (doubling)

What can I do in my custom AddRange implementation to specify the size the BindingList needs to resize to be based on the item count, rather than letting it constantly re-allocate the array with each item added?

Community
  • 1
  • 1
Rachel
  • 130,264
  • 66
  • 304
  • 490
  • 4
    It's highly unlikely that inner list resizing might take "unacceptable amount of time" for just 30000 items. Most likely real problem is it raises changed event every time you add new item, and you have some handlers for that event (like list is used by some UI control). You can easily test that by just creating binding list and filling it with 30k items, without doing anything else. You will see it takes few milliseconds at most. PS: it will not resize list with each new item also, it will double its size when limit is reached. – Evk Apr 10 '17 at 20:20
  • 1
    @Evk After some tests, you are correct. My problem is with the change events. Thanks for confirming! – Rachel Apr 10 '17 at 20:31

2 Answers2

12

CSharpie explained in his answer that the bad performance is due to the ListChanged-event firing after each Add, and showed a way to implement AddRange for your custom BindingList.

An alternative would be to implement the AddRange functionality as an extension method for BindingList<T>. Based on on CSharpies implementation:

/// <summary>
/// Extension methods for <see cref="System.ComponentModel.BindingList{T}"/>.
/// </summary>
public static class BindingListExtensions
{
  /// <summary>
  /// Adds the elements of the specified collection to the end of the <see cref="System.ComponentModel.BindingList{T}"/>,
  /// while only firing the <see cref="System.ComponentModel.BindingList{T}.ListChanged"/>-event once.
  /// </summary>
  /// <typeparam name="T">
  /// The type T of the values of the <see cref="System.ComponentModel.BindingList{T}"/>.
  /// </typeparam>
  /// <param name="bindingList">
  /// The <see cref="System.ComponentModel.BindingList{T}"/> to which the values shall be added.
  /// </param>
  /// <param name="collection">
  /// The collection whose elements should be added to the end of the <see cref="System.ComponentModel.BindingList{T}"/>.
  /// The collection itself cannot be null, but it can contain elements that are null,
  /// if type T is a reference type.
  /// </param>
  /// <exception cref="ArgumentNullException">values is null.</exception>
  public static void AddRange<T>(this System.ComponentModel.BindingList<T> bindingList, IEnumerable<T> collection)
  {
    // The given collection may not be null.
    if (collection == null)
      throw new ArgumentNullException(nameof(collection));

    // Remember the current setting for RaiseListChangedEvents
    // (if it was already deactivated, we shouldn't activate it after adding!).
    var oldRaiseEventsValue = bindingList.RaiseListChangedEvents;

    // Try adding all of the elements to the binding list.
    try
    {
      bindingList.RaiseListChangedEvents = false;

      foreach (var value in collection)
        bindingList.Add(value);
    }

    // Restore the old setting for RaiseListChangedEvents (even if there was an exception),
    // and fire the ListChanged-event once (if RaiseListChangedEvents is activated).
    finally
    {
      bindingList.RaiseListChangedEvents = oldRaiseEventsValue;

      if (bindingList.RaiseListChangedEvents)
        bindingList.ResetBindings();
    }
  }
}

This way, depending on your needs, you might not even need to write your own BindingList-subclass.

Timitry
  • 2,646
  • 20
  • 26
8

You can pass in a List in the constructor and make use of List<T>.Capacity.

But i bet, the most significant speedup will come form suspending events when adding a range. So I included both things in my example code.

Probably needs some finetuning to handle some worst cases and what not.

public class MyBindingList<I> : BindingList<I>
{
    private readonly List<I> _baseList;

    public MyBindingList() : this(new List<I>())
    {

    }

    public MyBindingList(List<I> baseList) : base(baseList)
    {
        if(baseList == null)
            throw new ArgumentNullException();            
        _baseList = baseList;
    }

    public void AddRange(IEnumerable<I> vals)
    {
        ICollection<I> collection = vals as ICollection<I>;
        if (collection != null)
        {
            int requiredCapacity = Count + collection.Count;
            if (requiredCapacity > _baseList.Capacity)
                _baseList.Capacity = requiredCapacity;
        }

        bool restore = RaiseListChangedEvents;
        try
        {
            RaiseListChangedEvents = false;
            foreach (I v in vals)
                Add(v); // We cant call _baseList.Add, otherwise Events wont get hooked.
        }
        finally
        {
            RaiseListChangedEvents = restore;
            if (RaiseListChangedEvents)
                ResetBindings();
        }
    }
}

You cannot use the _baseList.AddRangesince BindingList<T> wont hook the PropertyChanged event then. You can bypass this only using Reflection by calling the private Method HookPropertyChanged for each Item after AddRange. this however only makes sence if vals (your method parameter) is a collection. Otherwise you risk enumerating the enumerable twice.

Thats the closest you can get to "optimal" without writing your own BindingList. Which shouldnt be too dificult as you could copy the source code from BindingList and alter the parts to your needs.

CSharpie
  • 9,195
  • 4
  • 44
  • 71
  • I don't like the idea of creating a different baseList because then I have to override all the other functionality, like Add, Remove, etc. And in this specific case, I am binding to the list and need the change notifications of `BindingList`. I thought I had tested disabling ListChanged events in the past and it didn't help, but I will check again. – Rachel Apr 10 '17 at 19:44
  • 1
    @Rachel i dont see why you need to override any functionality. What exactly doesnt work when using my approach? – CSharpie Apr 10 '17 at 19:45
  • Aren't you using a `List` as the base holder for your items instead of the default implementation of `BindingList`? So if I add items using `myList.Add(something)` it will get added to the `BindingList` but not to `_baseList` – Rachel Apr 10 '17 at 19:48
  • 1
    @Rachel Yes i do, but [so does BindingList](http://referencesource.microsoft.com/#mscorlib/system/collections/objectmodel/collection.cs,27) if you dont pass a List into the constructor. Bindinglist wraps arround its baseList. All my code does is get a reference to it so we can modify Capactiy. I still call `Add` and not `_baseList.Add` – CSharpie Apr 10 '17 at 19:50
  • What's the point of `_bindingList` then? I would think we'd need to alter `BindingList.Capacity`, not `_baseList.Capacity`. I am testing again though with the `RaiseListChangedEvents` property though, and I think you might be onto something here – Rachel Apr 10 '17 at 19:55
  • @Rachel [here is a working example](https://dotnetfiddle.net/Z69i2y) BindingList works with every `IList`. However only `List` has a `Capacity`. You can also pass an array into a `BindingList` which is immutable in its size. – CSharpie Apr 10 '17 at 20:01
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/141384/discussion-between-csharpie-and-rachel). – CSharpie Apr 10 '17 at 20:07