Skip to content

Conversation

@Erarndt
Copy link
Contributor

@Erarndt Erarndt commented Jun 5, 2025

Slightly reduce allocations in GetReferencedItemNamesAndMetadata

Context

We can avoid eagerly creating a substring in this method to reduce allocations. The resulting "name" string is conditionally used in one place

                // If we've got this far, we know the item expression was
                // well formed, so make sure the name's in the table
                if ((whatToShredFor & ShredderOptions.ItemTypes) != 0)
                {
                    pair.Items ??= new HashSet<string>(MSBuildNameIgnoreCaseComparer.Default);
                    pair.Items.Add(name);
                }

We can capture the boundaries of the desired substring and only create it when we actually need it.

Before:
image

After:
image

Not a dramatic difference, but the change is small.

Changes Made

Testing

Notes

Copilot AI review requested due to automatic review settings June 5, 2025 17:46
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

Defers string allocation in GetReferencedItemNamesAndMetadata by capturing name boundaries instead of creating substrings when scanning expressions.

  • Updates the comment to reflect the boundary-capture approach.
  • Introduces nameLength and defers the Substring call until inserting into pair.Items.
Comments suppressed due to low confidence (1)

src/Build/Evaluation/ExpressionShredder.cs:303

  • [nitpick] Clarify the comment to reflect that substring creation is deferred until needed, e.g.: 'Determine name start and length, deferring substring creation until storage.'
// Grab the name boundaries, but continue to verify it's a well-formed expression

@JanProvaznik JanProvaznik merged commit fe47950 into dotnet:main Jun 6, 2025
10 checks passed
@Erarndt Erarndt deleted the dev/erarndt/deferStringAllocInGetReferencedItemNamesAndMetadata branch June 6, 2025 16:27
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.

3 participants