Remove some allocation from NotifyCollectionChangedEventArgs (#54899)

* Remove some allocation from NotifyCollectionChangedEventArgs

When initialized with a single object rather than a list of objects (single object is very common), the args is allocating an object[] and then wrapping that object[] in a ReadOnlyList.  We can instead just allocate a simple read-only IList wrapper for the object directly; that also makes accessing it faster.

Additionally, when constructing an instance using `public NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction action, IList? changedItems, int index, int oldIndex)`, even though the same collection instance is used for both the new and old items, it wraps it twice.  We can wrap it once.

Along the way I also simplified the code, enabling the fields to become readonly, removing duplicate assignments of fields, etc.

* Address PR feedback

src/libraries/System.ObjectModel/src/System.ObjectModel.csproj

@@ -5,6 +5,7 @@
  <Nullable>enable</Nullable>
  </PropertyGroup>
  <ItemGroup>
+ <Compile Include="System\Collections\CollectionHelpers.cs" />
  <Compile Include="System\Collections\Generic\DebugView.cs" />
  <Compile Include="System\Collections\Specialized\INotifyCollectionChanged.cs" />
  <Compile Include="System\Collections\Specialized\NotifyCollectionChangedAction.cs" />

src/libraries/System.ObjectModel/src/System/Collections/CollectionHelpers.cs

@@ -0,0 +1,73 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+
+namespace System.Collections
+{
+ internal static class CollectionHelpers
+ {
+ internal static void ValidateCopyToArguments(int sourceCount, Array array, int index)
+ {
+ if (array == null)
+ {
+ throw new ArgumentNullException(nameof(array));
+ }
+
+ if (array.Rank != 1)
+ {
+ throw new ArgumentException(SR.Arg_RankMultiDimNotSupported, nameof(array));
+ }
+
+ if (array.GetLowerBound(0) != 0)
+ {
+ throw new ArgumentException(SR.Arg_NonZeroLowerBound, nameof(array));
+ }
+
+ if (index < 0 || index > array.Length)
+ {
+ throw new ArgumentOutOfRangeException(nameof(index), SR.ArgumentOutOfRange_NeedNonNegNum);
+ }
+
+ if (array.Length - index < sourceCount)
+ {
+ throw new ArgumentException(SR.Arg_ArrayPlusOffTooSmall);
+ }
+ }
+
+ internal static void CopyTo<T>(ICollection<T> collection, Array array, int index)
+ {
+ ValidateCopyToArguments(collection.Count, array, index);
+
+ if (collection is ICollection nonGenericCollection)
+ {
+ // Easy out if the ICollection<T> implements the non-generic ICollection
+ nonGenericCollection.CopyTo(array, index);
+ }
+ else if (array is T[] items)
+ {
+ collection.CopyTo(items, index);
+ }
+ else
+ {
+ // We can't cast array of value type to object[], so we don't support widening of primitive types here.
+ if (array is not object?[] objects)
+ {
+ throw new ArgumentException(SR.Argument_InvalidArrayType, nameof(array));
+ }
+
+ try
+ {
+ foreach (T item in collection)
+ {
+ objects[index++] = item;
+ }
+ }
+ catch (ArrayTypeMismatchException)
+ {
+ throw new ArgumentException(SR.Argument_InvalidArrayType, nameof(array));
+ }
+ }
+ }
+ }
+}

src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ReadOnlyDictionary.cs

@@ -175,26 +175,7 @@ void IDictionary.Remove(object key)
 
  void ICollection.CopyTo(Array array, int index)
  {
- if (array == null)
- {
- throw new ArgumentNullException(nameof(array));
- }
- if (array.Rank != 1)
- {
- throw new ArgumentException(SR.Arg_RankMultiDimNotSupported, nameof(array));
- }
- if (array.GetLowerBound(0) != 0)
- {
- throw new ArgumentException(SR.Arg_NonZeroLowerBound, nameof(array));
- }
- if (index < 0 || index > array.Length)
- {
- throw new ArgumentOutOfRangeException(nameof(index), SR.ArgumentOutOfRange_NeedNonNegNum);
- }
- if (array.Length - index < Count)
- {
- throw new ArgumentException(SR.Arg_ArrayPlusOffTooSmall);
- }
+ CollectionHelpers.ValidateCopyToArguments(Count, array, index);
 
  if (array is KeyValuePair<TKey, TValue>[] pairs)
  {
@@ -313,7 +294,7 @@ bool ICollection<TKey>.Remove(TKey item)
 
  void ICollection.CopyTo(Array array, int index)
  {
- ReadOnlyDictionaryHelpers.CopyToNonGenericICollectionHelper<TKey>(_collection, array, index);
+ CollectionHelpers.CopyTo(_collection, array, index);
  }
 
  bool ICollection.IsSynchronized => false;
@@ -363,75 +344,12 @@ bool ICollection<TValue>.Remove(TValue item)
 
  void ICollection.CopyTo(Array array, int index)
  {
- ReadOnlyDictionaryHelpers.CopyToNonGenericICollectionHelper<TValue>(_collection, array, index);
+ CollectionHelpers.CopyTo(_collection, array, index);
  }
 
  bool ICollection.IsSynchronized => false;
 
  object ICollection.SyncRoot => (_collection is ICollection coll) ? coll.SyncRoot : this;
  }
  }
-
- // To share code when possible, use a non-generic class to get rid of irrelevant type parameters.
- internal static class ReadOnlyDictionaryHelpers
- {
- // Abstracted away to avoid redundant implementations.
- internal static void CopyToNonGenericICollectionHelper<T>(ICollection<T> collection, Array array, int index)
- {
- if (array == null)
- {
- throw new ArgumentNullException(nameof(array));
- }
- if (array.Rank != 1)
- {
- throw new ArgumentException(SR.Arg_RankMultiDimNotSupported, nameof(array));
- }
- if (array.GetLowerBound(0) != 0)
- {
- throw new ArgumentException(SR.Arg_NonZeroLowerBound, nameof(array));
- }
- if (index < 0)
- {
- throw new ArgumentOutOfRangeException(nameof(index), SR.ArgumentOutOfRange_NeedNonNegNum);
- }
- if (array.Length - index < collection.Count)
- {
- throw new ArgumentException(SR.Arg_ArrayPlusOffTooSmall);
- }
-
- // Easy out if the ICollection<T> implements the non-generic ICollection
- if (collection is ICollection nonGenericCollection)
- {
- nonGenericCollection.CopyTo(array, index);
- return;
- }
-
- if (array is T[] items)
- {
- collection.CopyTo(items, index);
- }
- else
- {
- // We can't cast array of value type to object[], so we don't support
- // widening of primitive types here.
- object?[]? objects = array as object?[];
- if (objects == null)
- {
- throw new ArgumentException(SR.Argument_InvalidArrayType, nameof(array));
- }
-
- try
- {
- foreach (var item in collection)
- {
- objects[index++] = item;
- }
- }
- catch (ArrayTypeMismatchException)
- {
- throw new ArgumentException(SR.Argument_InvalidArrayType, nameof(array));
- }
- }
- }
- }
 }