Make diagnostic checksum computation into an extension method. #77277
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
|
@jasonmalinowski @dibarbet this is ready for review. |
|
Is the motivation for this just to simplify the implementation? |
| return _lazyDependentChecksum.GetValueAsync(cancellationToken); | ||
| } | ||
|
|
||
| private async Task<Checksum> ComputeDependentChecksumAsync(SolutionCompilationState compilationState, CancellationToken cancellationToken) |
There was a problem hiding this comment.
Correct. But I don't think the complexity is worth it. Having the new system operate uniformly on any Project instance is more desirable for me.
| // Calculate a checksum this project and for each dependent project that could affect semantics for this | ||
| // project. We order the projects guid so that we are resilient to the underlying in-memory graph structure | ||
| // changing this arbitrarily. | ||
| foreach (var projectRef in project.ProjectReferences.OrderBy(r => r.ProjectId.Id)) |
There was a problem hiding this comment.
We shouldn't have any repeats afaict.
There was a problem hiding this comment.
Note: an the expensive checksumming will be cached (at the ProjectState level). All this is doing is mixing the checksums together to make the final checksum.
We honestly could even consider not caching this. I was a bit nervous about going that much, in case of LSP where we might have N requests on the same Project.
| @@ -21,6 +22,8 @@ namespace Microsoft.CodeAnalysis.Diagnostics; | |||
|
|
|||
| internal static partial class Extensions | |||
| { | |||
| private static readonly ConditionalWeakTable<Project, AsyncLazy<Checksum>> s_projectToDiagnosticChecksum = new(); | |||
There was a problem hiding this comment.
So I didn't think there would be an issue with cwt here given how few keys there would be.
We can definitely run speedometer to see if there are concerns here
That's a large part yes. Also, to not depend as much on internal data which is easier to get wrong. |
|
Kicked off pipeline. |
|
No issues found. @ToddGrun ptal. |
Pipeline: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=11052330&view=results