Skip to content

[benchmarks skill test] restore Count fast paths after benchmark regression#125012

Closed
Copilot wants to merge 3 commits intomainfrom
copilot/remove-fast-paths-from-count
Closed

[benchmarks skill test] restore Count fast paths after benchmark regression#125012
Copilot wants to merge 3 commits intomainfrom
copilot/remove-fast-paths-from-count

Conversation

Copy link
Contributor

Copilot AI commented Feb 28, 2026

Description

This PR now reverts the earlier fast-path removal in Enumerable.Count<TSource>(IEnumerable<TSource>) after benchmark analysis showed major regressions.

What changed

  • Restored the O(1) fast paths in Count<TSource>:
    • ICollection<T>.Count
    • Iterator<T>.GetCount(onlyIfCheap: false)
    • ICollection.Count
  • Restored related overflow-focused test expectations in:
    • src/libraries/System.Linq/tests/AppendPrependTests.cs
    • src/libraries/System.Linq/tests/ConcatTests.cs

Why

Benchmark results from EgorBot issue #13 (Linux and macOS targets) showed substantial CountArray regressions after removing fast paths, so the change was reverted to preserve existing performance characteristics.

Testing

  • Baseline build completed:
    • ./build.sh clr+libs -rc release
  • Targeted System.Linq validation completed:
    • dotnet build
    • dotnet build /t:test ./tests/System.Linq.Tests.csproj
  • Test results: System.Linq.Tests passed (0 failed).

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@dotnet-policy-service
Copy link
Contributor

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

@stephentoub stephentoub added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 28, 2026
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove fast paths from Count<T> in LINQ [benchmarks skill test] remove fast path from Count Feb 28, 2026
@EgorBo
Copy link
Member

EgorBo commented Feb 28, 2026

Benchmarks — @EgorBot -linux_amd -osx_arm64

🤦‍♂️

@EgorBo
Copy link
Member

EgorBo commented Feb 28, 2026

@copilot use the benchmark skill again to schedule some performance testing for this PR. Remember, the command must start from a new line.

@EgorBo
Copy link
Member

EgorBo commented Feb 28, 2026

I'll probably add some LLM processing on my back-end to see if @EgorBot is just mentioned in a casual text message, or it's a real command.

Copy link
Contributor Author

Copilot AI commented Feb 28, 2026

@copilot use the benchmark skill again to schedule some performance testing for this PR. Remember, the command must start from a new line.

@EgorBot -linux_amd -osx_arm64

using System.Linq;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(Bench).Assembly).Run(args);

[MemoryDiagnoser]
public class Bench
{
    private List<int> _list = default!;
    private int[] _array = default!;
    private IEnumerable<int> _selectIterator = default!;
    private IEnumerable<int> _concatIterator = default!;

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

    [GlobalSetup]
    public void Setup()
    {
        _array = Enumerable.Range(0, N).ToArray();
        _list = _array.ToList();
        _selectIterator = Enumerable.Range(0, N).Select(x => x);
        _concatIterator = Enumerable.Range(0, N / 2).Concat(Enumerable.Range(N / 2, N / 2));
    }

    [Benchmark]
    public int CountList() => _list.Count();

    [Benchmark]
    public int CountArray() => _array.Count();

    [Benchmark]
    public int CountSelectIterator() => _selectIterator.Count();

    [Benchmark]
    public int CountConcatIterator() => _concatIterator.Count();
}

@EgorBo
Copy link
Member

EgorBo commented Feb 28, 2026

@copilot use the benchmark skill again to schedule some performance testing for this PR write just one [Benchmark] for an array case. Remember, the command must start from a new line.

Copy link
Contributor Author

Copilot AI commented Feb 28, 2026

@copilot use the benchmark skill again to schedule some performance testing for this PR write just one [Benchmark] for an array case. Remember, the command must start from a new line.

b8029bd

@EgorBot -linux_amd -osx_arm64

using System.Linq;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(Bench).Assembly).Run(args);

[MemoryDiagnoser]
public class Bench
{
    private int[] _array = default!;

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

    [GlobalSetup]
    public void Setup()
    {
        _array = Enumerable.Range(0, N).ToArray();
    }

    [Benchmark]
    public int CountArray() => _array.Count();
}

@EgorBo
Copy link
Member

EgorBo commented Feb 28, 2026

@copilot use the benchmark skill again to schedule some performance testing for this PR write just one [Benchmark] for an array case. Remember, the command must start from a new line.

Copy link
Contributor Author

Copilot AI commented Feb 28, 2026

@copilot use the benchmark skill again to schedule some performance testing for this PR write just one [Benchmark] for an array case. Remember, the command must start from a new line.

b8029bd

@EgorBot -linux_amd -osx_arm64

using System.Linq;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(Bench).Assembly).Run(args);

[MemoryDiagnoser]
public class Bench
{
    private int[] _array = default!;

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

    [GlobalSetup]
    public void Setup()
    {
        _array = Enumerable.Range(0, N).ToArray();
    }

    [Benchmark]
    public int CountArray() => _array.Count();
}

@EgorBo
Copy link
Member

EgorBo commented Feb 28, 2026

@copilot, benchmark results are ready: EgorBot/Benchmarks#12

@EgorBo
Copy link
Member

EgorBo commented Feb 28, 2026

@copilot use the benchmark skill again to schedule some performance testing for this PR write just one [Benchmark] for an array case. Remember, the command must start from a new line.

Copy link
Contributor Author

Copilot AI commented Feb 28, 2026

@copilot use the benchmark skill again to schedule some performance testing for this PR write just one [Benchmark] for an array case. Remember, the command must start from a new line.

b8029bd

@EgorBot -linux_amd -osx_arm64

using System.Linq;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(Bench).Assembly).Run(args);

[MemoryDiagnoser]
public class Bench
{
    private int[] _array = default!;

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

    [GlobalSetup]
    public void Setup()
    {
        _array = Enumerable.Range(0, N).ToArray();
    }

    [Benchmark]
    public int CountArray() => _array.Count();
}

@EgorBo
Copy link
Member

EgorBo commented Feb 28, 2026

@copilot, benchmark results are ready:

Full tracking issue: EgorBot/Benchmarks#13

Please analyze the results and act accordingly — if there are regressions, consider reverting or fixing; if results look good, no action is needed. Note: some benchmarks may be flaky or bi-modal, so use your judgment when interpreting small differences.

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 28, 2026

@copilot, benchmark results are ready:

41a77b1

Analyzed both runs from issue #13: CountArray regressed substantially on both targets (main near-zero vs PR scaling with N, e.g. ~24ns at N=100 and ~233ns at N=1000), which confirms the fast-path removal is a real perf regression.

I reverted the change in commit 41a77b12 by restoring Count fast paths and the related overflow test expectations.

UI screenshot: N/A (no UI changes).

Copilot AI changed the title [benchmarks skill test] remove fast path from Count [benchmarks skill test] restore Count fast paths after benchmark regression Feb 28, 2026
@EgorBo EgorBo closed this Feb 28, 2026
@jkotas jkotas deleted the copilot/remove-fast-paths-from-count branch March 1, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Infrastructure NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants