Slightly Faster version of Chunk

[TonyValenti]

@inputfalken -
I've written a version of Chunk that is slightly faster. Can you review and commit? I ran this a bunch of times and every time Mine2 is about 20μs faster.

        private static IEnumerable<TSource[]> ChunkIterator<TSource>(IEnumerable<TSource> source, int size) {
            using IEnumerator<TSource> e = source.GetEnumerator();

            while (true) {
                var chunk = new TSource[size];

                int count = 0;
                while (count < size && e.MoveNext()) {
                    chunk[count] = e.Current;
                    count++;
                }

                if (count > 0) {
                    Array.Resize(ref chunk, count);
                    yield return chunk;
                } else {
                    yield break;
                }
            }

        }

[MemoryDiagnoser]
    [DisassemblyDiagnoser(2)]
    public class Benchmarks {

        [Benchmark]
        public void Theirs() {
            Enumerable.Range(0, 100_005).Chunk(100).ToList();
        }

        [Benchmark]
        public void Mine2() {
            Enumerable.Range(0, 100_005).Chunk2(100).ToList();
        }
    }

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated	Code Size
Theirs	396.0 μs	1.96 μs	1.74 μs	52.7344	26.3672	-	431 KB	1 KB
Mine2	377.9 μs	1.41 μs	1.18 μs	52.7344	26.3672	-	431 KB	1 KB

[TonyValenti]

Also, is it possible to get an async version of of Chunk?

[stephentoub]

We're not adding async LINQ APIs in dotnet/runtime. The https://github.com/dotnet/reactive/tree/main/Ix.NET/Source/System.Interactive.Async does so, and already exposes the same basic operation, but called Buffer:
https://github.com/dotnet/reactive/blob/d3392b70d3ea6e8fff66d5722af8beae72e0aed5/Ix.NET/Source/System.Interactive.Async/System/Linq/Operators/Buffer.cs#L13-L22

[TonyValenti]

One other thought - if an async version named "buffer" already exists, perhaps this method could be renamed Buffer to keep parity between method names?

[stephentoub]

That was discussed and rejected in the API review.

[inputfalken]

Hi, I don't have the right to apply changes. I would suggest you have a look at Contribution to .NET Runtime.

[stephentoub]

Thanks for the suggestion. We don't want to do exactly what you proposed... in particular, it severely penalizes the empty enumerable case, which with LINQ in general is fairly common, as it ends up allocating a TSource[size] unnecessarily. Similarly, it unnecessarily ends up allocating an extra TSource[size] if the enumerable isn't an exact multiple of size. That said, the core thing the above has is moving the yielding and passing-array-by-ref out of the tight loop. I've incorporated that in #54782.

[stephentoub]

No, the size check isn't there to avoid the Resize call, it's there to end the iteration. We then only do the resize when the iteration is ending, which will only be when the resize is definitely required.

[TonyValenti]

Oh ya. Doh! You're right. Sorry about that. :-)

[cstruct]

Using the optimization from the original PR would also be able to speed this up even more in the case of a backing IList<T> which I believe would be very common in the real world. My question is where is the line drawn of what is a worthwhile optimization?

[TonyValenti]

@cstruct @stephentoub - How much optimization is too much optimization? That's a good question and I'm not sure I know the answer, especially since an improvement in one regard tends to be a loss in another regard. A common example seems to be with Enumerables and how much work people do to try to squeeze as much performance out of it as they can using methods like TryGetEnumeratedCount, checking for IList<> and such. While that gives better performance, I feel like it makes the code less maintainable and more complex.

I personally think the best solution to that problem would be some combination of the following:

1. Allow C# to do multi-level type inferencing so you can write a signature like this:

public static IEnumerable<TItem[]> Chunk<TCollection, TItem>(this TCollection This, int Count)
            where TCollection : IEnumerable<TItem>
        {
            //...
        }

And use it like this:

var Data = ???.Chunk(10);

2. Then teach the CLR about enumerables so that when it goes to generate the code, it will automatically check for IList and if detected, generate an IList specific version of the method that enumerates differently. That would get the maximum performance benefit and keep performance special-casing out of user code.

[stephentoub]

This is not a high performance API. Anyone who cares about really getting maximum throughput shouldn't be using a method that allocates a new array for each chunk; you'd instead craft something specific to your input type, reusing whatever buffers you can for each chunk, etc. Given that, it's fine to make tweaks to reduce overheads here and there, but we're not going to invest a lot of additional code adding special-cases for this or that input data type.