Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Collection expressions use AddRange instead of manual foreach in presence of a struct enumerator #74615

Closed
DoctorKrolic opened this issue Jul 31, 2024 · 3 comments · Fixed by #74630
Labels
Area-Compilers Code Gen Quality Room for improvement in the quality of the compiler's generated code Feature - Collection Expressions untriaged Issues and PRs which have not yet been triaged by a lead
Milestone

Comments

@DoctorKrolic
Copy link
Contributor

Version Used:
Noticed while working on other PR, so latest main

Steps to Reproduce:

using System.Collections;
using System.Collections.Generic;

class MyCollection(List<int> list) : IEnumerable<int>
{
    public Enumerator GetEnumerator() => new(list.GetEnumerator());

    IEnumerator<int> IEnumerable<int>.GetEnumerator() => GetEnumerator();

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

    public struct Enumerator(List<int>.Enumerator enumerator) : IEnumerator<int>
    {
        public int Current => enumerator.Current;

        object IEnumerator.Current => Current;

        public bool MoveNext() => enumerator.MoveNext();

        public void Dispose() => enumerator.Dispose();

        public void Reset() { }
    }
}

class C
{
    static List<int> M(MyCollection c) => [..c];
}

Expected Behavior:
Codegen of C.M is a manual foreach due to the fact, that MyCollection has a struct enumerator and doesn't implement ICollection<T>

Actual Behavior:
Codegen of C.M is:

private static List<int> M(MyCollection c)
{
    List<int> list = new List<int>();
    list.AddRange(c);
    return list;
}

sharplab

Benchmark results:

Benchmark code
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Collections;

BenchmarkRunner.Run<Benchmarks>();

[MemoryDiagnoser]
public class Benchmarks
{
    private MyCollection _collection;

    [Params(0, 10, 100, 1000)]
    public int CollectionSize { get; set; }

    [GlobalSetup]
    public void Setup()
    {
        _collection = new(Enumerable.Range(0, CollectionSize).ToList());
    }

    [Benchmark(Baseline = true)]
    public List<int> AddRange()
    {
        var list = new List<int>();
        list.AddRange(_collection);
        return list;
    }

    [Benchmark]
    public List<int> ManualForeach()
    {
        var list = new List<int>();
        MyCollection.Enumerator enumerator = _collection.GetEnumerator();
        try
        {
            while (enumerator.MoveNext())
            {
                list.Add(enumerator.Current);
            }
            return list;
        }
        finally
        {
            ((IDisposable)enumerator).Dispose();
        }
    }
}

class MyCollection(List<int> list) : IEnumerable<int>
{
    public Enumerator GetEnumerator() => new(list.GetEnumerator());

    IEnumerator<int> IEnumerable<int>.GetEnumerator() => GetEnumerator();

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

    public struct Enumerator(List<int>.Enumerator enumerator) : IEnumerator<int>
    {
        public int Current => enumerator.Current;

        object IEnumerator.Current => Current;

        public bool MoveNext() => enumerator.MoveNext();

        public void Dispose() => enumerator.Dispose();

        public void Reset() { }
    }
}
BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3880/23H2/2023Update/SunValley3)
Intel Core i5-8300H CPU 2.30GHz (Coffee Lake), 1 CPU, 8 logical and 4 physical cores
.NET SDK 9.0.100-preview.6.24328.19
  [Host]     : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2


| Method        | CollectionSize | Mean         | Error      | StdDev     | Ratio | Gen0   | Allocated | Alloc Ratio |
|-------------- |--------------- |-------------:|-----------:|-----------:|------:|-------:|----------:|------------:|
| AddRange      | 0              |    16.270 ns |  0.1821 ns |  0.1614 ns |  1.00 | 0.0172 |      72 B |        1.00 |
| ManualForeach | 0              |     5.486 ns |  0.0429 ns |  0.0401 ns |  0.34 | 0.0076 |      32 B |        0.44 |
|               |                |              |            |            |       |        |           |             |
| AddRange      | 10             |    82.285 ns |  0.6432 ns |  0.5702 ns |  1.00 | 0.0612 |     256 B |        1.00 |
| ManualForeach | 10             |    61.129 ns |  0.4521 ns |  0.3776 ns |  0.74 | 0.0516 |     216 B |        0.84 |
|               |                |              |            |            |       |        |           |             |
| AddRange      | 100            |   395.748 ns |  3.4296 ns |  3.2081 ns |  1.00 | 0.2923 |    1224 B |        1.00 |
| ManualForeach | 100            |   276.310 ns |  2.7586 ns |  2.5804 ns |  0.70 | 0.2828 |    1184 B |        0.97 |
|               |                |              |            |            |       |        |           |             |
| AddRange      | 1000           | 3,204.768 ns | 14.5439 ns | 12.8928 ns |  1.00 | 2.0218 |    8464 B |        1.00 |
| ManualForeach | 1000           | 2,089.974 ns |  9.6826 ns |  9.0571 ns |  0.65 | 2.0142 |    8424 B |        1.00 |
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Jul 31, 2024
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

1 similar comment
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@RikkiGibson
Copy link
Contributor

Related: #71273 (comment)

@jaredpar jaredpar added the Code Gen Quality Room for improvement in the quality of the compiler's generated code label Aug 5, 2024
@jaredpar jaredpar added this to the Backlog milestone Aug 5, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area-Compilers Code Gen Quality Room for improvement in the quality of the compiler's generated code Feature - Collection Expressions untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants