From 248d92400516a183b1700fc32f2cc5955cde209b Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 2 Mar 2023 10:03:22 -0500 Subject: [PATCH 1/2] Add dedicated empty array enumerator for footprint reduction Its singleton can be used in places where it's less likely the consumer will otherwise be using a T[], such that it's less likely we'll need to generate code for T[]. --- .../src/System/Array.Enumerators.cs | 50 +++++++++++++++++-- .../System/Collections/Generic/Dictionary.cs | 2 +- .../CompilerServices/ConditionalWeakTable.cs | 2 +- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs b/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs index ddb472aaa8f1e..d5b8c35c68b63 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs @@ -67,7 +67,7 @@ public void Reset() } } - internal class SZGenericArrayEnumeratorBase + internal abstract class SZGenericArrayEnumeratorBase { protected readonly Array _array; protected int _index; @@ -104,10 +104,17 @@ public void Dispose() internal sealed class SZGenericArrayEnumerator : SZGenericArrayEnumeratorBase, IEnumerator { - // Array.Empty is intentionally omitted here, since we don't want to pay for generic instantiations that - // wouldn't have otherwise been used. + /// Provides an empty enumerator singleton. + /// + /// If the consumer is using SZGenericArrayEnumerator elsewhere or is otherwise likely + /// to be using T[] elsewhere, this singleton should be used. Otherwise, GenericEmptyArrayEnumerator's + /// singleton should be used instead, as it doesn't reference T[] in order to reduce footprint. + /// #pragma warning disable CA1825 - internal static readonly SZGenericArrayEnumerator Empty = new SZGenericArrayEnumerator(new T[0]); + internal static readonly SZGenericArrayEnumerator Empty = + // Array.Empty is intentionally omitted here, since we don't want to pay for generic instantiations + // that wouldn't have otherwise been used. + new SZGenericArrayEnumerator(new T[0]); #pragma warning restore CA1825 public SZGenericArrayEnumerator(T[] array) @@ -133,4 +140,39 @@ public T Current object? IEnumerator.Current => Current; } + + internal abstract class GenericEmptyArrayEnumeratorBase + { +#pragma warning disable CA1822 // https://github.com/dotnet/roslyn-analyzers/issues/5911 + public bool MoveNext() => false; + + public void Reset() { } + + public void Dispose() { } +#pragma warning restore CA1822 + } + + /// Provides an empty enumerator singleton. + /// + /// If the consumer is using SZGenericArrayEnumerator elsewhere or is otherwise likely + /// to be using T[] elsewhere, SZGenericArrayEnumerator's singleton should be used. Otherwise, + /// this singleton should be used, as it doesn't reference T[] in order to reduce footprint. + /// + internal sealed class GenericEmptyArrayEnumerator : GenericEmptyArrayEnumeratorBase, IEnumerator + { + public static readonly GenericEmptyArrayEnumerator Instance = new(); + + private GenericEmptyArrayEnumerator() { } + + public T Current + { + get + { + ThrowHelper.ThrowInvalidOperationException_EnumCurrent(-1); + return default; + } + } + + object? IEnumerator.Current => Current; + } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs index f959827b1da60..b183a9527af0c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs @@ -339,7 +339,7 @@ private void CopyTo(KeyValuePair[] array, int index) public Enumerator GetEnumerator() => new Enumerator(this, Enumerator.KeyValuePair); IEnumerator> IEnumerable>.GetEnumerator() => - Count == 0 ? SZGenericArrayEnumerator>.Empty : + Count == 0 ? GenericEmptyArrayEnumerator>.Instance : GetEnumerator(); public virtual void GetObjectData(SerializationInfo info, StreamingContext context) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs index 7a8f8da784c08..b868d06ec4a4c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs @@ -258,7 +258,7 @@ IEnumerator> IEnumerable>. { Container c = _container; return c is null || c.FirstFreeEntry == 0 ? - SZGenericArrayEnumerator>.Empty : + GenericEmptyArrayEnumerator>.Instance : new Enumerator(this); } } From 14e1032ff7c994645d0cba208163fbd3d673fb6e Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 2 Mar 2023 11:13:00 -0500 Subject: [PATCH 2/2] Address PR feedback --- .../src/System/Array.Enumerators.cs | 23 ++++++++++++------- .../System/Collections/Generic/Dictionary.cs | 2 +- .../CompilerServices/ConditionalWeakTable.cs | 2 +- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs b/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs index d5b8c35c68b63..0b80108d95286 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs @@ -107,7 +107,7 @@ internal sealed class SZGenericArrayEnumerator : SZGenericArrayEnumeratorBase /// Provides an empty enumerator singleton. /// /// If the consumer is using SZGenericArrayEnumerator elsewhere or is otherwise likely - /// to be using T[] elsewhere, this singleton should be used. Otherwise, GenericEmptyArrayEnumerator's + /// to be using T[] elsewhere, this singleton should be used. Otherwise, GenericEmptyEnumerator's /// singleton should be used instead, as it doesn't reference T[] in order to reduce footprint. /// #pragma warning disable CA1825 @@ -141,11 +141,20 @@ public T Current object? IEnumerator.Current => Current; } - internal abstract class GenericEmptyArrayEnumeratorBase + internal abstract class GenericEmptyEnumeratorBase { #pragma warning disable CA1822 // https://github.com/dotnet/roslyn-analyzers/issues/5911 public bool MoveNext() => false; + public object Current + { + get + { + ThrowHelper.ThrowInvalidOperationException_EnumCurrent(-1); + return default; + } + } + public void Reset() { } public void Dispose() { } @@ -158,13 +167,13 @@ public void Dispose() { } /// to be using T[] elsewhere, SZGenericArrayEnumerator's singleton should be used. Otherwise, /// this singleton should be used, as it doesn't reference T[] in order to reduce footprint. /// - internal sealed class GenericEmptyArrayEnumerator : GenericEmptyArrayEnumeratorBase, IEnumerator + internal sealed class GenericEmptyEnumerator : GenericEmptyEnumeratorBase, IEnumerator { - public static readonly GenericEmptyArrayEnumerator Instance = new(); + public static readonly GenericEmptyEnumerator Instance = new(); - private GenericEmptyArrayEnumerator() { } + private GenericEmptyEnumerator() { } - public T Current + public new T Current { get { @@ -172,7 +181,5 @@ public T Current return default; } } - - object? IEnumerator.Current => Current; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs index b183a9527af0c..339a95ed4ff70 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs @@ -339,7 +339,7 @@ private void CopyTo(KeyValuePair[] array, int index) public Enumerator GetEnumerator() => new Enumerator(this, Enumerator.KeyValuePair); IEnumerator> IEnumerable>.GetEnumerator() => - Count == 0 ? GenericEmptyArrayEnumerator>.Instance : + Count == 0 ? GenericEmptyEnumerator>.Instance : GetEnumerator(); public virtual void GetObjectData(SerializationInfo info, StreamingContext context) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs index b868d06ec4a4c..6e7cb22326fe3 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs @@ -258,7 +258,7 @@ IEnumerator> IEnumerable>. { Container c = _container; return c is null || c.FirstFreeEntry == 0 ? - GenericEmptyArrayEnumerator>.Instance : + GenericEmptyEnumerator>.Instance : new Enumerator(this); } }