Skip to content

Conversation

@ToddGrun
Copy link
Contributor

Currently, this method is calculating some data upfront for the value part of a TryAdd call. Instead, defer calculating that data until after we've determine it's not already in the dictionary.

I saw this accounting for 2.3% of CPU during loading Roslyn.sln. With this change, it was about 0.2%.

*** original CPU usage opening Roslyn.sln ***
image

*** CPU usage with this change opening Roslyn.sln ***
image

Currently, this method is calculating some data upfront for the value part of a TryAdd call. Instead, defer calculating that data until after we've determine it's not already in the dictionary.

I saw this accounting for 2.3% of CPU during loading Roslyn.sln. With this change, it was about 0.2%.
@ToddGrun ToddGrun requested a review from jaredpar March 31, 2025 19:58
@ToddGrun ToddGrun requested a review from a team as a code owner March 31, 2025 19:58
@ghost ghost added Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 31, 2025
{
if (_originalPathInfoMap.TryGetValue(originalPath, out var originalPathInfo))
{
Debug.Assert(GeneratedPathComparer.Equals(_originalPathInfoMap[originalPath].ResolvedPath, originalPathInfo.ResolvedPath));
Copy link
Member

Choose a reason for hiding this comment

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

I didn't fully understand what this assert is verifying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to match the code in the else clause of the TryAdd call below it, but that's pretty much not useful in this case

@ToddGrun ToddGrun merged commit f8cdce1 into dotnet:release/dev17.15 Apr 1, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants