Skip to content

Commit

Permalink
Cache based on project state not project. (#77141)
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi authored Feb 14, 2025
1 parent 6b9b958 commit 40b0cba
Show file tree
Hide file tree
Showing 16 changed files with 114 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editor.UnitTests;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Serialization;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Simplification;
using Microsoft.CodeAnalysis.Test.Utilities;
Expand Down Expand Up @@ -775,6 +776,7 @@ private static async Task TestNuGetAndVsixAnalyzerCoreAsync(
if (nugetAnalyzerReferences.Count > 0)
{
project = project.WithAnalyzerReferences([new AnalyzerImageReference([.. nugetAnalyzerReferences])]);
SerializerService.TestAccessor.AddAnalyzerImageReferences(project.AnalyzerReferences);
}

var document = project.Documents.Single();
Expand Down
6 changes: 5 additions & 1 deletion src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using Microsoft.CodeAnalysis.Extensions;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Serialization;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.SolutionCrawler;
using Microsoft.CodeAnalysis.Test.Utilities;
Expand Down Expand Up @@ -429,7 +430,8 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context)
}
}

private class MockAnalyzerReference : AnalyzerReference, ICodeFixProviderFactory
private class MockAnalyzerReference
: AnalyzerReference, ICodeFixProviderFactory, SerializerService.TestAccessor.IAnalyzerReferenceWithGuid
{
public readonly ImmutableArray<CodeFixProvider> Fixers;
public readonly ImmutableArray<DiagnosticAnalyzer> Analyzers;
Expand Down Expand Up @@ -490,6 +492,8 @@ public override object Id
}
}

public Guid Guid { get; } = Guid.NewGuid();

public override ImmutableArray<DiagnosticAnalyzer> GetAnalyzers(string language)
=> Analyzers;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Remote.Diagnostics;
using Microsoft.CodeAnalysis.Remote.Testing;
using Microsoft.CodeAnalysis.Serialization;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Simplification;
using Microsoft.CodeAnalysis.SolutionCrawler;
Expand Down Expand Up @@ -435,6 +436,7 @@ internal async Task TestAdditionalFileAnalyzer(bool registerFromInitialize, bool
project = project.AddAdditionalDocument(name: "dummy2.txt", text: "Additional File2 Text", filePath: "dummy2.txt").Project;
}

SerializerService.TestAccessor.AddAnalyzerImageReferences(project.AnalyzerReferences);
var applied = workspace.TryApplyChanges(project.Solution);
Assert.True(applied);

Expand Down
35 changes: 35 additions & 0 deletions src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ namespace Microsoft.CodeAnalysis.Diagnostics;
internal partial class DiagnosticAnalyzerService
{
/// <summary>
/// Cached data from a <see cref="Project"/> to the last <see cref="CompilationWithAnalyzersPair"/> instance created
/// for it. Note: the CompilationWithAnalyzersPair instance is dependent on the set of <see
/// Cached data from a <see cref="ProjectState"/> to the last <see cref="CompilationWithAnalyzersPair"/> instance
/// created for it. Note: the CompilationWithAnalyzersPair instance is dependent on the set of <see
/// cref="DiagnosticAnalyzer"/>s passed along with the project. As such, we might not be able to use a prior cached
/// value if the set of analyzers changes. In that case, a new instance will be created and will be cached for the
/// next caller.
/// </summary>
private static readonly ConditionalWeakTable<Project, StrongBox<(ImmutableArray<DiagnosticAnalyzer> analyzers, CompilationWithAnalyzersPair? compilationWithAnalyzersPair)>> s_projectToCompilationWithAnalyzers = new();
private static readonly ConditionalWeakTable<ProjectState, StrongBox<(Checksum checksum, ImmutableArray<DiagnosticAnalyzer> analyzers, CompilationWithAnalyzersPair? compilationWithAnalyzersPair)>> s_projectToCompilationWithAnalyzers = new();

private static async Task<CompilationWithAnalyzersPair?> GetOrCreateCompilationWithAnalyzersAsync(
Project project,
Expand All @@ -36,25 +36,30 @@ internal partial class DiagnosticAnalyzerService
if (!project.SupportsCompilation)
return null;

var projectState = project.State;
var checksum = await project.GetDependentChecksumAsync(cancellationToken).ConfigureAwait(false);

// Make sure the cached pair was computed with at least the same state sets we're asking about. if not,
// recompute and cache with the new state sets.
if (!s_projectToCompilationWithAnalyzers.TryGetValue(project, out var tupleBox) ||
if (!s_projectToCompilationWithAnalyzers.TryGetValue(projectState, out var tupleBox) ||
tupleBox.Value.checksum != checksum ||
!analyzers.IsSubsetOf(tupleBox.Value.analyzers))
{
var compilationWithAnalyzersPair = await CreateCompilationWithAnalyzersAsync().ConfigureAwait(false);
tupleBox = new((analyzers, compilationWithAnalyzersPair));
var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false);
var compilationWithAnalyzersPair = CreateCompilationWithAnalyzers(projectState, compilation);
tupleBox = new((checksum, analyzers, compilationWithAnalyzersPair));

#if NET
s_projectToCompilationWithAnalyzers.AddOrUpdate(project, tupleBox);
s_projectToCompilationWithAnalyzers.AddOrUpdate(projectState, tupleBox);
#else
// Make a best effort attempt to store the latest computed value against these state sets. If this
// fails (because another thread interleaves with this), that's ok. We still return the pair we
// computed, so our caller will still see the right data
s_projectToCompilationWithAnalyzers.Remove(project);
s_projectToCompilationWithAnalyzers.Remove(projectState);

// Intentionally ignore the result of this. We still want to use the value we computed above, even if
// another thread interleaves and sets a different value.
s_projectToCompilationWithAnalyzers.GetValue(project, _ => tupleBox);
s_projectToCompilationWithAnalyzers.GetValue(projectState, _ => tupleBox);
#endif
}

Expand All @@ -63,13 +68,12 @@ internal partial class DiagnosticAnalyzerService
// <summary>
// Should only be called on a <see cref="Project"/> that <see cref="Project.SupportsCompilation"/>.
// </summary>
async Task<CompilationWithAnalyzersPair?> CreateCompilationWithAnalyzersAsync()
CompilationWithAnalyzersPair? CreateCompilationWithAnalyzers(
ProjectState project, Compilation compilation)
{
var projectAnalyzers = analyzers.WhereAsArray(static (s, info) => !info.IsHostAnalyzer(s), hostAnalyzerInfo);
var hostAnalyzers = analyzers.WhereAsArray(static (s, info) => info.IsHostAnalyzer(s), hostAnalyzerInfo);

var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false);

// Create driver that holds onto compilation and associated analyzers
var filteredProjectAnalyzers = projectAnalyzers.WhereAsArray(static a => !a.IsWorkspaceDiagnosticAnalyzer());
var filteredHostAnalyzers = hostAnalyzers.WhereAsArray(static a => !a.IsWorkspaceDiagnosticAnalyzer());
Expand All @@ -83,9 +87,6 @@ internal partial class DiagnosticAnalyzerService
return null;
}

Contract.ThrowIfFalse(project.SupportsCompilation);
AssertCompilation(project, compilation);

var exceptionFilter = (Exception ex) =>
{
if (ex is not OperationCanceledException && crashOnAnalyzerException)
Expand All @@ -105,7 +106,7 @@ internal partial class DiagnosticAnalyzerService
var projectCompilation = !filteredProjectAnalyzers.Any()
? null
: compilation.WithAnalyzers(filteredProjectAnalyzers, new CompilationWithAnalyzersOptions(
options: project.AnalyzerOptions,
options: project.ProjectAnalyzerOptions,
onAnalyzerException: null,
analyzerExceptionFilter: exceptionFilter,
concurrentAnalysis: false,
Expand All @@ -126,12 +127,4 @@ internal partial class DiagnosticAnalyzerService
return new CompilationWithAnalyzersPair(projectCompilation, hostCompilation);
}
}

[Conditional("DEBUG")]
private static void AssertCompilation(Project project, Compilation compilation1)
{
// given compilation must be from given project.
Contract.ThrowIfFalse(project.TryGetCompilation(out var compilation2));
Contract.ThrowIfFalse(compilation1 == compilation2);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ private partial class DiagnosticIncrementalAnalyzer
{
private partial class StateManager
{
private HostAnalyzerInfo GetOrCreateHostAnalyzerInfo(Project project, ProjectAnalyzerInfo projectAnalyzerInfo)
private HostAnalyzerInfo GetOrCreateHostAnalyzerInfo(
Project project, ProjectAnalyzerInfo projectAnalyzerInfo)
{
var key = new HostAnalyzerInfoKey(project.Language, project.State.HasSdkCodeStyleAnalyzers, project.Solution.SolutionState.Analyzers.HostAnalyzerReferences);
var analyzers = project.Solution.SolutionState.Analyzers;
var key = new HostAnalyzerInfoKey(project.Language, project.State.HasSdkCodeStyleAnalyzers, analyzers.HostAnalyzerReferences);
// Some Host Analyzers may need to be treated as Project Analyzers so that they do not have access to the
// Host fallback options. These ids will be used when building up the Host and Project analyzer collections.
var referenceIdsToRedirect = GetReferenceIdsToRedirectAsProjectAnalyzers(project);
var hostAnalyzerInfo = ImmutableInterlocked.GetOrAdd(ref _hostAnalyzerStateMap, key, CreateLanguageSpecificAnalyzerMap, (project.Solution.SolutionState.Analyzers, referenceIdsToRedirect));
var hostAnalyzerInfo = ImmutableInterlocked.GetOrAdd(ref _hostAnalyzerStateMap, key, CreateLanguageSpecificAnalyzerMap, (Analyzers: analyzers, referenceIdsToRedirect));
return hostAnalyzerInfo.WithExcludedAnalyzers(projectAnalyzerInfo.SkippedAnalyzersInfo.SkippedAnalyzers);

static HostAnalyzerInfo CreateLanguageSpecificAnalyzerMap(HostAnalyzerInfoKey arg, (HostDiagnosticAnalyzers HostAnalyzers, ImmutableHashSet<object> ReferenceIdsToRedirect) state)
Expand Down Expand Up @@ -65,7 +67,8 @@ static HostAnalyzerInfo CreateLanguageSpecificAnalyzerMap(HostAnalyzerInfoKey ar
}
}

private static ImmutableHashSet<object> GetReferenceIdsToRedirectAsProjectAnalyzers(Project project)
private static ImmutableHashSet<object> GetReferenceIdsToRedirectAsProjectAnalyzers(
Project project)
{
if (project.State.HasSdkCodeStyleAnalyzers)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,15 @@ private ProjectAnalyzerInfo CreateProjectAnalyzerInfo(Project project)
// workspace placeholder analyzers. So we should never get host analyzers back here.
Contract.ThrowIfTrue(newHostAnalyzers.Count > 0);

var skippedAnalyzersInfo = project.GetSkippedAnalyzersInfo(_analyzerInfoCache);
var skippedAnalyzersInfo = hostAnalyzers.GetSkippedAnalyzersInfo(project.State, _analyzerInfoCache);
return new ProjectAnalyzerInfo(project.AnalyzerReferences, newAllAnalyzers, skippedAnalyzersInfo);
}

/// <summary>
/// Updates the map to the given project snapshot.
/// </summary>
private async Task<ProjectAnalyzerInfo> UpdateProjectAnalyzerInfoAsync(Project project, CancellationToken cancellationToken)
private async Task<ProjectAnalyzerInfo> UpdateProjectAnalyzerInfoAsync(
Project project, CancellationToken cancellationToken)
{
// This code is called concurrently for a project, so the guard prevents duplicated effort calculating StateSets.
using (await _projectAnalyzerStateMapGuard.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,16 @@ private partial class StateManager(DiagnosticAnalyzerInfoCache analyzerInfoCache
/// <summary>
/// Return <see cref="DiagnosticAnalyzer"/>s for the given <see cref="Project"/>.
/// </summary>
public async Task<ImmutableArray<DiagnosticAnalyzer>> GetOrCreateAnalyzersAsync(Project project, CancellationToken cancellationToken)
public async Task<ImmutableArray<DiagnosticAnalyzer>> GetOrCreateAnalyzersAsync(
Project project, CancellationToken cancellationToken)
{
var hostAnalyzerInfo = await GetOrCreateHostAnalyzerInfoAsync(project, cancellationToken).ConfigureAwait(false);
var projectAnalyzerInfo = await GetOrCreateProjectAnalyzerInfoAsync(project, cancellationToken).ConfigureAwait(false);
return hostAnalyzerInfo.OrderedAllAnalyzers.AddRange(projectAnalyzerInfo.Analyzers);
}

public async Task<HostAnalyzerInfo> GetOrCreateHostAnalyzerInfoAsync(Project project, CancellationToken cancellationToken)
public async Task<HostAnalyzerInfo> GetOrCreateHostAnalyzerInfoAsync(
Project project, CancellationToken cancellationToken)
{
var projectAnalyzerInfo = await GetOrCreateProjectAnalyzerInfoAsync(project, cancellationToken).ConfigureAwait(false);
return GetOrCreateHostAnalyzerInfo(project, projectAnalyzerInfo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Workspaces.Diagnostics;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Diagnostics;

Expand All @@ -29,6 +28,8 @@ private partial class DiagnosticIncrementalAnalyzer

public async Task<ImmutableArray<DiagnosticData>> ForceAnalyzeProjectAsync(Project project, CancellationToken cancellationToken)
{
var projectState = project.State;

try
{
if (!_projectToForceAnalysisData.TryGetValue(project, out var box))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private async Task<DiagnosticAnalysisResultMap<DiagnosticAnalyzer, DiagnosticAna

var projectAnalyzers = documentAnalysisScope?.ProjectAnalyzers ?? compilationWithAnalyzers.ProjectAnalyzers;
var hostAnalyzers = documentAnalysisScope?.HostAnalyzers ?? compilationWithAnalyzers.HostAnalyzers;
var skippedAnalyzersInfo = project.GetSkippedAnalyzersInfo(AnalyzerInfoCache);
var skippedAnalyzersInfo = project.Solution.SolutionState.Analyzers.GetSkippedAnalyzersInfo(project.State, AnalyzerInfoCache);

// get compiler result builder map
var builderMap = ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResultBuilder>.Empty;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ private static ImmutableDictionary<object, ImmutableArray<DiagnosticAnalyzer>> M
return current;
}

public SkippedHostAnalyzersInfo GetSkippedAnalyzersInfo(Project project, DiagnosticAnalyzerInfoCache infoCache)
public SkippedHostAnalyzersInfo GetSkippedAnalyzersInfo(ProjectState project, DiagnosticAnalyzerInfoCache infoCache)
{
var box = _skippedHostAnalyzers.GetOrCreateValue(project.AnalyzerReferences);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

namespace Microsoft.CodeAnalysis.Serialization;

using static Microsoft.CodeAnalysis.Serialization.SerializerService.TestAccessor;
using static TemporaryStorageService;

internal partial class SerializerService
Expand All @@ -29,18 +30,18 @@ internal partial class SerializerService
/// pretend that a <see cref="AnalyzerImageReference"/> is a <see cref="AnalyzerFileReference"/> during tests.
/// </summary>
private static readonly object s_analyzerImageReferenceMapGate = new();
private static IBidirectionalMap<AnalyzerImageReference, Guid> s_analyzerImageReferenceMap = BidirectionalMap<AnalyzerImageReference, Guid>.Empty;
private static IBidirectionalMap<AnalyzerReference, Guid> s_analyzerReferenceMap = BidirectionalMap<AnalyzerReference, Guid>.Empty;

private static bool TryGetAnalyzerImageReferenceGuid(AnalyzerImageReference imageReference, out Guid guid)
{
lock (s_analyzerImageReferenceMapGate)
return s_analyzerImageReferenceMap.TryGetValue(imageReference, out guid);
return s_analyzerReferenceMap.TryGetValue(imageReference, out guid);
}

private static bool TryGetAnalyzerImageReferenceFromGuid(Guid guid, [NotNullWhen(true)] out AnalyzerImageReference? imageReference)
private static bool TryGetAnalyzerImageReferenceFromGuid(Guid guid, [NotNullWhen(true)] out AnalyzerReference? analyzerReference)
{
lock (s_analyzerImageReferenceMapGate)
return s_analyzerImageReferenceMap.TryGetKey(guid, out imageReference);
return s_analyzerReferenceMap.TryGetKey(guid, out analyzerReference);
}

private static Checksum CreateChecksum(MetadataReference reference)
Expand Down Expand Up @@ -78,6 +79,12 @@ protected virtual Checksum CreateChecksum(AnalyzerReference reference)
writer.WriteGuid(guid);
break;

case IAnalyzerReferenceWithGuid analyzerReferenceWithGuid:
lock (s_analyzerImageReferenceMapGate)
s_analyzerReferenceMap = s_analyzerReferenceMap.Add(reference, analyzerReferenceWithGuid.Guid);
writer.WriteGuid(analyzerReferenceWithGuid.Guid);
break;

default:
throw ExceptionUtilities.UnexpectedValue(reference);
}
Expand Down Expand Up @@ -482,12 +489,6 @@ private static unsafe void WriteTo(MetadataReader reader, ObjectWriter writer)
writer.WriteSpan(new ReadOnlySpan<byte>(reader.MetadataPointer, reader.MetadataLength));
}

private static void WriteUnresolvedAnalyzerReferenceTo(AnalyzerReference reference, ObjectWriter writer)
{
writer.WriteString(nameof(UnresolvedAnalyzerReference));
writer.WriteString(reference.FullPath);
}

private static Metadata? TryGetMetadata(PortableExecutableReference reference)
{
try
Expand Down Expand Up @@ -541,9 +542,23 @@ public static void AddAnalyzerImageReference(AnalyzerImageReference analyzerImag
{
lock (s_analyzerImageReferenceMapGate)
{
if (!s_analyzerImageReferenceMap.ContainsKey(analyzerImageReference))
s_analyzerImageReferenceMap = s_analyzerImageReferenceMap.Add(analyzerImageReference, Guid.NewGuid());
if (!s_analyzerReferenceMap.ContainsKey(analyzerImageReference))
s_analyzerReferenceMap = s_analyzerReferenceMap.Add(analyzerImageReference, Guid.NewGuid());
}
}

public static void AddAnalyzerImageReferences(IReadOnlyList<AnalyzerReference> analyzerReferences)
{
foreach (var analyzer in analyzerReferences)
{
if (analyzer is AnalyzerImageReference analyzerImageReference)
AddAnalyzerImageReference(analyzerImageReference);
}
}

public interface IAnalyzerReferenceWithGuid
{
Guid Guid { get; }
}
}
}
Loading

0 comments on commit 40b0cba

Please sign in to comment.