Add ArrayBuilder.AddRange and use it instead of List.ToArray pattern

[Copilot]

• Add AddRange method to ArrayBuilder<T> in /src/libraries/Common/src/System/Collections/Generic/ArrayBuilder.cs
• Refactor Directory.cs to use ArrayBuilder<T> with AddRange instead of new List<T>(enumerable).ToArray()
• Refactor DirectoryInfo.cs to use ArrayBuilder<T> with AddRange instead of new List<T>(enumerable).ToArray()
• Add tests for ArrayBuilder<T>.AddRange method
• Optimize AddRange to pre-allocate for ICollection<T> inputs and use ICollection.CopyTo for better performance
• Add AddRange method to NativeAOT's ArrayBuilder<T> in /src/coreclr/tools/Common/System/Collections/Generic/ArrayBuilder.cs
• Rename EnsureCapacity to Grow in Common ArrayBuilder for clarity (the method expects capacity checks before calling)
• Add tests specifically for ICollection<T> optimization:
  • AddRange_ICollection_PreallocatesCapacity - verifies capacity is pre-allocated for ICollection inputs
  • AddRange_ICollection_EmptyCollection - verifies empty ICollection works correctly
  • AddRange_ICollection_AfterExistingItems - verifies ICollection optimization works when items already exist
  • AddRange_NonICollection_GrowsIncrementally - verifies non-ICollection path grows incrementally
• Build and test to verify changes work correctly

Original prompt

There are a few locations in src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs src/System/IO/DirectoryInfo.cs, and src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs that have code of the pattern new List<concretetype>(enumerableData).ToArray(). They do this because they're in System.Private.Corelib, and don't have access to LINQ's ToArray method. However, these locations can be improved by using System.Collections.Generic.ArrayBuilder<T>, which is in corelib and can potentially avoid intermediate allocations. Refactor these locations to use ArrayBuilder<T>, and do it by adding an AddRange helper to ArrayBuilder to encapsulate the enumeration loop and keep callsites fairly simple. You don't need to search for any instances of this code pattern outside the files I've listed; they exist, but they're all tests and don't really need to be adjusted now.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Pull request overview

This PR adds an AddRange method to ArrayBuilder<T> to improve performance in locations that previously used the new List<T>(enumerable).ToArray() pattern. The implementation optimizes for ICollection<T> inputs by pre-allocating capacity, while falling back to incremental growth for general IEnumerable<T> inputs.

Key changes:

• Added AddRange(IEnumerable<T>) method to both Common and NativeAOT ArrayBuilder<T> implementations with ICollection<T> optimization
• Refactored I/O methods in Directory.cs, DirectoryInfo.cs, and Timer.cs to use ArrayBuilder<T>.AddRange() instead of List<T>.ToArray()
• Added comprehensive test coverage including tests for the ICollection optimization path, empty collections, and incremental growth

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/Common/src/System/Collections/Generic/ArrayBuilder.cs	Adds AddRange method with ICollection optimization for pre-allocating capacity
src/coreclr/tools/Common/System/Collections/Generic/ArrayBuilder.cs	Adds AddRange method to NativeAOT's ArrayBuilder with same optimization
src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs	Refactors three methods (GetFiles, GetDirectories, GetFileSystemEntries) to use ArrayBuilder.AddRange instead of List.ToArray pattern
src/libraries/System.Private.CoreLib/src/System/IO/DirectoryInfo.cs	Refactors three methods (GetFiles, GetFileSystemInfos, GetDirectories) to use ArrayBuilder.AddRange instead of List.ToArray pattern
src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs	Refactors debugger proxy property to use ArrayBuilder.AddRange instead of List.ToArray pattern
src/libraries/Common/tests/Tests/System/Collections/Generic/ArrayBuilderTests.cs	Adds comprehensive test coverage for AddRange including ICollection optimization, empty collections, and non-ICollection paths

[stephentoub]

This is in a debugger type proxy.... the extra allocations here don't matter.... better to keep the code simpler.

[333fred]

@copilot please make this change.

[Copilot]

Reverted Timer.cs to use the original List approach in 9491f52.

[stephentoub]

This is a deoptimization over using List, as it's copying element by element via enumeration rather than using ICollection.CopyTo.

[333fred]

@copilot please make this change.

[Copilot]

Changed to use ICollection.CopyTo instead of element-by-element enumeration in 9491f52. Both the Common and NativeAOT ArrayBuilder implementations now use CopyTo for better performance.

[stephentoub]

@333fred, all of the cited uses won't have an ICollection, as they all appear to be iterators. Are there other callers where this code will be used?

[333fred]

There aren't currently. However, if AddRange is an overload that is offered on ArrayBuilder, then it seems likely that at some point, someone will try to call it with an ICollection; while I didn't suggest that Copilot add this optimization, it seems like a reasonable thing to keep (assuming, of course, it uses CopyTo.

[jkotas]

The internal helper types like ArrayBuilder are implemented to be minimal. They are often not behaving like you would expect from a public BCL API and they tend to expose methods to squeeze bits of extra performance (e.g. ArrayBuilder.UncheckedAdd).

[333fred]

I'm happy to go either way here. Just tell whether you want me to keep the optimization around ICollection or not 🙂.

[stephentoub]

@333fred, I'm wondering if the additional complexity here is warranted. The possible allocation savings are:

• the list object itself
• the array backing the list oif it ends up being of exactly the right size

Presumably the latter rarely occurs, so this is primarily about avoiding the list object on code paths that are already allocating a bunch of FileSystemInfo objects and an array to store them all?

[333fred]

Presumably the latter rarely occurs, so this is primarily about avoiding the list object on code paths that are already allocating a bunch of FileSystemInfo objects and an array to store them all?

Right. Perfectly fine if we want to just keep the Lists here, but it seemed like a reasonable potential perf optimization.

[tannergooding]

The possible allocation savings are:

I'm not sure the allocation savings are particularly meaningful, except maybe in some more niche edge cases. I think the bigger thing is avoiding repeated copies by ensuring that the backing collection is big enough.

Allocations really aren't that expensive. The GC is also very good at its job, we have plenty of analyzers and tools to detect allocations when they do get meaningful, etc.

The copy cost, however, can be much more important as it can impact the rest of the program due to how it invalidates the cache and involves touching two or more times the memory than needed. I think this is more what makes this a meaningful thing to handle

The backing collection isn't likely to be the right size, so at least one copy is likely needed. But there's still all the intermediate growth copies that happen in the interim as we go 4->8->16->32... items. At 8x int, we're touching at least a full cache line of memory (and likely a couple due to alignment, allocations being in different areas, etc). Then we double that at each growth boundary.

[stephentoub]

The possible allocation savings are:

I'm not sure the allocation savings are particularly meaningful, except maybe in some more niche edge cases. I think the bigger thing is avoiding repeated copies by ensuring that the backing collection is big enough.

Allocations really aren't that expensive. The GC is also very good at its job, we have plenty of analyzers and tools to detect allocations when they do get meaningful, etc.

The copy cost, however, can be much more important as it can impact the rest of the program due to how it invalidates the cache and involves touching two or more times the memory than needed. I think this is more what makes this a meaningful thing to handle

How does this meaningfully help avoid copies? There's no difference here in presizing, and the only time a copy will be avoided is if the resulting array happens to be exactly the same size as what's required, which is unlikely.