Skip to content

Commit

Permalink
Return singleton enumerators from IEnumerable.GetEnumerator for empty…
Browse files Browse the repository at this point in the history 
… collections (#82499)

* Return singleton enumerators from IEnumerable.GetEnumerator for empty collections

Change the `IEnumerable<T>.GetEnumerator()` implementations on our core collection types to special-case Count==0 in order to return a single enumerator instead of allocating one a new each time.  This saves an allocation when enumerating these collections via the interface in exchange for an extra length check as part of GetEnumerator.

* Address PR feedback

- Create helper function for empty enumerator
- Add tests for singletons

* Fix a few tests
  • Loading branch information
@stephentoub
stephentoub authored Feb 23, 2023
1 parent f8b9b8d commit dc6ad37
Show file tree
Hide file tree
Showing 45 changed files with 319 additions and 157 deletions.
4 changes: 4 additions & 0 deletions src/libraries/Common/src/System/Collections/Generic/EnumerableHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ namespace System.Collections.Generic
/// </summary>
internal static partial class EnumerableHelpers
{
/// <summary>Gets an enumerator singleton for an empty collection.</summary>
internal static IEnumerator<T> GetEmptyEnumerator<T>() =>
((IEnumerable<T>)Array.Empty<T>()).GetEnumerator();

/// <summary>Converts an enumerable to an array using the same logic as List{T}.</summary>
/// <param name="source">The enumerable to convert.</param>
/// <param name="length">The number of items stored in the resulting array, 0-indexed.</param>
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/tests/System/Collections/IDictionary.Generic.Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ public void IDictionary_Generic_Keys_Enumeration_ParentDictionaryModifiedInvalid
ICollection<TKey> keys = dictionary.Keys;
IEnumerator<TKey> keysEnum = keys.GetEnumerator();
dictionary.Add(GetNewKey(dictionary), CreateTValue(3432));
if (IDictionary_Generic_Keys_Values_Enumeration_ThrowsInvalidOperation_WhenParentModified)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : IDictionary_Generic_Keys_Values_Enumeration_ThrowsInvalidOperation_WhenParentModified)
{
Assert.Throws<InvalidOperationException>(() => keysEnum.MoveNext());
Assert.Throws<InvalidOperationException>(() => keysEnum.Reset());
Expand Down Expand Up @@ -528,7 +528,7 @@ public void IDictionary_Generic_Values_Enumeration_ParentDictionaryModifiedInval
ICollection<TValue> values = dictionary.Values;
IEnumerator<TValue> valuesEnum = values.GetEnumerator();
dictionary.Add(GetNewKey(dictionary), CreateTValue(3432));
if (IDictionary_Generic_Keys_Values_Enumeration_ThrowsInvalidOperation_WhenParentModified)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : IDictionary_Generic_Keys_Values_Enumeration_ThrowsInvalidOperation_WhenParentModified)
{
Assert.Throws<InvalidOperationException>(() => valuesEnum.MoveNext());
Assert.Throws<InvalidOperationException>(() => valuesEnum.Reset());
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/tests/System/Collections/IDictionary.NonGeneric.Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ public void IDictionary_NonGeneric_Keys_Enumeration_ParentDictionaryModifiedInva
ICollection keys = dictionary.Keys;
IEnumerator keysEnum = keys.GetEnumerator();
dictionary.Add(GetNewKey(dictionary), CreateTValue(3432));
if (IDictionary_NonGeneric_Keys_Values_ParentDictionaryModifiedInvalidates)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : IDictionary_NonGeneric_Keys_Values_ParentDictionaryModifiedInvalidates)
{
Assert.Throws<InvalidOperationException>(() => keysEnum.MoveNext());
Assert.Throws<InvalidOperationException>(() => keysEnum.Reset());
Expand Down Expand Up @@ -487,7 +487,7 @@ public virtual void IDictionary_NonGeneric_Values_Enumeration_ParentDictionaryMo
ICollection values = dictionary.Values;
IEnumerator valuesEnum = values.GetEnumerator();
dictionary.Add(GetNewKey(dictionary), CreateTValue(3432));
if (IDictionary_NonGeneric_Keys_Values_ParentDictionaryModifiedInvalidates)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : IDictionary_NonGeneric_Keys_Values_ParentDictionaryModifiedInvalidates)
{
Assert.Throws<InvalidOperationException>(() => valuesEnum.MoveNext());
Assert.Throws<InvalidOperationException>(() => valuesEnum.Reset());
Expand Down
58 changes: 47 additions & 11 deletions src/libraries/Common/tests/System/Collections/IEnumerable.Generic.Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,20 @@ public abstract partial class IEnumerable_Generic_Tests<T> : TestBase<T>
/// </summary>
protected virtual bool Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException => true;

/// <summary>
/// When calling MoveNext or Reset after modification of an empty enumeration, the resulting behavior is
/// undefined. Tests are included to cover two behavioral scenarios:
/// - Throwing an InvalidOperationException
/// - Execute MoveNext or Reset.
///
/// If this property is set to true, the tests ensure that the exception is thrown. The default value is
/// <see cref="Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException"/>.
/// </summary>
protected virtual bool Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException => Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException;

/// <summary>Whether the enumerator returned from GetEnumerator is a singleton instance when the collection is empty.</summary>
protected virtual bool Enumerator_Empty_UsesSingletonInstance => false;

/// <summary>
/// Specifies whether this IEnumerable follows some sort of ordering pattern.
/// </summary>
Expand Down Expand Up @@ -302,6 +316,30 @@ private void VerifyEnumerator(

#region GetEnumerator()

[Fact]
public void IEnumerable_NonGeneric_GetEnumerator_EmptyCollection_UsesSingleton()
{
IEnumerable enumerable = GenericIEnumerableFactory(0);

IEnumerator enumerator1 = enumerable.GetEnumerator();
try
{
IEnumerator enumerator2 = enumerable.GetEnumerator();
try
{
Assert.Equal(Enumerator_Empty_UsesSingletonInstance, ReferenceEquals(enumerator1, enumerator2));
}
finally
{
if (enumerator2 is IDisposable d2) d2.Dispose();
}
}
finally
{
if (enumerator1 is IDisposable d1) d1.Dispose();
}
}

[Theory]
[MemberData(nameof(ValidCollectionSizes))]
public void IEnumerable_Generic_GetEnumerator_NoExceptionsWhileGetting(int count)
Expand Down Expand Up @@ -381,7 +419,7 @@ public void IEnumerable_Generic_Enumerator_MoveNext_ModifiedBeforeEnumeration_Th
{
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.MoveNext());
}
Expand Down Expand Up @@ -427,7 +465,7 @@ public void IEnumerable_Generic_Enumerator_MoveNext_ModifiedDuringEnumeration_Th
enumerator.MoveNext();
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.MoveNext());
}
Expand Down Expand Up @@ -471,7 +509,7 @@ public void IEnumerable_Generic_Enumerator_MoveNext_ModifiedAfterEnumeration_Thr
while (enumerator.MoveNext()) ;
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.MoveNext());
}
Expand Down Expand Up @@ -602,8 +640,7 @@ public void IEnumerable_Generic_Enumerator_Current_BeforeFirstMoveNext_Undefined
IEnumerable<T> enumerable = GenericIEnumerableFactory(count);
using (IEnumerator<T> enumerator = enumerable.GetEnumerator())
{
if (Enumerator_Current_UndefinedOperation_Throws ||
(count == 0 && Enumerator_Empty_Current_UndefinedOperation_Throws))
if (count == 0 ? Enumerator_Empty_Current_UndefinedOperation_Throws : Enumerator_Current_UndefinedOperation_Throws)
Assert.Throws<InvalidOperationException>(() => enumerator.Current);
else
current = enumerator.Current;
Expand All @@ -619,8 +656,7 @@ public void IEnumerable_Generic_Enumerator_Current_AfterEndOfEnumerable_Undefine
using (IEnumerator<T> enumerator = enumerable.GetEnumerator())
{
while (enumerator.MoveNext()) ;
if (Enumerator_Current_UndefinedOperation_Throws ||
(count == 0 && Enumerator_Empty_Current_UndefinedOperation_Throws))
if (count == 0 ? Enumerator_Empty_Current_UndefinedOperation_Throws : Enumerator_Current_UndefinedOperation_Throws)
Assert.Throws<InvalidOperationException>(() => enumerator.Current);
else
current = enumerator.Current;
Expand All @@ -639,7 +675,7 @@ public void IEnumerable_Generic_Enumerator_Current_ModifiedDuringEnumeration_Und
{
if (ModifyEnumerable(enumerable))
{
if (Enumerator_Current_UndefinedOperation_Throws)
if (count == 0 ? Enumerator_Empty_Current_UndefinedOperation_Throws : Enumerator_Current_UndefinedOperation_Throws)
Assert.Throws<InvalidOperationException>(() => enumerator.Current);
else
current = enumerator.Current;
Expand Down Expand Up @@ -694,7 +730,7 @@ public void IEnumerable_Generic_Enumerator_Reset_ModifiedBeforeEnumeration_Throw
{
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.Reset());
}
Expand Down Expand Up @@ -737,7 +773,7 @@ public void IEnumerable_Generic_Enumerator_Reset_ModifiedDuringEnumeration_Throw
enumerator.MoveNext();
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.Reset());
}
Expand Down Expand Up @@ -781,7 +817,7 @@ public void IEnumerable_Generic_Enumerator_Reset_ModifiedAfterEnumeration_Throws
while (enumerator.MoveNext()) ;
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.Reset());
}
Expand Down
Loading

0 comments on commit dc6ad37

Please sign in to comment.