Enumerable.ToArray performance improvement using InlineArray

[neuecc]

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

Author:	neuecc
Assignees:	-
Labels:	area-System.Collections
Milestone:	-

[stephentoub]

Thanks. What would it take to augment your PR to the point where we could delete LargeArrayBuilder? That'd make me much more interested in this.

[neuecc]

LargeArrayBuilder is used in many places, and given that it includes numerous methods specific to certain scenarios, the amount of rewrites, including from the user's side, would likely be extensive. Ideally, we'd like to replace it entirely, but we want to exclude that from the initial implementation.

Using the ArrayPool as suggested in the option might not be advisable if we replace the LargeArrayBuilder.

[stephentoub]

My concern is that these code paths are used in a bunch of places on common paths, and adding more generic types will increase code size. It'd be nice to get rid of the LargeArrayBuilder so that it's a net win for both throughput and size, rather than improving throughput at the expense of size.

There's no rush here as it's not going to make .NET 8, anyway, so there's plenty of time to make and evaluate the larger change.

[neuecc]

Thank you.
If this implementation is acceptable as a basic policy,
I'll take on the challenge of replacing the LargeArrayBuilder.
(Indeed, I was hoping it might make it into .NET 8! That would be great!)

[eiriktsarpalis]

If this implementation is acceptable as a basic policy,

We'd support it in principle. Would you like to build on top of this PR or would you prefer to open a new one?

[neuecc]

I am willing to work on this PR.
NET 8 release seems to be coming soon, so I will do it from the branch there when it is released. ......

[stephentoub]

main is the correct branch. Any such change will be for .NET 9.

[neuecc]

@dotnet-policy-service agree

[Windows10CE]

Your benchmarks show this being faster than ToList (almost) across the board, could it use the same (or similar) optimization?

[MichalPetryka]

AllocateUninitializedArray will introduce additional overhead for small arrays here.

[reflectronic]

The code for AllocateUninitializedArray already has a special case for small-ish arrays:

runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

Lines 756 to 762 in 616f758

	// small arrays are allocated using `new[]` as that is generally faster.
	#pragma warning disable 8500// sizeof of managed types
	if(length<2048/sizeof(T))
	#pragma warning restore 8500
	{
	returnnewT[length];
	}

Given that AllocateUninitializedArray is marked as AggressiveInlining, there's likely no discernable perf impact here (modulo tuning on the threshold).

[neuecc]

when length(_count + lastBlockCount) == 0, return Array.Empty<byte>().

[MichalPetryka]

Suggested change

	for(inti=0;i<_index;i++)
	{
	_blocks[i].CopyTo(dest);
	dest=dest.Slice(_blocks[i].Length);
	}
	ReadOnlySpan<T[]>blocks=_blocks;
	foreach(T[]blockinblocks.Slice(0,_index))
	{
	block.CopyTo(dest);
	dest=dest.Slice(block.Length);
	}

This should remove the bounds checks here too. Not sure whether this is the best syntax possible here.

[neuecc]

use ref var item = ref _blocks[i].

[reflectronic]

Something that'd be neat to add here (though the feasibility is questionable) is a special case for small arrays (say, fewer than 32 elements) using an InlineArray of T. Something like:

structArrayBuilder<T>{[InlineArray(32)]structElementsBlock{T_field;}[InlineArray(32)]structArraysBlock{T[]_field;}ElementsBlock_elements;ArraysBlock_arrays;// ...}

Inside the implementation of ArrayBuilder, you'd first fill up ElementsBlock before adding any new elements to ArraysBlock. This eliminates all intermediate allocations for small arrays.

The problem with this idea is that it'll chew through the stack for large structs. This could cause stack overflows when user-defined types come into the picture, even if the entire ElementsBlock is not filled.

The solutions are not pretty. You'd have to defer the stack allocation of ElementsBlock until you're sure that its size is reasonable. But, as far as I'm aware, the only way to reliably do this is with a non-inlineable method at every callsite of ArrayBuilder, plus a very unpleasant contortion of the callsite's actual logic. Doesn't seem very appealing.

I'm curious to hear if there's a way to salvage this idea.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

[neuecc]

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

Author:	neuecc
Assignees:	-
Labels:	area-System.Collections
Milestone:	-

[stephentoub]

Thanks. What would it take to augment your PR to the point where we could delete LargeArrayBuilder? That'd make me much more interested in this.

[neuecc]

LargeArrayBuilder is used in many places, and given that it includes numerous methods specific to certain scenarios, the amount of rewrites, including from the user's side, would likely be extensive. Ideally, we'd like to replace it entirely, but we want to exclude that from the initial implementation.

Using the ArrayPool as suggested in the option might not be advisable if we replace the LargeArrayBuilder.

[stephentoub]

My concern is that these code paths are used in a bunch of places on common paths, and adding more generic types will increase code size. It'd be nice to get rid of the LargeArrayBuilder so that it's a net win for both throughput and size, rather than improving throughput at the expense of size.

There's no rush here as it's not going to make .NET 8, anyway, so there's plenty of time to make and evaluate the larger change.

[neuecc]

Thank you.
If this implementation is acceptable as a basic policy,
I'll take on the challenge of replacing the LargeArrayBuilder.
(Indeed, I was hoping it might make it into .NET 8! That would be great!)

[eiriktsarpalis]

If this implementation is acceptable as a basic policy,

We'd support it in principle. Would you like to build on top of this PR or would you prefer to open a new one?

[neuecc]

I am willing to work on this PR.
NET 8 release seems to be coming soon, so I will do it from the branch there when it is released. ......

[stephentoub]

main is the correct branch. Any such change will be for .NET 9.

[neuecc]

@dotnet-policy-service agree

[Windows10CE]

Your benchmarks show this being faster than ToList (almost) across the board, could it use the same (or similar) optimization?

[MichalPetryka]

AllocateUninitializedArray will introduce additional overhead for small arrays here.

[reflectronic]

The code for AllocateUninitializedArray already has a special case for small-ish arrays:

runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

Lines 756 to 762 in 616f758

	// small arrays are allocated using `new[]` as that is generally faster.
	#pragma warning disable 8500// sizeof of managed types
	if(length<2048/sizeof(T))
	#pragma warning restore 8500
	{
	returnnewT[length];
	}

Given that AllocateUninitializedArray is marked as AggressiveInlining, there's likely no discernable perf impact here (modulo tuning on the threshold).

[neuecc]

when length(_count + lastBlockCount) == 0, return Array.Empty<byte>().

[MichalPetryka]

Suggested change

	for(inti=0;i<_index;i++)
	{
	_blocks[i].CopyTo(dest);
	dest=dest.Slice(_blocks[i].Length);
	}
	ReadOnlySpan<T[]>blocks=_blocks;
	foreach(T[]blockinblocks.Slice(0,_index))
	{
	block.CopyTo(dest);
	dest=dest.Slice(block.Length);
	}

This should remove the bounds checks here too. Not sure whether this is the best syntax possible here.

[neuecc]

use ref var item = ref _blocks[i].

[reflectronic]

Something that'd be neat to add here (though the feasibility is questionable) is a special case for small arrays (say, fewer than 32 elements) using an InlineArray of T. Something like:

structArrayBuilder<T>{[InlineArray(32)]structElementsBlock{T_field;}[InlineArray(32)]structArraysBlock{T[]_field;}ElementsBlock_elements;ArraysBlock_arrays;// ...}

Inside the implementation of ArrayBuilder, you'd first fill up ElementsBlock before adding any new elements to ArraysBlock. This eliminates all intermediate allocations for small arrays.

The problem with this idea is that it'll chew through the stack for large structs. This could cause stack overflows when user-defined types come into the picture, even if the entire ElementsBlock is not filled.

The solutions are not pretty. You'd have to defer the stack allocation of ElementsBlock until you're sure that its size is reasonable. But, as far as I'm aware, the only way to reliably do this is with a non-inlineable method at every callsite of ArrayBuilder, plus a very unpleasant contortion of the callsite's actual logic. Doesn't seem very appealing.

I'm curious to hear if there's a way to salvage this idea.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

[neuecc]

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

Author:	neuecc
Assignees:	-
Labels:	area-System.Collections
Milestone:	-

[stephentoub]

Thanks. What would it take to augment your PR to the point where we could delete LargeArrayBuilder? That'd make me much more interested in this.

[neuecc]

LargeArrayBuilder is used in many places, and given that it includes numerous methods specific to certain scenarios, the amount of rewrites, including from the user's side, would likely be extensive. Ideally, we'd like to replace it entirely, but we want to exclude that from the initial implementation.

Using the ArrayPool as suggested in the option might not be advisable if we replace the LargeArrayBuilder.

[stephentoub]

My concern is that these code paths are used in a bunch of places on common paths, and adding more generic types will increase code size. It'd be nice to get rid of the LargeArrayBuilder so that it's a net win for both throughput and size, rather than improving throughput at the expense of size.

There's no rush here as it's not going to make .NET 8, anyway, so there's plenty of time to make and evaluate the larger change.

[neuecc]

Thank you.
If this implementation is acceptable as a basic policy,
I'll take on the challenge of replacing the LargeArrayBuilder.
(Indeed, I was hoping it might make it into .NET 8! That would be great!)

[eiriktsarpalis]

If this implementation is acceptable as a basic policy,

We'd support it in principle. Would you like to build on top of this PR or would you prefer to open a new one?

[neuecc]

I am willing to work on this PR.
NET 8 release seems to be coming soon, so I will do it from the branch there when it is released. ......

[stephentoub]

main is the correct branch. Any such change will be for .NET 9.

[neuecc]

@dotnet-policy-service agree

[Windows10CE]

Your benchmarks show this being faster than ToList (almost) across the board, could it use the same (or similar) optimization?

[MichalPetryka]

AllocateUninitializedArray will introduce additional overhead for small arrays here.

[reflectronic]

The code for AllocateUninitializedArray already has a special case for small-ish arrays:

runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

Lines 756 to 762 in 616f758

	// small arrays are allocated using `new[]` as that is generally faster.
	#pragma warning disable 8500// sizeof of managed types
	if(length<2048/sizeof(T))
	#pragma warning restore 8500
	{
	returnnewT[length];
	}

Given that AllocateUninitializedArray is marked as AggressiveInlining, there's likely no discernable perf impact here (modulo tuning on the threshold).

[neuecc]

when length(_count + lastBlockCount) == 0, return Array.Empty<byte>().

[MichalPetryka]

Suggested change

	for(inti=0;i<_index;i++)
	{
	_blocks[i].CopyTo(dest);
	dest=dest.Slice(_blocks[i].Length);
	}
	ReadOnlySpan<T[]>blocks=_blocks;
	foreach(T[]blockinblocks.Slice(0,_index))
	{
	block.CopyTo(dest);
	dest=dest.Slice(block.Length);
	}

This should remove the bounds checks here too. Not sure whether this is the best syntax possible here.

[neuecc]

use ref var item = ref _blocks[i].

[reflectronic]

Something that'd be neat to add here (though the feasibility is questionable) is a special case for small arrays (say, fewer than 32 elements) using an InlineArray of T. Something like:

structArrayBuilder<T>{[InlineArray(32)]structElementsBlock{T_field;}[InlineArray(32)]structArraysBlock{T[]_field;}ElementsBlock_elements;ArraysBlock_arrays;// ...}

Inside the implementation of ArrayBuilder, you'd first fill up ElementsBlock before adding any new elements to ArraysBlock. This eliminates all intermediate allocations for small arrays.

The problem with this idea is that it'll chew through the stack for large structs. This could cause stack overflows when user-defined types come into the picture, even if the entire ElementsBlock is not filled.

The solutions are not pretty. You'd have to defer the stack allocation of ElementsBlock until you're sure that its size is reasonable. But, as far as I'm aware, the only way to reliably do this is with a non-inlineable method at every callsite of ArrayBuilder, plus a very unpleasant contortion of the callsite's actual logic. Doesn't seem very appealing.

I'm curious to hear if there's a way to salvage this idea.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

[neuecc]

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

Author:	neuecc
Assignees:	-
Labels:	area-System.Collections
Milestone:	-

[stephentoub]

Thanks. What would it take to augment your PR to the point where we could delete LargeArrayBuilder? That'd make me much more interested in this.

[neuecc]

LargeArrayBuilder is used in many places, and given that it includes numerous methods specific to certain scenarios, the amount of rewrites, including from the user's side, would likely be extensive. Ideally, we'd like to replace it entirely, but we want to exclude that from the initial implementation.

Using the ArrayPool as suggested in the option might not be advisable if we replace the LargeArrayBuilder.

[stephentoub]

My concern is that these code paths are used in a bunch of places on common paths, and adding more generic types will increase code size. It'd be nice to get rid of the LargeArrayBuilder so that it's a net win for both throughput and size, rather than improving throughput at the expense of size.

There's no rush here as it's not going to make .NET 8, anyway, so there's plenty of time to make and evaluate the larger change.

[neuecc]

Thank you.
If this implementation is acceptable as a basic policy,
I'll take on the challenge of replacing the LargeArrayBuilder.
(Indeed, I was hoping it might make it into .NET 8! That would be great!)

[eiriktsarpalis]

If this implementation is acceptable as a basic policy,

We'd support it in principle. Would you like to build on top of this PR or would you prefer to open a new one?

[neuecc]

I am willing to work on this PR.
NET 8 release seems to be coming soon, so I will do it from the branch there when it is released. ......

[stephentoub]

main is the correct branch. Any such change will be for .NET 9.

[neuecc]

@dotnet-policy-service agree

[Windows10CE]

Your benchmarks show this being faster than ToList (almost) across the board, could it use the same (or similar) optimization?

[MichalPetryka]

AllocateUninitializedArray will introduce additional overhead for small arrays here.

[reflectronic]

The code for AllocateUninitializedArray already has a special case for small-ish arrays:

runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

Lines 756 to 762 in 616f758

	// small arrays are allocated using `new[]` as that is generally faster.
	#pragma warning disable 8500// sizeof of managed types
	if(length<2048/sizeof(T))
	#pragma warning restore 8500
	{
	returnnewT[length];
	}

Given that AllocateUninitializedArray is marked as AggressiveInlining, there's likely no discernable perf impact here (modulo tuning on the threshold).

[neuecc]

when length(_count + lastBlockCount) == 0, return Array.Empty<byte>().

[MichalPetryka]

Suggested change

	for(inti=0;i<_index;i++)
	{
	_blocks[i].CopyTo(dest);
	dest=dest.Slice(_blocks[i].Length);
	}
	ReadOnlySpan<T[]>blocks=_blocks;
	foreach(T[]blockinblocks.Slice(0,_index))
	{
	block.CopyTo(dest);
	dest=dest.Slice(block.Length);
	}

This should remove the bounds checks here too. Not sure whether this is the best syntax possible here.

[neuecc]

use ref var item = ref _blocks[i].

[reflectronic]

Something that'd be neat to add here (though the feasibility is questionable) is a special case for small arrays (say, fewer than 32 elements) using an InlineArray of T. Something like:

structArrayBuilder<T>{[InlineArray(32)]structElementsBlock{T_field;}[InlineArray(32)]structArraysBlock{T[]_field;}ElementsBlock_elements;ArraysBlock_arrays;// ...}

Inside the implementation of ArrayBuilder, you'd first fill up ElementsBlock before adding any new elements to ArraysBlock. This eliminates all intermediate allocations for small arrays.

The problem with this idea is that it'll chew through the stack for large structs. This could cause stack overflows when user-defined types come into the picture, even if the entire ElementsBlock is not filled.

The solutions are not pretty. You'd have to defer the stack allocation of ElementsBlock until you're sure that its size is reasonable. But, as far as I'm aware, the only way to reliably do this is with a non-inlineable method at every callsite of ArrayBuilder, plus a very unpleasant contortion of the callsite's actual logic. Doesn't seem very appealing.

I'm curious to hear if there's a way to salvage this idea.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

[neuecc]

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

Author:	neuecc
Assignees:	-
Labels:	area-System.Collections
Milestone:	-

[stephentoub]

Thanks. What would it take to augment your PR to the point where we could delete LargeArrayBuilder? That'd make me much more interested in this.

[neuecc]

LargeArrayBuilder is used in many places, and given that it includes numerous methods specific to certain scenarios, the amount of rewrites, including from the user's side, would likely be extensive. Ideally, we'd like to replace it entirely, but we want to exclude that from the initial implementation.

Using the ArrayPool as suggested in the option might not be advisable if we replace the LargeArrayBuilder.

[stephentoub]

My concern is that these code paths are used in a bunch of places on common paths, and adding more generic types will increase code size. It'd be nice to get rid of the LargeArrayBuilder so that it's a net win for both throughput and size, rather than improving throughput at the expense of size.

There's no rush here as it's not going to make .NET 8, anyway, so there's plenty of time to make and evaluate the larger change.

[neuecc]

Thank you.
If this implementation is acceptable as a basic policy,
I'll take on the challenge of replacing the LargeArrayBuilder.
(Indeed, I was hoping it might make it into .NET 8! That would be great!)

[eiriktsarpalis]

If this implementation is acceptable as a basic policy,

We'd support it in principle. Would you like to build on top of this PR or would you prefer to open a new one?

[neuecc]

I am willing to work on this PR.
NET 8 release seems to be coming soon, so I will do it from the branch there when it is released. ......

[stephentoub]

main is the correct branch. Any such change will be for .NET 9.

[neuecc]

@dotnet-policy-service agree

[Windows10CE]

Your benchmarks show this being faster than ToList (almost) across the board, could it use the same (or similar) optimization?

[MichalPetryka]

AllocateUninitializedArray will introduce additional overhead for small arrays here.

[reflectronic]

The code for AllocateUninitializedArray already has a special case for small-ish arrays:

runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

Lines 756 to 762 in 616f758

	// small arrays are allocated using `new[]` as that is generally faster.
	#pragma warning disable 8500// sizeof of managed types
	if(length<2048/sizeof(T))
	#pragma warning restore 8500
	{
	returnnewT[length];
	}

Given that AllocateUninitializedArray is marked as AggressiveInlining, there's likely no discernable perf impact here (modulo tuning on the threshold).

[neuecc]

when length(_count + lastBlockCount) == 0, return Array.Empty<byte>().

[MichalPetryka]

Suggested change

	for(inti=0;i<_index;i++)
	{
	_blocks[i].CopyTo(dest);
	dest=dest.Slice(_blocks[i].Length);
	}
	ReadOnlySpan<T[]>blocks=_blocks;
	foreach(T[]blockinblocks.Slice(0,_index))
	{
	block.CopyTo(dest);
	dest=dest.Slice(block.Length);
	}

This should remove the bounds checks here too. Not sure whether this is the best syntax possible here.

[neuecc]

use ref var item = ref _blocks[i].

[reflectronic]

Something that'd be neat to add here (though the feasibility is questionable) is a special case for small arrays (say, fewer than 32 elements) using an InlineArray of T. Something like:

structArrayBuilder<T>{[InlineArray(32)]structElementsBlock{T_field;}[InlineArray(32)]structArraysBlock{T[]_field;}ElementsBlock_elements;ArraysBlock_arrays;// ...}

Inside the implementation of ArrayBuilder, you'd first fill up ElementsBlock before adding any new elements to ArraysBlock. This eliminates all intermediate allocations for small arrays.

The problem with this idea is that it'll chew through the stack for large structs. This could cause stack overflows when user-defined types come into the picture, even if the entire ElementsBlock is not filled.

The solutions are not pretty. You'd have to defer the stack allocation of ElementsBlock until you're sure that its size is reasonable. But, as far as I'm aware, the only way to reliably do this is with a non-inlineable method at every callsite of ArrayBuilder, plus a very unpleasant contortion of the callsite's actual logic. Doesn't seem very appealing.

I'm curious to hear if there's a way to salvage this idea.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

[neuecc]

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

Author:	neuecc
Assignees:	-
Labels:	area-System.Collections
Milestone:	-

[stephentoub]

Thanks. What would it take to augment your PR to the point where we could delete LargeArrayBuilder? That'd make me much more interested in this.

[neuecc]

LargeArrayBuilder is used in many places, and given that it includes numerous methods specific to certain scenarios, the amount of rewrites, including from the user's side, would likely be extensive. Ideally, we'd like to replace it entirely, but we want to exclude that from the initial implementation.

Using the ArrayPool as suggested in the option might not be advisable if we replace the LargeArrayBuilder.

[stephentoub]

My concern is that these code paths are used in a bunch of places on common paths, and adding more generic types will increase code size. It'd be nice to get rid of the LargeArrayBuilder so that it's a net win for both throughput and size, rather than improving throughput at the expense of size.

There's no rush here as it's not going to make .NET 8, anyway, so there's plenty of time to make and evaluate the larger change.

[neuecc]

Thank you.
If this implementation is acceptable as a basic policy,
I'll take on the challenge of replacing the LargeArrayBuilder.
(Indeed, I was hoping it might make it into .NET 8! That would be great!)

[eiriktsarpalis]

If this implementation is acceptable as a basic policy,

We'd support it in principle. Would you like to build on top of this PR or would you prefer to open a new one?

[neuecc]

I am willing to work on this PR.
NET 8 release seems to be coming soon, so I will do it from the branch there when it is released. ......

[stephentoub]

main is the correct branch. Any such change will be for .NET 9.

[neuecc]

@dotnet-policy-service agree

[Windows10CE]

Your benchmarks show this being faster than ToList (almost) across the board, could it use the same (or similar) optimization?

[MichalPetryka]

AllocateUninitializedArray will introduce additional overhead for small arrays here.

[reflectronic]

The code for AllocateUninitializedArray already has a special case for small-ish arrays:

runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

Lines 756 to 762 in 616f758

	// small arrays are allocated using `new[]` as that is generally faster.
	#pragma warning disable 8500// sizeof of managed types
	if(length<2048/sizeof(T))
	#pragma warning restore 8500
	{
	returnnewT[length];
	}

Given that AllocateUninitializedArray is marked as AggressiveInlining, there's likely no discernable perf impact here (modulo tuning on the threshold).

[neuecc]

when length(_count + lastBlockCount) == 0, return Array.Empty<byte>().

[MichalPetryka]

Suggested change

	for(inti=0;i<_index;i++)
	{
	_blocks[i].CopyTo(dest);
	dest=dest.Slice(_blocks[i].Length);
	}
	ReadOnlySpan<T[]>blocks=_blocks;
	foreach(T[]blockinblocks.Slice(0,_index))
	{
	block.CopyTo(dest);
	dest=dest.Slice(block.Length);
	}

This should remove the bounds checks here too. Not sure whether this is the best syntax possible here.

[neuecc]

use ref var item = ref _blocks[i].

[reflectronic]

Something that'd be neat to add here (though the feasibility is questionable) is a special case for small arrays (say, fewer than 32 elements) using an InlineArray of T. Something like:

structArrayBuilder<T>{[InlineArray(32)]structElementsBlock{T_field;}[InlineArray(32)]structArraysBlock{T[]_field;}ElementsBlock_elements;ArraysBlock_arrays;// ...}

Inside the implementation of ArrayBuilder, you'd first fill up ElementsBlock before adding any new elements to ArraysBlock. This eliminates all intermediate allocations for small arrays.

The problem with this idea is that it'll chew through the stack for large structs. This could cause stack overflows when user-defined types come into the picture, even if the entire ElementsBlock is not filled.

The solutions are not pretty. You'd have to defer the stack allocation of ElementsBlock until you're sure that its size is reasonable. But, as far as I'm aware, the only way to reliably do this is with a non-inlineable method at every callsite of ArrayBuilder, plus a very unpleasant contortion of the callsite's actual logic. Doesn't seem very appealing.

I'm curious to hear if there's a way to salvage this idea.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

[neuecc]

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

Author:	neuecc
Assignees:	-
Labels:	area-System.Collections
Milestone:	-

[stephentoub]

Thanks. What would it take to augment your PR to the point where we could delete LargeArrayBuilder? That'd make me much more interested in this.

[neuecc]

LargeArrayBuilder is used in many places, and given that it includes numerous methods specific to certain scenarios, the amount of rewrites, including from the user's side, would likely be extensive. Ideally, we'd like to replace it entirely, but we want to exclude that from the initial implementation.

Using the ArrayPool as suggested in the option might not be advisable if we replace the LargeArrayBuilder.

[stephentoub]

My concern is that these code paths are used in a bunch of places on common paths, and adding more generic types will increase code size. It'd be nice to get rid of the LargeArrayBuilder so that it's a net win for both throughput and size, rather than improving throughput at the expense of size.

There's no rush here as it's not going to make .NET 8, anyway, so there's plenty of time to make and evaluate the larger change.

[neuecc]

Thank you.
If this implementation is acceptable as a basic policy,
I'll take on the challenge of replacing the LargeArrayBuilder.
(Indeed, I was hoping it might make it into .NET 8! That would be great!)

[eiriktsarpalis]

If this implementation is acceptable as a basic policy,

We'd support it in principle. Would you like to build on top of this PR or would you prefer to open a new one?

[neuecc]

I am willing to work on this PR.
NET 8 release seems to be coming soon, so I will do it from the branch there when it is released. ......

[stephentoub]

main is the correct branch. Any such change will be for .NET 9.

[neuecc]

@dotnet-policy-service agree

[Windows10CE]

Your benchmarks show this being faster than ToList (almost) across the board, could it use the same (or similar) optimization?

[MichalPetryka]

AllocateUninitializedArray will introduce additional overhead for small arrays here.

[reflectronic]

The code for AllocateUninitializedArray already has a special case for small-ish arrays:

runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

Lines 756 to 762 in 616f758

	// small arrays are allocated using `new[]` as that is generally faster.
	#pragma warning disable 8500// sizeof of managed types
	if(length<2048/sizeof(T))
	#pragma warning restore 8500
	{
	returnnewT[length];
	}

Given that AllocateUninitializedArray is marked as AggressiveInlining, there's likely no discernable perf impact here (modulo tuning on the threshold).

[neuecc]

when length(_count + lastBlockCount) == 0, return Array.Empty<byte>().

[MichalPetryka]

Suggested change

	for(inti=0;i<_index;i++)
	{
	_blocks[i].CopyTo(dest);
	dest=dest.Slice(_blocks[i].Length);
	}
	ReadOnlySpan<T[]>blocks=_blocks;
	foreach(T[]blockinblocks.Slice(0,_index))
	{
	block.CopyTo(dest);
	dest=dest.Slice(block.Length);
	}

This should remove the bounds checks here too. Not sure whether this is the best syntax possible here.

[neuecc]

use ref var item = ref _blocks[i].

[reflectronic]

Something that'd be neat to add here (though the feasibility is questionable) is a special case for small arrays (say, fewer than 32 elements) using an InlineArray of T. Something like:

structArrayBuilder<T>{[InlineArray(32)]structElementsBlock{T_field;}[InlineArray(32)]structArraysBlock{T[]_field;}ElementsBlock_elements;ArraysBlock_arrays;// ...}

Inside the implementation of ArrayBuilder, you'd first fill up ElementsBlock before adding any new elements to ArraysBlock. This eliminates all intermediate allocations for small arrays.

The problem with this idea is that it'll chew through the stack for large structs. This could cause stack overflows when user-defined types come into the picture, even if the entire ElementsBlock is not filled.

The solutions are not pretty. You'd have to defer the stack allocation of ElementsBlock until you're sure that its size is reasonable. But, as far as I'm aware, the only way to reliably do this is with a non-inlineable method at every callsite of ArrayBuilder, plus a very unpleasant contortion of the callsite's actual logic. Doesn't seem very appealing.

I'm curious to hear if there's a way to salvage this idea.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

[neuecc]

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I've optimized the Enumerable.ToArray method for the case when the source is a pure IEnumerable<T>.
Typically, when the buffer overflows, an array with double the previous capacity is created, and the copying process repeats.
In this PR, although we still create an array of double the size, instead of copying immediately, we add the array to a list and perform all the copying at the end.
By reducing the number of copy operations, we see a significant performance improvement.

Benchmark

BenchmarkDotNet v0.13.7, Windows 10 (10.0.19045.3324/22H2/2022Update)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.100-preview.7.23376.3
[Host] : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
Job-NGJHUQ : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2
IterationCount=1 WarmupCount=1 

Method	Length	Mean	Error	Gen0	Gen1	Gen2	Allocated
ToList	10	53.12 ns	NA	0.0153	-	-	256 B
ToArray	10	71.83 ns	NA	0.0153	-	-	256 B
PR_ToArray	10	38.40 ns	NA	0.0119	-	-	200 B
ToList	100	220.14 ns	NA	0.0730	-	-	1224 B
ToArray	100	326.60 ns	NA	0.0710	-	-	1192 B
PR_ToArray	100	180.50 ns	NA	0.0644	-	-	1080 B
ToList	1000	1,679.32 ns	NA	0.5054	0.0057	-	8464 B
ToArray	1000	2,081.20 ns	NA	0.5074	-	-	8536 B
PR_ToArray	1000	1,384.58 ns	NA	0.4978	-	-	8336 B
ToList	10000	17,214.66 ns	NA	7.8430	1.2817	-	131440 B
ToArray	10000	21,014.18 ns	NA	6.3171	-	-	106224 B
PR_ToArray	10000	13,196.72 ns	NA	6.3171	-	-	105872 B
ToList	100000	338,986.18 ns	NA	285.6445	285.6445	285.6445	1049112 B
ToArray	100000	382,722.36 ns	NA	249.5117	249.5117	249.5117	925132 B
PR_ToArray	100000	358,964.21 ns	NA	249.5117	249.5117	249.5117	924780 B
ToList	1000000	4,533,538.28 ns	NA	1984.3750	1984.3750	1984.3750	8389748 B
ToArray	1000000	3,067,601.56 ns	NA	796.8750	796.8750	796.8750	8195588 B
PR_ToArray	1000000	2,418,546.88 ns	NA	800.7813	800.7813	800.7813	8195040 B
ToList	10000000	197,990,525.00 ns	NA	3875.0000	3875.0000	3875.0000	134219664 B
ToArray	10000000	229,693,275.00 ns	NA	1875.0000	1875.0000	1875.0000	107110743 B
PR_ToArray	10000000	29,184,353.12 ns	NA	1968.7500	1968.7500	1968.7500	107110478 B
ToList	100000000	583,235,200.00 ns	NA	6000.0000	6000.0000	6000.0000	1073744896 B
ToArray	100000000	501,066,800.00 ns	NA	3000.0000	3000.0000	3000.0000	936873600 B
PR_ToArray	100000000	270,801,200.00 ns	NA	2000.0000	2000.0000	2000.0000	936872432 B
ToList	1000000000	4,677,241,700.00 ns	NA	9000.0000	9000.0000	9000.0000	8589938744 B
ToArray	1000000000	5,229,577,600.00 ns	NA	4000.0000	4000.0000	4000.0000	8294970392 B
PR_ToArray	1000000000	3,136,817,900.00 ns	NA	4000.0000	4000.0000	4000.0000	8294969760 B

publicclassToArrayBenchmark{[Params(10,100,1000,10000,100000,1000000,10000000,100000000,1000000000)]publicintLength{get;set;}[Benchmark]publicList<int>ToList(){returnGenerateNumber(Length).ToList();}[Benchmark]publicint[]ToArray(){returnGenerateNumber(Length).ToArray();}[Benchmark]publicint[]PR_ToArray(){returnEnumerableHelpers.ToArray2(GenerateNumber(Length));}publicstaticIEnumerable<int>GenerateNumber(intcount){for(inti=0;i<count;i++){yieldreturni;}}}

Implementation detail

In this scenario, there's a fixed maximum length for the list of arrays that should be allocated.
Starting with 4 and doubling the array size each time, we reach the maximum length after 29 arrays.
Therefore, using [InlineArray(29)], I've reserved a fixed-length space for the T[].

Option

It's also possible to use ArrayPool<T>.Shared.Rent instead of new T[].
In that case, by returning the array when calling ToArray, both copying and returning can be efficiently handled.
I wasn't sure if using ArrayPool was appropriate for this scenario, so in this PR, I've opted for creating new arrays.

Author:	neuecc
Assignees:	-
Labels:	area-System.Collections
Milestone:	-

[stephentoub]

Thanks. What would it take to augment your PR to the point where we could delete LargeArrayBuilder? That'd make me much more interested in this.

[neuecc]

LargeArrayBuilder is used in many places, and given that it includes numerous methods specific to certain scenarios, the amount of rewrites, including from the user's side, would likely be extensive. Ideally, we'd like to replace it entirely, but we want to exclude that from the initial implementation.

Using the ArrayPool as suggested in the option might not be advisable if we replace the LargeArrayBuilder.

[stephentoub]

My concern is that these code paths are used in a bunch of places on common paths, and adding more generic types will increase code size. It'd be nice to get rid of the LargeArrayBuilder so that it's a net win for both throughput and size, rather than improving throughput at the expense of size.

There's no rush here as it's not going to make .NET 8, anyway, so there's plenty of time to make and evaluate the larger change.

[neuecc]

Thank you.
If this implementation is acceptable as a basic policy,
I'll take on the challenge of replacing the LargeArrayBuilder.
(Indeed, I was hoping it might make it into .NET 8! That would be great!)

[eiriktsarpalis]

If this implementation is acceptable as a basic policy,

We'd support it in principle. Would you like to build on top of this PR or would you prefer to open a new one?

[neuecc]

I am willing to work on this PR.
NET 8 release seems to be coming soon, so I will do it from the branch there when it is released. ......

[stephentoub]

main is the correct branch. Any such change will be for .NET 9.

[neuecc]

@dotnet-policy-service agree

[Windows10CE]

Your benchmarks show this being faster than ToList (almost) across the board, could it use the same (or similar) optimization?

[MichalPetryka]

AllocateUninitializedArray will introduce additional overhead for small arrays here.

[reflectronic]

The code for AllocateUninitializedArray already has a special case for small-ish arrays:

runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

Lines 756 to 762 in 616f758

	// small arrays are allocated using `new[]` as that is generally faster.
	#pragma warning disable 8500// sizeof of managed types
	if(length<2048/sizeof(T))
	#pragma warning restore 8500
	{
	returnnewT[length];
	}

Given that AllocateUninitializedArray is marked as AggressiveInlining, there's likely no discernable perf impact here (modulo tuning on the threshold).

[neuecc]

when length(_count + lastBlockCount) == 0, return Array.Empty<byte>().

[MichalPetryka]

Suggested change

	for(inti=0;i<_index;i++)
	{
	_blocks[i].CopyTo(dest);
	dest=dest.Slice(_blocks[i].Length);
	}
	ReadOnlySpan<T[]>blocks=_blocks;
	foreach(T[]blockinblocks.Slice(0,_index))
	{
	block.CopyTo(dest);
	dest=dest.Slice(block.Length);
	}

This should remove the bounds checks here too. Not sure whether this is the best syntax possible here.

[neuecc]

use ref var item = ref _blocks[i].

[reflectronic]

Something that'd be neat to add here (though the feasibility is questionable) is a special case for small arrays (say, fewer than 32 elements) using an InlineArray of T. Something like:

structArrayBuilder<T>{[InlineArray(32)]structElementsBlock{T_field;}[InlineArray(32)]structArraysBlock{T[]_field;}ElementsBlock_elements;ArraysBlock_arrays;// ...}

Inside the implementation of ArrayBuilder, you'd first fill up ElementsBlock before adding any new elements to ArraysBlock. This eliminates all intermediate allocations for small arrays.

The problem with this idea is that it'll chew through the stack for large structs. This could cause stack overflows when user-defined types come into the picture, even if the entire ElementsBlock is not filled.

The solutions are not pretty. You'd have to defer the stack allocation of ElementsBlock until you're sure that its size is reasonable. But, as far as I'm aware, the only way to reliably do this is with a non-inlineable method at every callsite of ArrayBuilder, plus a very unpleasant contortion of the callsite's actual logic. Doesn't seem very appealing.

I'm curious to hear if there's a way to salvage this idea.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.