From eb7ecd79a346cacb9cd15348553dcd59a7ddf326 Mon Sep 17 00:00:00 2001 From: Henrik Gedionsen Date: Thu, 2 Oct 2025 22:27:55 +0200 Subject: [PATCH 1/5] Array.FindAll improvements --- .../src/System/Array.cs | 45 ++++++++++++++++--- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.cs b/src/libraries/System.Private.CoreLib/src/System/Array.cs index 5ff7790fad3ae0..53a5e078692859 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Buffers; using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; @@ -1539,24 +1540,54 @@ public static void Fill(T[] array, T value, int startIndex, int count) public static T[] FindAll(T[] array, Predicate match) { if (array == null) - { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.array); - } if (match == null) - { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.match); - } - List list = new List(); + if (array.Length == 0) + return array; + + ArrayPool? sharedArrayPool = null; + T[]? matchArray = null; // only rent/allocate if needed + + nuint matchesFound = 0; for (int i = 0; i < array.Length; i++) { if (match(array[i])) { - list.Add(array[i]); + // Just assume the rest is matches, it is cheaper than to always allocate an empty List, + // that might need to grow the internal array & copy the values, and is expensive to do, both copy & allocations+GC. + // Large arrays are rented, cheaper than to grow a List multiple times & no GC, only release + matchArray ??= GetMatchArray(array.Length - i); + matchArray[matchesFound++] = array[i]; + } + } + + if (matchArray == null) + return EmptyArray.Value; + + // If all were matches, we could have returned the existing array, + // but the old code always made a new array, so it would be a breaking change + T[] result = new T[matchesFound]; + Buffer.Memmove(ref MemoryMarshal.GetArrayDataReference(result), ref MemoryMarshal.GetArrayDataReference(matchArray), matchesFound); + sharedArrayPool?.Return(matchArray); + + return result; + + T[] GetMatchArray(int size) + { + // This is about where execution time breaks even for: + // Windows 11 (10.0.26100.6584/24H2/2024Update/HudsonValley) + // .NET 10.0.0 (10.0.0-rc.1.25451.107, 10.0.25.45207), X64 RyuJIT x86-64-v3 + if (size >= 48) + { + sharedArrayPool = ArrayPool.Shared; + return sharedArrayPool.Rent(size); } + + return new T[size]; } - return list.ToArray(); } public static int FindIndex(T[] array, Predicate match) From 5cb2e20ae49db72bd8c58093897391a8c97e8466 Mon Sep 17 00:00:00 2001 From: Henrik Gedionsen Date: Fri, 3 Oct 2025 00:25:14 +0200 Subject: [PATCH 2/5] If all allocated are matched AND the match-array is not rented, we can just return the already allocated array --- src/libraries/System.Private.CoreLib/src/System/Array.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.cs b/src/libraries/System.Private.CoreLib/src/System/Array.cs index 53a5e078692859..9f0e7be8dfdf75 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.cs @@ -1551,7 +1551,7 @@ public static T[] FindAll(T[] array, Predicate match) ArrayPool? sharedArrayPool = null; T[]? matchArray = null; // only rent/allocate if needed - nuint matchesFound = 0; + uint matchesFound = 0; for (int i = 0; i < array.Length; i++) { if (match(array[i])) @@ -1567,6 +1567,9 @@ public static T[] FindAll(T[] array, Predicate match) if (matchArray == null) return EmptyArray.Value; + if (matchArray.Length == matchesFound && sharedArrayPool == null) + return matchArray; // If we have matched all allocated and not rented the array, we can just return it, brilliant suggestion from lechu445 + // If all were matches, we could have returned the existing array, // but the old code always made a new array, so it would be a breaking change T[] result = new T[matchesFound]; From c7e3678d0d99de2f0ef20f95f844b3505bac863e Mon Sep 17 00:00:00 2001 From: Henrik Gedionsen Date: Fri, 3 Oct 2025 10:37:34 +0200 Subject: [PATCH 3/5] Style changes, makes the method not fit within screen height. --- .../System.Private.CoreLib/src/System/Array.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.cs b/src/libraries/System.Private.CoreLib/src/System/Array.cs index 9f0e7be8dfdf75..f967ab3c3a6cae 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.cs @@ -1540,13 +1540,19 @@ public static void Fill(T[] array, T value, int startIndex, int count) public static T[] FindAll(T[] array, Predicate match) { if (array == null) + { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.array); + } if (match == null) + { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.match); + } if (array.Length == 0) + { return array; + } ArrayPool? sharedArrayPool = null; T[]? matchArray = null; // only rent/allocate if needed @@ -1565,10 +1571,14 @@ public static T[] FindAll(T[] array, Predicate match) } if (matchArray == null) + { return EmptyArray.Value; + } if (matchArray.Length == matchesFound && sharedArrayPool == null) + { return matchArray; // If we have matched all allocated and not rented the array, we can just return it, brilliant suggestion from lechu445 + } // If all were matches, we could have returned the existing array, // but the old code always made a new array, so it would be a breaking change From a0694b2e4a7367ce56dda82a2d196887d7ed4243 Mon Sep 17 00:00:00 2001 From: Henrik Gedionsen Date: Fri, 3 Oct 2025 15:58:37 +0200 Subject: [PATCH 4/5] Use a small amount of stack allocated matches before optionally allocating a list --- .../src/System/Array.cs | 70 +++++++------------ 1 file changed, 27 insertions(+), 43 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.cs b/src/libraries/System.Private.CoreLib/src/System/Array.cs index f967ab3c3a6cae..97d380df22dc07 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Buffers; using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; @@ -1540,67 +1539,52 @@ public static void Fill(T[] array, T value, int startIndex, int count) public static T[] FindAll(T[] array, Predicate match) { if (array == null) - { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.array); - } if (match == null) - { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.match); - } - - if (array.Length == 0) - { - return array; - } - - ArrayPool? sharedArrayPool = null; - T[]? matchArray = null; // only rent/allocate if needed - uint matchesFound = 0; + List? heapMatches = null; // only allocate if needed + InlineArray16 stackAllocatedMatches = default; + const int InlineArrayLength = 16; + int stackAllocatedMatchesFound = 0; for (int i = 0; i < array.Length; i++) { if (match(array[i])) { - // Just assume the rest is matches, it is cheaper than to always allocate an empty List, - // that might need to grow the internal array & copy the values, and is expensive to do, both copy & allocations+GC. - // Large arrays are rented, cheaper than to grow a List multiple times & no GC, only release - matchArray ??= GetMatchArray(array.Length - i); - matchArray[matchesFound++] = array[i]; + if (stackAllocatedMatchesFound < InlineArrayLength) + { + stackAllocatedMatches[stackAllocatedMatchesFound++] = array[i]; + } + else + { + // Revert to the old logic, allocating and growing a List + heapMatches ??= []; + heapMatches.Add(array[i]); + } } } - if (matchArray == null) - { + if (stackAllocatedMatchesFound == 0) return EmptyArray.Value; - } - if (matchArray.Length == matchesFound && sharedArrayPool == null) + int resultLength = stackAllocatedMatchesFound; + if (heapMatches != null) + resultLength += heapMatches.Count; + + T[] result = new T[resultLength]; + + int index = 0; + foreach (T stackAllocatedMatch in stackAllocatedMatches) { - return matchArray; // If we have matched all allocated and not rented the array, we can just return it, brilliant suggestion from lechu445 + result[index++] = stackAllocatedMatch; + if (index >= stackAllocatedMatchesFound) + break; } - // If all were matches, we could have returned the existing array, - // but the old code always made a new array, so it would be a breaking change - T[] result = new T[matchesFound]; - Buffer.Memmove(ref MemoryMarshal.GetArrayDataReference(result), ref MemoryMarshal.GetArrayDataReference(matchArray), matchesFound); - sharedArrayPool?.Return(matchArray); + heapMatches?.CopyTo(result.AsSpan(start: InlineArrayLength)); return result; - - T[] GetMatchArray(int size) - { - // This is about where execution time breaks even for: - // Windows 11 (10.0.26100.6584/24H2/2024Update/HudsonValley) - // .NET 10.0.0 (10.0.0-rc.1.25451.107, 10.0.25.45207), X64 RyuJIT x86-64-v3 - if (size >= 48) - { - sharedArrayPool = ArrayPool.Shared; - return sharedArrayPool.Rent(size); - } - - return new T[size]; - } } public static int FindIndex(T[] array, Predicate match) From f6269d696a571cd682c6af3031e0a7d5cc19c7ed Mon Sep 17 00:00:00 2001 From: Henrik Gedionsen Date: Fri, 3 Oct 2025 19:13:22 +0200 Subject: [PATCH 5/5] Style changes --- .../System.Private.CoreLib/src/System/Array.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.cs b/src/libraries/System.Private.CoreLib/src/System/Array.cs index 97d380df22dc07..15a6012bef33cd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.cs @@ -1539,10 +1539,14 @@ public static void Fill(T[] array, T value, int startIndex, int count) public static T[] FindAll(T[] array, Predicate match) { if (array == null) + { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.array); + } if (match == null) + { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.match); + } List? heapMatches = null; // only allocate if needed InlineArray16 stackAllocatedMatches = default; @@ -1566,11 +1570,15 @@ public static T[] FindAll(T[] array, Predicate match) } if (stackAllocatedMatchesFound == 0) + { return EmptyArray.Value; + } int resultLength = stackAllocatedMatchesFound; if (heapMatches != null) + { resultLength += heapMatches.Count; + } T[] result = new T[resultLength]; @@ -1579,7 +1587,9 @@ public static T[] FindAll(T[] array, Predicate match) { result[index++] = stackAllocatedMatch; if (index >= stackAllocatedMatchesFound) + { break; + } } heapMatches?.CopyTo(result.AsSpan(start: InlineArrayLength));