Skip to content

Conversation

@stephentoub
Copy link
Member

There's a non-trivial amount of unnecessary overhead/complexity in Stack<T>'s enumerator. This streamlines it.

(It does also change the undefined behavior of using Current erroneously. We can put back most of its current semantics if we really care. This change puts it on a plan closer to List<T>.)

Method Toolchain Mean Error StdDev Ratio Allocated Alloc Ratio
IntsEnumerable \main\corerun.exe 232.50 ns 1.022 ns 0.956 ns 1.00 40 B 1.00
IntsEnumerable \pr\corerun.exe 57.13 ns 0.401 ns 0.356 ns 0.25 - 0.00
IntsStack \main\corerun.exe 220.76 ns 1.008 ns 0.943 ns 1.00 - NA
IntsStack \pr\corerun.exe 54.96 ns 0.377 ns 0.335 ns 0.25 - NA
StringsEnumerable \main\corerun.exe 345.99 ns 1.714 ns 1.604 ns 1.00 40 B 1.00
StringsEnumerable \pr\corerun.exe 67.93 ns 0.639 ns 0.598 ns 0.20 - 0.00
StringsStack \main\corerun.exe 243.20 ns 1.808 ns 1.603 ns 1.00 - NA
StringsStack \pr\corerun.exe 55.86 ns 0.603 ns 0.534 ns 0.23 - NA
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

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

[MemoryDiagnoser(false)]
public class Bench
{
    const int Length = 100;

    private IEnumerable<int> _intsEnumerable = new Stack<int>(Enumerable.Range(0, Length));
    private Stack<int> _intsStack = new Stack<int>(Enumerable.Range(0, Length));
    private IEnumerable<string> _stringsEnumerable = new Stack<string>(Enumerable.Range(0, Length).Select(i => i.ToString()));
    private Stack<string> _stringsStack = new Stack<string>(Enumerable.Range(0, Length).Select(i => i.ToString()));

    [Benchmark]
    public int IntsEnumerable()
    {
        int sum = 0;
        foreach (var i in _intsEnumerable) sum += i;
        return sum;
    }

    [Benchmark]
    public int IntsStack()
    {
        int sum = 0;
        foreach (var i in _intsStack) sum += i;
        return sum;
    }

    [Benchmark]
    public int StringsEnumerable()
    {
        int sum = 0;
        foreach (var s in _stringsEnumerable) sum += s.Length;
        return sum;
    }

    [Benchmark]
    public int StringsStack()
    {
        int sum = 0;
        foreach (var s in _stringsStack) sum += s.Length;
        return sum;
    }
}

There's a non-trivial amount of unnecessary overhead/complexity in `Stack<T>`'s enumerator. This streamlines it.

(It does also change the undefined behavior of using Current erroneously. We can put back most of its current semantics if we really care. This change puts it on a plan closer to `List<T>`.)
Copilot AI review requested due to automatic review settings July 5, 2025 04:25
@dotnet-policy-service
Copy link
Contributor

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Streamlines Stack<T>'s enumerator by removing extra state checks and aligning its behavior more closely with List<T>, resulting in a significant performance improvement.

  • Renames test properties related to undefined Current behavior.
  • Refactors Enumerator to use a single index and simpler MoveNext/Current logic.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/libraries/System.Collections/tests/Generic/Stack/Stack.Tests.cs Renamed test property for undefined Current behavior
src/libraries/System.Collections/tests/Generic/Stack/Stack.Generic.Tests.cs Renamed test property for undefined Current behavior
src/libraries/System.Collections/src/System/Collections/Generic/Stack.cs Simplified Enumerator implementation, updated index handling and removed old guards
Comments suppressed due to low confidence (1)

src/libraries/System.Collections/tests/Generic/Stack/Stack.Tests.cs:27

  • The property name looks like it has a typo. It should be Enumerator_Empty_Current_UndefinedOperation_Throws (with a trailing 's') to match the naming pattern and the corresponding base property.
        protected override bool Enumerator_Empty_Current_UndefinedOperation_Throw => true;

@stephentoub stephentoub merged commit 14a8b18 into dotnet:main Jul 7, 2025
80 of 88 checks passed
@stephentoub stephentoub deleted the stackenumerator branch July 7, 2025 13:17
@github-actions github-actions bot locked and limited conversation to collaborators Aug 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants