Skip to content

Conversation

stephentoub
Copy link
Member

SortedDictionary's struct enumerator wraps an underlying TreeSet's enumerator. But when the SortedDictionary is enumerated via is IEnumerable<T> implementation, rather than boxing and returning the SortedDictionary's enumerator, we can skip that layer and instead directly use the TreeSet's.

(Changing this does mean if someone got an enumerator from the IEnumerable<> and then tried to cast it back to the strongly-typed Enumerator, that would now fail. But that falls into undefined behavior territory.)

Method Toolchain Mean Ratio
Sum \main\corerun.exe 979.6 ns 1.00
Sum \pr\corerun.exe 870.3 ns 0.89
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Collections.Generic;

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

public class Bench
{
    private IEnumerable<KeyValuePair<int, int>> _e = new SortedDictionary<int, int>(Enumerable.Range(0, 100).Select(i => (i, i)).ToDictionary());

    [Benchmark]
    public int Sum()
    {
        int sum = 0;
        foreach (var entry in _e) sum += entry.Value;
        return sum;
    }
}

SortedDictionary's struct enumerator wraps an underlying TreeSet's enumerator. But when the SortedDictionary is enumerated via is `IEnumerable<T>` implementation, rather than boxing and returning the SortedDictionary's enumerator, we can skip that layer and instead directly use the TreeSet's.
@Copilot Copilot AI review requested due to automatic review settings July 7, 2025 01:53
Copy link
Contributor

@Copilot 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

This PR optimizes SortedDictionary enumeration by removing an extra wrapper layer when using the generic IEnumerable interface and streamlines the non-generic Current implementation.

  • IEnumerable<KeyValuePair<TKey, TValue>>.GetEnumerator now directly returns the TreeSet’s enumerator.
  • Non-generic IEnumerator.Current no longer constructs a new KeyValuePair for the generic path and simply returns the Current struct.
  • Benchmark shows ~11% improvement in enumeration throughput.
Comments suppressed due to low confidence (2)

src/libraries/System.Collections/src/System/Collections/Generic/SortedDictionary.cs:248

  • Add unit tests to verify that IEnumerable<KeyValuePair<TKey, TValue>>.GetEnumerator() produces the same sequence as SortedDictionary<TKey, TValue>.GetEnumerator(), covering both empty and populated dictionaries.
            _set.GetEnumerator();

src/libraries/System.Collections/src/System/Collections/Generic/SortedDictionary.cs:469

  • Add tests for the non-generic IDictionaryEnumerator.Current path to ensure it returns a DictionaryEntry when expected and correctly boxes the KeyValuePair in the generic path.
                    return Current;

Copy link
Contributor

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

@eiriktsarpalis
Copy link
Member

(Changing this does mean if someone got an enumerator from the IEnumerable<> and then tried to cast it back to the strongly-typed Enumerator, that would now fail. But that falls into undefined behavior territory.)

Having the interface method return a different type than the strongly typed variant certainly violates my expectations, although I guess we crossed that Rubicon by special casing empty collection enumerators.

@stephentoub
Copy link
Member Author

I guess we crossed that Rubicon by special casing empty collection enumerators.

Yup.

@stephentoub stephentoub enabled auto-merge (squash) July 7, 2025 14:03
@stephentoub stephentoub merged commit 0a727a5 into dotnet:main Jul 7, 2025
79 of 88 checks passed
@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.

2 participants