Skip to content
New issue

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

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

Already on GitHub? Sign in 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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