Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix hot path string allocations from ProjectKey #10138

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

DustinCampbell
Copy link
Member

@ToddGrun pointed me to a fair amount of string allocation due ProjectKey.From(Project) being called repeatedly on a hot path. To address this, I've added ProjectKey.Matches(Project), which simply compares the current key's Id with the project's intermediate output path. This uses a new FilePathNormalizer.AreDirectoryPathsEquivalent(...) helper to avoid allocating a new string.

image

@ToddGrun pointed me to a fair amount of string allocation due `ProjectKey.From(Project)` being called repeatedly on a hot path. To address this, I've added `ProjectKey.Matches(Project)`, which simply compares the current key's `Id` with the project's intermediate output path. This uses a new `FilePathNormalizer.AreDirectoryPathsEquivalent(...)` helper to avoid allocating a new string.
@DustinCampbell DustinCampbell requested a review from a team as a code owner March 21, 2024 17:15
Copy link
Contributor

@alexgav alexgav left a comment

Choose a reason for hiding this comment

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

:shipit:

@DustinCampbell DustinCampbell requested a review from a team as a code owner March 21, 2024 23:05
@DustinCampbell DustinCampbell merged commit 08189eb into dotnet:main Mar 21, 2024
12 checks passed
@DustinCampbell DustinCampbell deleted the fix-projectkey-alloc branch March 21, 2024 23:38
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Mar 21, 2024
@RikkiGibson RikkiGibson modified the milestones: Next, 17.10 P3 Mar 25, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants