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

Enable non-allocating enumeration via collection interfaces #5246

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

drewnoakes
Copy link
Contributor

@drewnoakes drewnoakes commented Jun 16, 2023

Bug

Fixes: NuGet/Home#12679

Regression? Last working version: N/A

Description

Interfaces like IEnumerable<T>, IList<T> and IDictionary<TKey, TValue> are used in many APIs. The upside of this is that the implementation of these APIs may change the concrete type of the collection as it sees fit without breaking consumers. The downside of this is that some operations through the interface are more expensive than if they were performed directly on the concrete type. Enumeration is perhaps the biggest challege here, as enumerating through these interfaces with foreach always requires a heap allocation.

Most concrete implementations these implementations will be types such as List<T> and Dictionary<TKey,TValue> which have a non-allocating struct enumerators.

Specifically:

List<int> list = new();
IList<int> ilist = list;

foreach (int i in list) {} // does not allocate
foreach (int i in ilist) {} // ❌ allocates a boxed enumerator

This change adds NoAllocEnumerate() extension methods that avoid any enumerator allocation when the concrete collection type is known to have such a struct enumerator at runtime.

List<int> list = new();
IList<int> ilist = list;

foreach (int i in list) {} // does not allocate
foreach (int i in ilist.NoAllocEnumerate()) {} // ✅ does not allocate

Note that this pattern can be extended to other commonly enumerated types that have struct enumerators. Here it's specialized for IEnumerable<T>, IList<T> and IDictionary<TKey,TValue> in order to limit the number of struct enumerators needed on the structs themselves. It seems like NuGet.Client mostly uses List<T> and Dictionary<TKey,TValue> (as opposed to other collections like ImmutableArray<T>, ImmutableList<T>, ImmutableDictionary<TKey,TValue>, etc.). The list of supported types can be extended as investigations and investment in this area continue.

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

Sorry, something went wrong.

@drewnoakes drewnoakes requested a review from a team as a code owner June 16, 2023 05:03
@drewnoakes drewnoakes force-pushed the dev-drnoakes-no-alloc-enumerate branch from 7f60154 to e5195d3 Compare June 16, 2023 05:22

Verified

This commit was signed with the committer’s verified signature.
drewnoakes Drew Noakes
Interfaces like `IEnumerable<T>`, `IList<T>` and `IDictionary<TKey, TValue>` are used in many APIs. The upside of this is that the implementation of these APIs may change the concrete type of the collection as it sees fit without breaking consumers. The downside of this is that some operations through the interface are more expensive than if they were performed directly on the concrete type. Enumeration is perhaps the biggest challege here, as enumerating through these interfaces with `foreach` always requires a heap allocation.

Most concrete implementations these implementations will be types such as `List<T>` and `Dictionary<TKey,TValue>` which have a non-allocating struct enumerators.

This change adds `NoAllocEnumerate()` extension methods that avoid any enumerator allocation when the concrete collection type is known to have such a struct enumerator at runtime.
@drewnoakes drewnoakes force-pushed the dev-drnoakes-no-alloc-enumerate branch from e5195d3 to 830e6ee Compare June 16, 2023 10:48
@drewnoakes drewnoakes changed the title Enable non-allocating enumeration for IList<T> Enable non-allocating enumeration via collection interfaces Jun 16, 2023
@jeffkl jeffkl mentioned this pull request Jun 19, 2023
8 tasks

namespace NuGet;

internal static class NoAllocEnumerateExtensions
Copy link
Contributor

Choose a reason for hiding this comment

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

I wish this sort of thing was built in. I also really want an analyzer to ensure we keep using this sort of thing as we add more code.

@drewnoakes
Copy link
Contributor Author

Once this is merged in I'll file a series of PRs that close off perfathon issues by using these extension methods.

@nkolev92 nkolev92 merged commit 6e78daf into NuGet:dev Jun 21, 2023
@drewnoakes drewnoakes deleted the dev-drnoakes-no-alloc-enumerate branch June 21, 2023 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add NoAllocEnumerateExtensions
3 participants