Conversation
…llocations *** Reduce allocations from doing repeated adds into an ImmutableDictionary in SolutionCompilationState.ComputeFrozenSnapshot Instead, use an ImmutableSegmentedDictionaryBuilder to do the adds and then do a single update to the SolutionCompilationState._projectIdToTrackerMap after all new compilation trackers are created. Note that there is a potential negative aspect to this change as it increases the time window during which a compilation state can be queried for in the SolutionCompilationState and not found. However, I don't believe this will cause functional issues, and wasn't hitting that concern during local testing.
|
Test insertion PR: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/597687 |
Speedometer results look good:
|
| } | ||
|
|
||
| // Update _projectIdToTrackerMap to include all the newly created compilation trackers. | ||
| RoslynImmutableInterlocked.InterlockedExchange(ref _projectIdToTrackerMap, updatedIdToTrackerMapBuilder.ToImmutable()); |
There was a problem hiding this comment.
i feel like there's some sort of race here. where multiple threads come in, update the build, and the other maps, and then blast back into this field. this actually feels like it could potentially be a problem. convince me this is 100% safe.
There was a problem hiding this comment.
this may need to be a spin, with an interlocked compare-exchange. and if the field changes out from underneath us, we have to go redo this work to pick up any comp trackers made by another thread.
There was a problem hiding this comment.
Actually, I'm now not convinced there wasn't a timing issue in this code before.
There was a problem hiding this comment.
Actually, this PR was the one that added the multiple usages of _projectIdToTrackerMap within the method. I've pushed out a commit that I think essentially does what you requested.
|
Commit 2 VS Test insertion to get new speedometer numbers: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/598001 Results look good again:
|
Instead, use an ImmutableSegmentedDictionaryBuilder to do the adds and then do a single update to the SolutionCompilationState._projectIdToTrackerMap after all new compilation trackers are created.
Note that there is a potential negative aspect to this change as it increases the time window during which a compilation state can be queried for in the SolutionCompilationState and not found. However, I don't believe this will cause functional issues, and wasn't hitting that concern during local testing.
*** allocations before change from typing period in csharpediting speedometer ***
