Skip to content

Use Builder in ImmutableDictionary.SetItems extension#12402

Merged
AR-May merged 4 commits into
dotnet:mainfrom
ccastanedaucf:dev/chcasta/perf-setitems
Oct 1, 2025
Merged

Use Builder in ImmutableDictionary.SetItems extension#12402
AR-May merged 4 commits into
dotnet:mainfrom
ccastanedaucf:dev/chcasta/perf-setitems

Conversation

@ccastanedaucf

@ccastanedaucf ccastanedaucf commented Aug 22, 2025

Copy link
Copy Markdown
Contributor

Context

Small change to avoid allocating a generated enumerable. Using a Builder here is otherwise identical in perf to SetItems, as both hit the same paths to allocate a new instance + modified subtree nodes. ~34MB diff below.

Before
image

After
image

Copilot AI review requested due to automatic review settings August 22, 2025 21:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This change optimizes memory allocation in the SetItems extension method for ImmutableDictionary<string, string> by using ImmutableDictionary.Builder instead of a generator-based approach. The modification eliminates the intermediate enumerable allocation while maintaining the same validation logic and null-to-empty-string conversion behavior.

Key Changes

  • Replaced ValidateItems generator method with direct Builder operations
  • Eliminated intermediate enumerable allocation, saving ~34MB according to profiling data
  • Maintained identical functionality for key validation and null value handling

Comment thread src/Framework/ImmutableDictionaryExtensions.cs
@ccastanedaucf ccastanedaucf changed the title Use ImmutableDictionary.Builder in SetItems extension Use Builder in ImmutableDictionary.SetItems extension Aug 22, 2025
@ghost ghost assigned AR-May Sep 2, 2025

@AR-May AR-May left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. I will make exp insertion and look at numbers.

Comment thread src/Framework/ImmutableDictionaryExtensions.cs

@surayya-MS surayya-MS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me!

@AR-May

AR-May commented Oct 1, 2025

Copy link
Copy Markdown
Member

Ok, exp insertion came up showing improvements. merging the PR.
There were some weird regressions, but they are for sure unrelated, that for kitten to investigate.

@AR-May AR-May merged commit ccf3981 into dotnet:main Oct 1, 2025
9 checks passed
This was referenced Jul 10, 2026
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.

4 participants