-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make diagnostic checksum computation into an extension method. #77277
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
using System.Collections.Immutable; | ||
using System.Diagnostics; | ||
using System.Linq; | ||
using System.Runtime.CompilerServices; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.Collections; | ||
|
@@ -21,6 +22,8 @@ namespace Microsoft.CodeAnalysis.Diagnostics; | |
|
||
internal static partial class Extensions | ||
{ | ||
private static readonly ConditionalWeakTable<Project, AsyncLazy<Checksum>> s_projectToDiagnosticChecksum = new(); | ||
|
||
public static async Task<ImmutableArray<Diagnostic>> ToDiagnosticsAsync(this IEnumerable<DiagnosticData> diagnostics, Project project, CancellationToken cancellationToken) | ||
{ | ||
var result = ArrayBuilder<Diagnostic>.GetInstance(); | ||
|
@@ -429,4 +432,76 @@ await suppressionAnalyzer.AnalyzeAsync( | |
semanticModel, span, hostCompilationWithAnalyzers, analyzerInfoCache.GetDiagnosticDescriptors, reportDiagnostic, cancellationToken).ConfigureAwait(false); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Calculates a checksum that contains a project's checksum along with a checksum for each of the project's | ||
/// transitive dependencies. | ||
/// </summary> | ||
/// <remarks> | ||
/// This checksum calculation can be used for cases where a feature needs to know if the semantics in this project | ||
/// changed. For example, for diagnostics or caching computed semantic data. The goal is to ensure that changes to | ||
/// <list type="bullet"> | ||
/// <item>Files inside the current project</item> | ||
/// <item>Project properties of the current project</item> | ||
/// <item>Visible files in referenced projects</item> | ||
/// <item>Project properties in referenced projects</item> | ||
/// </list> | ||
/// are reflected in the metadata we keep so that comparing solutions accurately tells us when we need to recompute | ||
/// semantic work. | ||
/// | ||
/// <para>This method of checking for changes has a few important properties that differentiate it from other methods of determining project version. | ||
/// <list type="bullet"> | ||
/// <item>Changes to methods inside the current project will be reflected to compute updated diagnostics. | ||
/// <see cref="Project.GetDependentSemanticVersionAsync(CancellationToken)"/> does not change as it only returns top level changes.</item> | ||
/// <item>Reloading a project without making any changes will re-use cached diagnostics. | ||
/// <see cref="Project.GetDependentSemanticVersionAsync(CancellationToken)"/> changes as the project is removed, then added resulting in a version change.</item> | ||
/// </list> | ||
/// </para> | ||
/// This checksum is also affected by the <see cref="SourceGeneratorExecutionVersion"/> for this project. | ||
/// As such, it is not usable across different sessions of a particular host. | ||
/// </remarks> | ||
public static Task<Checksum> GetDiagnosticChecksumAsync(this Project? project, CancellationToken cancellationToken) | ||
{ | ||
if (project is null) | ||
return SpecializedTasks.Default<Checksum>(); | ||
|
||
var lazyChecksum = s_projectToDiagnosticChecksum.GetValue( | ||
project, | ||
static project => AsyncLazy.Create( | ||
static (project, cancellationToken) => ComputeDiagnosticChecksumAsync(project, cancellationToken), | ||
project)); | ||
|
||
return lazyChecksum.GetValueAsync(cancellationToken); | ||
|
||
static async Task<Checksum> ComputeDiagnosticChecksumAsync(Project project, CancellationToken cancellationToken) | ||
{ | ||
var solution = project.Solution; | ||
|
||
using var _ = ArrayBuilder<Checksum>.GetInstance(out var tempChecksumArray); | ||
|
||
// Mix in the SG information for this project. That way if it changes, we will have a different | ||
// checksum (since semantics could have changed because of this). | ||
if (solution.CompilationState.SourceGeneratorExecutionVersionMap.Map.TryGetValue(project.Id, out var executionVersion)) | ||
tempChecksumArray.Add(executionVersion.Checksum); | ||
|
||
// Get the checksum for the project itself. Note: this will normally be cached. As such, even if we | ||
// have a different Project instance (due to a change in an unrelated project), this will be fast to | ||
// compute and return. | ||
var projectChecksum = await project.State.GetChecksumAsync(cancellationToken).ConfigureAwait(false); | ||
tempChecksumArray.Add(projectChecksum); | ||
|
||
// 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't have any repeats afaict. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
{ | ||
// Note that these checksums should only actually be calculated once, if the project is unchanged | ||
// the same checksum will be returned. | ||
tempChecksumArray.Add(await GetDiagnosticChecksumAsync( | ||
solution.GetProject(projectRef.ProjectId), cancellationToken).ConfigureAwait(false)); | ||
} | ||
|
||
return Checksum.Create(tempChecksumArray); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,8 +28,6 @@ private sealed class WithFrozenSourceGeneratedDocumentsCompilationTracker : ICom | |
{ | ||
private readonly TextDocumentStates<SourceGeneratedDocumentState> _replacementDocumentStates; | ||
|
||
private AsyncLazy<Checksum>? _lazyDependentChecksum; | ||
|
||
/// <summary> | ||
/// The lazily-produced compilation that has the generated document updated. This is initialized by call to | ||
/// <see cref="GetCompilationAsync"/>. | ||
|
@@ -145,28 +143,6 @@ public Task<VersionStamp> GetDependentVersionAsync(SolutionCompilationState comp | |
public Task<VersionStamp> GetDependentSemanticVersionAsync(SolutionCompilationState compilationState, CancellationToken cancellationToken) | ||
=> UnderlyingTracker.GetDependentSemanticVersionAsync(compilationState, cancellationToken); | ||
|
||
public Task<Checksum> GetDependentChecksumAsync(SolutionCompilationState compilationState, CancellationToken cancellationToken) | ||
{ | ||
if (_lazyDependentChecksum == null) | ||
{ | ||
var tmp = compilationState; // temp. local to avoid a closure allocation for the fast path | ||
// note: solution is captured here, but it will go away once GetValueAsync executes. | ||
Interlocked.CompareExchange( | ||
ref _lazyDependentChecksum, | ||
AsyncLazy.Create(static (arg, c) => | ||
arg.self.ComputeDependentChecksumAsync(arg.tmp, c), | ||
arg: (self: this, tmp)), | ||
null); | ||
} | ||
|
||
return _lazyDependentChecksum.GetValueAsync(cancellationToken); | ||
} | ||
|
||
private async Task<Checksum> ComputeDependentChecksumAsync(SolutionCompilationState compilationState, CancellationToken cancellationToken) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
=> Checksum.Create( | ||
await UnderlyingTracker.GetDependentChecksumAsync(compilationState, cancellationToken).ConfigureAwait(false), | ||
(await _replacementDocumentStates.GetDocumentChecksumsAndIdsAsync(cancellationToken).ConfigureAwait(false)).Checksum); | ||
|
||
public async ValueTask<TextDocumentStates<SourceGeneratedDocumentState>> GetSourceGeneratedDocumentStatesAsync( | ||
SolutionCompilationState compilationState, bool withFrozenSourceGeneratedDocuments, CancellationToken cancellationToken) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding:
I thought generally it was best to avoid CWTs unless it was quite difficult not to do so as they add extra strain to GC. Is this not the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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