Skip to content

Reduce allocations in LazyItemEvaluator.ProcessMetadataElements by avoiding enumerator boxing#12908

Merged
JanProvaznik merged 5 commits intodotnet:mainfrom
nareshjo:dev/nareshjo/processmetadataelementsboxing
Dec 18, 2025
Merged

Reduce allocations in LazyItemEvaluator.ProcessMetadataElements by avoiding enumerator boxing#12908
JanProvaznik merged 5 commits intodotnet:mainfrom
nareshjo:dev/nareshjo/processmetadataelementsboxing

Conversation

@nareshjo
Copy link
Contributor

@nareshjo nareshjo commented Dec 9, 2025 •
edited
Loading

This pull request was generated by the VS Perf Rel AI Agent. Please review this AI-generated PR with extra care! For more information, visit our wiki. Please share feedback with TIP Insights

  • Issue: ProcessMetadataElements(ProjectItemElement, OperationBuilderWithMetadata) calls AddRange(itemElement.Metadata) on ImmutableArray.Builder. ETW traces show this method as an allocation hotspot due to allocations from IEnumerable.GetEnumerator interface calls during AddRange(KeyValuePair<K, V>[]), indicating the collection's struct enumerator is being boxed to satisfy the IEnumerable parameter requirement.
  • Issue type: AVOID boxing enumerators of built-in types
  • Proposed fix: Remove the AddRange(KeyValuePair<K, V>[]) call and add elements individually inside the existing ForEach(Action) loop. The C# compiler optimizes ForEach(Action) to directly use the collection's concrete GetEnumerator() method, avoiding boxing when a struct enumerator is available. This also eliminates duplicate enumeration of Metadata, providing two performance benefits:
  1. Eliminates boxing allocation - The struct enumerator is used directly without boxing to the IEnumerable interface
  2. Eliminates duplicate enumeration - The collection is only enumerated once instead of twice

Best practices wiki
See related failure in PRISM
ADO work item

Copilot AI review requested due to automatic review settings December 9, 2025 18:07
Copilot AI reviewed Dec 9, 2025
View reviewed changes
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

This PR optimizes ProcessMetadataElements to reduce allocations by eliminating enumerator boxing. The change moves metadata element addition from a single AddRange call to individual Add calls within the existing loop.

Key changes:

  • Removed AddRange(itemElement.Metadata) call that caused boxing of struct enumerator
  • Added individual Add(metadatumElement) calls inside the existing foreach loop
  • Eliminated duplicate enumeration of the Metadata collection

@nareshjo
Copy link
Contributor Author

nareshjo commented Dec 16, 2025

@rainersigwald thanks for reviewing. I don't have permissions to merge. Could you please merge it if you do.

DustinCampbell
DustinCampbell approved these changes Dec 16, 2025
View reviewed changes
Copy link
Member

@DustinCampbell DustinCampbell left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I don't know that I have merge permissions either, but I've spent a lot of time in LazyItemEvaluator lately, and this looks good to me.

@rainersigwald
Copy link
Member

rainersigwald commented Dec 16, 2025

We'll merge after an insertion with #12950 validates that we haven't accidentally regressed VS while we were missing coverage.

@JanProvaznik JanProvaznik merged commit 92212c0 into dotnet:main Dec 18, 2025
10 checks passed
This was referenced Dec 19, 2025
Merged
Merged
@dotnet-maestro dotnet-maestro bot mentioned this pull request Feb 12, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@DustinCampbell DustinCampbell DustinCampbell approved these changes

@rainersigwald rainersigwald rainersigwald approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@nareshjo @rainersigwald @DustinCampbell @JanProvaznik @YuliiaKovalova