From 0199590012d36f533668c097c5aae61060417ca6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Oct 2021 15:22:14 -0700 Subject: [PATCH 01/14] Remove branch-ids from teh workspace --- .../Workspace/CompileTimeSolutionProvider.cs | 60 +++++++------------ .../FindSymbols/SyntaxTree/SyntaxTreeIndex.cs | 2 - .../Portable/Remote/PinnedSolutionInfo.cs | 17 +----- .../Shared/Extensions/DocumentExtensions.cs | 3 - .../Shared/Extensions/ProjectExtensions.cs | 3 - .../Portable/Workspace/Solution/BranchId.cs | 28 --------- .../Portable/Workspace/Solution/Solution.cs | 4 +- .../Workspace/Solution/SolutionState.cs | 42 +------------ .../Core/Portable/Workspace/Workspace.cs | 9 +-- .../Remote/Core/SolutionAssetStorage.cs | 1 - .../Remote/Core/SolutionChecksumUpdater.cs | 5 -- .../DiagnosticAnalyzer/DiagnosticComputer.cs | 6 +- 12 files changed, 30 insertions(+), 150 deletions(-) delete mode 100644 src/Workspaces/Core/Portable/Workspace/Solution/BranchId.cs diff --git a/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs b/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs index 83d868caf9472..35fef13368e68 100644 --- a/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs +++ b/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs @@ -8,6 +8,7 @@ using System.Diagnostics; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -48,16 +49,11 @@ public Factory() private readonly object _gate = new(); - /// - /// Cached compile time solution corresponding to the - /// - private (int DesignTimeSolutionVersion, BranchId DesignTimeSolutionBranch, Solution CompileTimeSolution)? _primaryBranchCompileTimeCache; - - /// - /// Cached compile time solution for a forked branch. This is used primarily by LSP cases where - /// we fork the workspace solution and request diagnostics for the forked solution. - /// - private (int DesignTimeSolutionVersion, BranchId DesignTimeSolutionBranch, Solution CompileTimeSolution)? _forkedBranchCompileTimeCache; +#if NETCOREAPP + private readonly ConditionalWeakTable _designTimeToCompileTimeSoution = new(); +#else + private ConditionalWeakTable _designTimeToCompileTimeSoution = new(); +#endif public CompileTimeSolutionProvider(Workspace workspace) { @@ -67,8 +63,11 @@ public CompileTimeSolutionProvider(Workspace workspace) { lock (_gate) { - _primaryBranchCompileTimeCache = null; - _forkedBranchCompileTimeCache = null; +#if NETCOREAPP + _designTimeToCompileTimeSoution.Clear(); +#else + _designTimeToCompileTimeSoution = new(); +#endif } } }; @@ -82,10 +81,8 @@ public Solution GetCompileTimeSolution(Solution designTimeSolution) { lock (_gate) { - var cachedCompileTimeSolution = GetCachedCompileTimeSolution(designTimeSolution); - - // Design time solution hasn't changed since we calculated the last compile-time solution: - if (cachedCompileTimeSolution != null) + if (_designTimeToCompileTimeSoution.TryGetValue(designTimeSolution, out var cachedCompileTimeSolution) && + cachedCompileTimeSolution != null) { return cachedCompileTimeSolution; } @@ -129,33 +126,16 @@ public Solution GetCompileTimeSolution(Solution designTimeSolution) } } - private Solution? GetCachedCompileTimeSolution(Solution designTimeSolution) - { - // If the design time solution is for the primary branch, retrieve the last cached solution for it. - // Otherwise this is a forked solution, so retrieve the last forked compile time solution we calculated. - var cachedCompileTimeSolution = designTimeSolution.BranchId == _workspace.PrimaryBranchId ? _primaryBranchCompileTimeCache : _forkedBranchCompileTimeCache; - - // Verify that the design time solution has not changed since the last calculated compile time solution and that - // the design time solution branch matches the branch of the design time solution we calculated the compile time solution for. - if (cachedCompileTimeSolution != null - && designTimeSolution.WorkspaceVersion == cachedCompileTimeSolution.Value.DesignTimeSolutionVersion - && designTimeSolution.BranchId == cachedCompileTimeSolution.Value.DesignTimeSolutionBranch) - { - return cachedCompileTimeSolution.Value.CompileTimeSolution; - } - - return null; - } - private void UpdateCachedCompileTimeSolution(Solution designTimeSolution, Solution compileTimeSolution) { - if (designTimeSolution.BranchId == _workspace.PrimaryBranchId) - { - _primaryBranchCompileTimeCache = (designTimeSolution.WorkspaceVersion, designTimeSolution.BranchId, compileTimeSolution); - } - else + lock (_gate) { - _forkedBranchCompileTimeCache = (designTimeSolution.WorkspaceVersion, designTimeSolution.BranchId, compileTimeSolution); +#if NETCOREAPP + _designTimeToCompileTimeSoution.AddOrUpdate(designTimeSolution, compileTimeSolution); +#else + _designTimeToCompileTimeSoution.Remove(designTimeSolution); + _designTimeToCompileTimeSoution.Add(designTimeSolution, compileTimeSolution); +#endif } } diff --git a/src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex.cs b/src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex.cs index f108a8ddcd4f3..c194a43a15ccf 100644 --- a/src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex.cs +++ b/src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex.cs @@ -54,8 +54,6 @@ public static async Task PrecalculateAsync(Document document, CancellationToken using (Logger.LogBlock(FunctionId.SyntaxTreeIndex_Precalculate, cancellationToken)) { - Debug.Assert(document.IsFromPrimaryBranch()); - var checksum = await GetChecksumAsync(document, cancellationToken).ConfigureAwait(false); // Check if we've already created and persisted the index for this document. diff --git a/src/Workspaces/Core/Portable/Remote/PinnedSolutionInfo.cs b/src/Workspaces/Core/Portable/Remote/PinnedSolutionInfo.cs index 471b6723e1d31..a93b96781fa46 100644 --- a/src/Workspaces/Core/Portable/Remote/PinnedSolutionInfo.cs +++ b/src/Workspaces/Core/Portable/Remote/PinnedSolutionInfo.cs @@ -20,41 +20,30 @@ internal sealed class PinnedSolutionInfo [DataMember(Order = 0)] public readonly int ScopeId; - /// - /// This indicates whether this scope is for primary branch or not (not forked solution) - /// - /// Features like OOP will use this flag to see whether caching information related to this solution - /// can benefit other requests or not - /// - [DataMember(Order = 1)] - public readonly bool FromPrimaryBranch; - /// /// This indicates a Solution.WorkspaceVersion of this solution. remote host engine uses this version /// to decide whether caching this solution will benefit other requests or not /// - [DataMember(Order = 2)] + [DataMember(Order = 1)] public readonly int WorkspaceVersion; - [DataMember(Order = 3)] + [DataMember(Order = 2)] public readonly Checksum SolutionChecksum; /// /// An optional project that we are pinning information for. This is used for features that only need /// information for a project (and its dependencies) and not the entire solution. /// - [DataMember(Order = 4)] + [DataMember(Order = 3)] public readonly ProjectId? ProjectId; public PinnedSolutionInfo( int scopeId, - bool fromPrimaryBranch, int workspaceVersion, Checksum solutionChecksum, ProjectId? projectId) { ScopeId = scopeId; - FromPrimaryBranch = fromPrimaryBranch; WorkspaceVersion = workspaceVersion; SolutionChecksum = solutionChecksum; ProjectId = projectId; diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/DocumentExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/DocumentExtensions.cs index 9042957444581..7bda5310f62d2 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/DocumentExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/DocumentExtensions.cs @@ -13,9 +13,6 @@ namespace Microsoft.CodeAnalysis.Shared.Extensions { internal static partial class DocumentExtensions { - public static bool IsFromPrimaryBranch(this Document document) - => document.Project.Solution.BranchId == document.Project.Solution.Workspace.PrimaryBranchId; - public static async ValueTask GetSyntaxTreeIndexAsync(this Document document, CancellationToken cancellationToken) { var result = await SyntaxTreeIndex.GetIndexAsync(document, loadOnly: false, cancellationToken).ConfigureAwait(false); diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/ProjectExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/ProjectExtensions.cs index e7d31b9f0edb7..08a642b53c413 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/ProjectExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/ProjectExtensions.cs @@ -11,9 +11,6 @@ namespace Microsoft.CodeAnalysis.Shared.Extensions { internal static partial class ProjectExtensions { - public static bool IsFromPrimaryBranch(this Project project) - => project.Solution.BranchId == project.Solution.Workspace.PrimaryBranchId; - internal static Project WithSolutionOptions(this Project project, OptionSet options) => project.Solution.WithOptions(options).GetProject(project.Id)!; diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/BranchId.cs b/src/Workspaces/Core/Portable/Workspace/Solution/BranchId.cs deleted file mode 100644 index 63f8970e5b9b6..0000000000000 --- a/src/Workspaces/Core/Portable/Workspace/Solution/BranchId.cs +++ /dev/null @@ -1,28 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.Diagnostics; -using System.Threading; - -namespace Microsoft.CodeAnalysis -{ - /// - /// solution branch Id - /// - [DebuggerDisplay("{_id}")] - internal class BranchId - { - private static int s_nextId; - -#pragma warning disable IDE0052 // Remove unread private members - private readonly int _id; -#pragma warning restore IDE0052 // Remove unread private members - - private BranchId(int id) - => _id = id; - - internal static BranchId GetNextId() - => new(Interlocked.Increment(ref s_nextId)); - } -} diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs index 9422d0778002c..9f231ded15600 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs @@ -38,7 +38,7 @@ private Solution(SolutionState state) } internal Solution(Workspace workspace, SolutionInfo.SolutionAttributes solutionAttributes, SerializableOptionSet options, IReadOnlyList analyzerReferences) - : this(new SolutionState(workspace.PrimaryBranchId, new SolutionServices(workspace), solutionAttributes, options, analyzerReferences)) + : this(new SolutionState(new SolutionServices(workspace), solutionAttributes, options, analyzerReferences)) { } @@ -48,8 +48,6 @@ internal Solution(Workspace workspace, SolutionInfo.SolutionAttributes solutionA internal SolutionServices Services => _state.Services; - internal BranchId BranchId => _state.BranchId; - internal ProjectState? GetProjectState(ProjectId projectId) => _state.GetProjectState(projectId); /// diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs index 59917352f8595..de1cc153ddf09 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs @@ -32,9 +32,6 @@ namespace Microsoft.CodeAnalysis /// internal partial class SolutionState { - // branch id for this solution - private readonly BranchId _branchId; - // the version of the workspace this solution is from private readonly int _workspaceVersion; @@ -73,7 +70,6 @@ internal partial class SolutionState private readonly SourceGeneratedDocumentState? _frozenSourceGeneratedDocumentState; private SolutionState( - BranchId branchId, int workspaceVersion, SolutionServices solutionServices, SolutionInfo.SolutionAttributes solutionAttributes, @@ -88,7 +84,6 @@ private SolutionState( Lazy? lazyAnalyzers, SourceGeneratedDocumentState? frozenSourceGeneratedDocument) { - _branchId = branchId; _workspaceVersion = workspaceVersion; _solutionAttributes = solutionAttributes; _solutionServices = solutionServices; @@ -115,13 +110,11 @@ static Lazy CreateLazyHostDiagnosticAnalyzers(IReadOnly } public SolutionState( - BranchId primaryBranchId, SolutionServices solutionServices, SolutionInfo.SolutionAttributes solutionAttributes, SerializableOptionSet options, IReadOnlyList analyzerReferences) : this( - primaryBranchId, workspaceVersion: 0, solutionServices, solutionAttributes, @@ -146,7 +139,7 @@ public SolutionState WithNewWorkspace(Workspace workspace, int workspaceVersion) // Note: this will potentially have problems if the workspace services are different, as some services // get locked-in by document states and project states when first constructed. - return CreatePrimarySolution(branchId: workspace.PrimaryBranchId, workspaceVersion: workspaceVersion, services: services); + return CreatePrimarySolution(workspaceVersion: workspaceVersion, services: services); } public HostDiagnosticAnalyzers Analyzers => _lazyAnalyzers.Value; @@ -163,19 +156,6 @@ public SolutionState WithNewWorkspace(Workspace workspace, int workspaceVersion) public SerializableOptionSet Options { get; } - /// - /// branch id of this solution - /// - /// currently, it only supports one level of branching. there is a primary branch of a workspace and all other - /// branches that are branched from the primary branch. - /// - /// one still can create multiple forked solutions from an already branched solution, but versions among those - /// can't be reliably used and compared. - /// - /// version only has a meaning between primary solution and branched one or between solutions from same branch. - /// - public BranchId BranchId => _branchId; - /// /// The Workspace this solution is associated with. /// @@ -230,8 +210,6 @@ private SolutionState Branch( ProjectDependencyGraph? dependencyGraph = null, Optional frozenSourceGeneratedDocument = default) { - var branchId = GetBranchId(); - if (idToProjectStateMap is not null) { Contract.ThrowIfNull(remoteSupportedProjectLanguages); @@ -251,8 +229,7 @@ private SolutionState Branch( var analyzerReferencesEqual = AnalyzerReferences.SequenceEqual(analyzerReferences); - if (branchId == _branchId && - solutionAttributes == _solutionAttributes && + if (solutionAttributes == _solutionAttributes && projectIds == ProjectIds && options == Options && analyzerReferencesEqual && @@ -266,7 +243,6 @@ private SolutionState Branch( } return new SolutionState( - branchId, _workspaceVersion, _solutionServices, solutionAttributes, @@ -283,19 +259,16 @@ private SolutionState Branch( } private SolutionState CreatePrimarySolution( - BranchId branchId, int workspaceVersion, SolutionServices services) { - if (branchId == _branchId && - workspaceVersion == _workspaceVersion && + if (workspaceVersion == _workspaceVersion && services == _solutionServices) { return this; } return new SolutionState( - branchId, workspaceVersion, services, _solutionAttributes, @@ -311,15 +284,6 @@ private SolutionState CreatePrimarySolution( frozenSourceGeneratedDocument: null); } - private BranchId GetBranchId() - { - // currently we only support one level branching. - // my reasonings are - // 1. it seems there is no-one who needs sub branches. - // 2. this lets us to branch without explicit branch API - return _branchId == Workspace.PrimaryBranchId ? BranchId.GetNextId() : _branchId; - } - /// /// The version of the most recently modified project. /// diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index 52f03e84ab2bd..a5be90fcee5e8 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -35,7 +35,6 @@ public abstract partial class Workspace : IDisposable { private readonly string? _workspaceKind; private readonly HostWorkspaceServices _services; - private readonly BranchId _primaryBranchId; private readonly IOptionService _optionService; @@ -71,7 +70,6 @@ public abstract partial class Workspace : IDisposable /// A string that can be used to identify the kind of workspace. Usually this matches the name of the class. protected Workspace(HostServices host, string? workspaceKind) { - _primaryBranchId = BranchId.GetNextId(); _workspaceKind = workspaceKind; _services = host.CreateWorkspaceServices(this); @@ -110,11 +108,6 @@ internal void SetTestLogger(Action? writeLineMessageLogger) /// public HostWorkspaceServices Services => _services; - /// - /// primary branch id that current solution has - /// - internal BranchId PrimaryBranchId => _primaryBranchId; - /// /// Override this property if the workspace supports partial semantics for documents. /// @@ -1200,7 +1193,7 @@ internal virtual bool TryApplyChanges(Solution newSolution, IProgressTracker pro // the given solution must be a branched one. // otherwise, there should be no change to apply. - if (oldSolution.BranchId == newSolution.BranchId) + if (oldSolution == newSolution) { CheckNoChanges(oldSolution, newSolution); return true; diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs index ce8ff0a0f7686..351a63ee882bf 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs @@ -53,7 +53,6 @@ private async ValueTask StoreAssetsAsync(Solution solution, ProjectId? pr var id = Interlocked.Increment(ref s_scopeId); var solutionInfo = new PinnedSolutionInfo( id, - fromPrimaryBranch: solutionState.BranchId == solutionState.Workspace.PrimaryBranchId, solutionState.WorkspaceVersion, solutionChecksum, projectId); diff --git a/src/Workspaces/Remote/Core/SolutionChecksumUpdater.cs b/src/Workspaces/Remote/Core/SolutionChecksumUpdater.cs index 7df27f3d2b349..4900fabbe3093 100644 --- a/src/Workspaces/Remote/Core/SolutionChecksumUpdater.cs +++ b/src/Workspaces/Remote/Core/SolutionChecksumUpdater.cs @@ -128,11 +128,6 @@ private void EnqueueChecksumUpdate() private async Task SynchronizePrimaryWorkspaceAsync(CancellationToken cancellationToken) { var solution = _workspace.CurrentSolution; - if (solution.BranchId != _workspace.PrimaryBranchId) - { - return; - } - var client = await RemoteHostClient.TryGetClientAsync(_workspace, cancellationToken).ConfigureAwait(false); if (client == null) { diff --git a/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs b/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs index b99ff55be839f..84a8a3890325c 100644 --- a/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs +++ b/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs @@ -32,8 +32,7 @@ internal class DiagnosticComputer /// NOTE: We do not re-use this cache for project analysis as it leads to significant memory increase in the OOP process, /// and CWT does not seem to drop entries until ForceGC happens. /// - private static readonly ConditionalWeakTable s_compilationWithAnalyzersCache - = new ConditionalWeakTable(); + private static readonly ConditionalWeakTable s_compilationWithAnalyzersCache = new(); private readonly TextDocument? _document; private readonly Project _project; @@ -55,8 +54,7 @@ public DiagnosticComputer( _analysisKind = analysisKind; _analyzerInfoCache = analyzerInfoCache; - // We only track performance from primary branch. All forked branch we don't care such as preview. - _performanceTracker = project.IsFromPrimaryBranch() ? project.Solution.Workspace.Services.GetService() : null; + _performanceTracker = project.Solution.Workspace.Services.GetService(); } public async Task GetDiagnosticsAsync( From 2715c699cb48cc751b426a51ca226700d7db973a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Oct 2021 15:42:41 -0700 Subject: [PATCH 02/14] Flow downwards --- .../Services/SolutionServiceTests.cs | 3 --- .../Portable/Remote/PinnedSolutionInfo.cs | 17 +++++++++--- .../Solution/MetadataOnlyReference.cs | 26 +++++-------------- ...eneratedFileReplacingCompilationTracker.cs | 10 +++---- .../Remote/Core/SolutionAssetStorage.cs | 1 + 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/src/VisualStudio/Core/Test.Next/Services/SolutionServiceTests.cs b/src/VisualStudio/Core/Test.Next/Services/SolutionServiceTests.cs index a910e0fdb3f74..9bbd8a6a68365 100644 --- a/src/VisualStudio/Core/Test.Next/Services/SolutionServiceTests.cs +++ b/src/VisualStudio/Core/Test.Next/Services/SolutionServiceTests.cs @@ -773,7 +773,6 @@ private static async Task VerifySolutionUpdate( Assert.IsAssignableFrom(recoveredSolution.Workspace); var primaryWorkspace = recoveredSolution.Workspace; Assert.Equal(solutionChecksum, await recoveredSolution.State.GetChecksumAsync(CancellationToken.None)); - Assert.Same(primaryWorkspace.PrimaryBranchId, recoveredSolution.BranchId); // get new solution var newSolution = newSolutionGetter(solution); @@ -784,14 +783,12 @@ private static async Task VerifySolutionUpdate( var recoveredNewSolution = await remoteWorkspace.GetSolutionAsync(assetProvider, newSolutionChecksum, fromPrimaryBranch: false, workspaceVersion: -1, projectId: null, CancellationToken.None); Assert.Equal(newSolutionChecksum, await recoveredNewSolution.State.GetChecksumAsync(CancellationToken.None)); - Assert.NotSame(primaryWorkspace.PrimaryBranchId, recoveredNewSolution.BranchId); // do same once updating primary workspace await remoteWorkspace.UpdatePrimaryBranchSolutionAsync(assetProvider, newSolutionChecksum, solution.WorkspaceVersion + 1, CancellationToken.None); var third = await remoteWorkspace.GetSolutionAsync(assetProvider, newSolutionChecksum, fromPrimaryBranch: false, workspaceVersion: -1, projectId: null, CancellationToken.None); Assert.Equal(newSolutionChecksum, await third.State.GetChecksumAsync(CancellationToken.None)); - Assert.Same(primaryWorkspace.PrimaryBranchId, third.BranchId); newSolutionValidator?.Invoke(recoveredNewSolution); } diff --git a/src/Workspaces/Core/Portable/Remote/PinnedSolutionInfo.cs b/src/Workspaces/Core/Portable/Remote/PinnedSolutionInfo.cs index a93b96781fa46..471b6723e1d31 100644 --- a/src/Workspaces/Core/Portable/Remote/PinnedSolutionInfo.cs +++ b/src/Workspaces/Core/Portable/Remote/PinnedSolutionInfo.cs @@ -20,30 +20,41 @@ internal sealed class PinnedSolutionInfo [DataMember(Order = 0)] public readonly int ScopeId; + /// + /// This indicates whether this scope is for primary branch or not (not forked solution) + /// + /// Features like OOP will use this flag to see whether caching information related to this solution + /// can benefit other requests or not + /// + [DataMember(Order = 1)] + public readonly bool FromPrimaryBranch; + /// /// This indicates a Solution.WorkspaceVersion of this solution. remote host engine uses this version /// to decide whether caching this solution will benefit other requests or not /// - [DataMember(Order = 1)] + [DataMember(Order = 2)] public readonly int WorkspaceVersion; - [DataMember(Order = 2)] + [DataMember(Order = 3)] public readonly Checksum SolutionChecksum; /// /// An optional project that we are pinning information for. This is used for features that only need /// information for a project (and its dependencies) and not the entire solution. /// - [DataMember(Order = 3)] + [DataMember(Order = 4)] public readonly ProjectId? ProjectId; public PinnedSolutionInfo( int scopeId, + bool fromPrimaryBranch, int workspaceVersion, Checksum solutionChecksum, ProjectId? projectId) { ScopeId = scopeId; + FromPrimaryBranch = fromPrimaryBranch; WorkspaceVersion = workspaceVersion; SolutionChecksum = solutionChecksum; ProjectId = projectId; diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs b/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs index 5db64f0829788..32d4cd41006a2 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs @@ -17,15 +17,12 @@ namespace Microsoft.CodeAnalysis internal class MetadataOnlyReference { // version based cache - private static readonly ConditionalWeakTable> s_cache - = new(); + private static readonly ConditionalWeakTable> s_cache = new(); // snapshot based cache - private static readonly ConditionalWeakTable s_snapshotCache - = new(); + private static readonly ConditionalWeakTable s_snapshotCache = new(); - private static readonly ConditionalWeakTable>.CreateValueCallback s_createReferenceSetMap = - _ => new ConditionalWeakTable(); + private static readonly ConditionalWeakTable>.CreateValueCallback s_createReferenceSetMap = _ => new(); internal static MetadataReference GetOrBuildReference( SolutionState solution, @@ -64,7 +61,7 @@ internal static MetadataReference GetOrBuildReference( // okay, proceed with whatever image we have // now, remove existing set - var mapFromBranch = s_cache.GetValue(solution.BranchId, s_createReferenceSetMap); + var mapFromBranch = s_cache.GetValue(solution, s_createReferenceSetMap); mapFromBranch.Remove(projectReference.ProjectId); // create new one @@ -106,31 +103,22 @@ internal static bool TryGetReference( // okay, now use version based cache that can live multiple compilation as long as there is no semantic changes. // get one for the branch - if (TryGetReferenceFromBranch(solution.BranchId, projectReference, finalOrDeclarationCompilation, version, out reference)) + if (TryGetReferenceFromBranch(solution, projectReference, finalOrDeclarationCompilation, version, out reference)) { solution.Workspace.LogTestMessage($"Found already cached metadata for the branch and version {version}"); return true; } - // see whether we can use primary branch one - var primaryBranchId = solution.Workspace.PrimaryBranchId; - if (solution.BranchId != primaryBranchId && - TryGetReferenceFromBranch(primaryBranchId, projectReference, finalOrDeclarationCompilation, version, out reference)) - { - solution.Workspace.LogTestMessage($"Found already cached metadata for the primary branch and version {version}"); - return true; - } - // noop, we don't have any reference = null; return false; } private static bool TryGetReferenceFromBranch( - BranchId branchId, ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp version, out MetadataReference reference) + SolutionState state, ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp version, out MetadataReference reference) { // get map for the branch - var mapFromBranch = s_cache.GetValue(branchId, s_createReferenceSetMap); + var mapFromBranch = s_cache.GetValue(state, s_createReferenceSetMap); // if we have one, return it if (mapFromBranch.TryGetValue(projectReference.ProjectId, out var referenceSet) && (version == VersionStamp.Default || referenceSet.Version == version)) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs index e24a52cbebd7c..354d2ee4d74a2 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs @@ -136,14 +136,10 @@ public async Task GetMetadataReferenceAsync(SolutionState sol // If it's the same language we can just make a CompilationReference if (this.ProjectState.LanguageServices == fromProject.LanguageServices) - { return compilation.ToMetadataReference(projectReference.Aliases, projectReference.EmbedInteropTypes); - } - else - { - var version = await GetDependentSemanticVersionAsync(solution, cancellationToken).ConfigureAwait(false); - return MetadataOnlyReference.GetOrBuildReference(solution, projectReference, compilation, version, cancellationToken); - } + + var version = await GetDependentSemanticVersionAsync(solution, cancellationToken).ConfigureAwait(false); + return MetadataOnlyReference.GetOrBuildReference(solution, projectReference, compilation, version, cancellationToken); } public CompilationReference? GetPartialMetadataReference(ProjectState fromProject, ProjectReference projectReference) diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs index 351a63ee882bf..94125b1473616 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs @@ -53,6 +53,7 @@ private async ValueTask StoreAssetsAsync(Solution solution, ProjectId? pr var id = Interlocked.Increment(ref s_scopeId); var solutionInfo = new PinnedSolutionInfo( id, + fromPrimaryBranch: solution == solution.Workspace.CurrentSolution, solutionState.WorkspaceVersion, solutionChecksum, projectId); From d6001fc44df33f14bbf36b5c17d07136ec09f70e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Oct 2021 16:04:26 -0700 Subject: [PATCH 03/14] inline --- .../Workspace/CompileTimeSolutionProvider.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs b/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs index 35fef13368e68..3c15f6050a98a 100644 --- a/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs +++ b/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs @@ -52,6 +52,8 @@ public Factory() #if NETCOREAPP private readonly ConditionalWeakTable _designTimeToCompileTimeSoution = new(); #else + // Framework lacks both a .Clear() method. So for Framework we simulate that by just overwriting this with a + // new instance. This happens under a lock, so everyone sees a consistent dictionary. private ConditionalWeakTable _designTimeToCompileTimeSoution = new(); #endif @@ -120,22 +122,14 @@ public Solution GetCompileTimeSolution(Solution designTimeSolution) .RemoveAnalyzerConfigDocuments(configIdsToRemove.ToImmutable()) .RemoveDocuments(documentIdsToRemove.ToImmutable()); - UpdateCachedCompileTimeSolution(designTimeSolution, compileTimeSolution); - - return compileTimeSolution; - } - } - - private void UpdateCachedCompileTimeSolution(Solution designTimeSolution, Solution compileTimeSolution) - { - lock (_gate) - { #if NETCOREAPP _designTimeToCompileTimeSoution.AddOrUpdate(designTimeSolution, compileTimeSolution); #else _designTimeToCompileTimeSoution.Remove(designTimeSolution); _designTimeToCompileTimeSoution.Add(designTimeSolution, compileTimeSolution); #endif + + return compileTimeSolution; } } From 681215529e73aa4c582f1a418a932495e8ee6837 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Oct 2021 16:13:25 -0700 Subject: [PATCH 04/14] revert --- ...onState.GeneratedFileReplacingCompilationTracker.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs index 354d2ee4d74a2..e24a52cbebd7c 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs @@ -136,10 +136,14 @@ public async Task GetMetadataReferenceAsync(SolutionState sol // If it's the same language we can just make a CompilationReference if (this.ProjectState.LanguageServices == fromProject.LanguageServices) + { return compilation.ToMetadataReference(projectReference.Aliases, projectReference.EmbedInteropTypes); - - var version = await GetDependentSemanticVersionAsync(solution, cancellationToken).ConfigureAwait(false); - return MetadataOnlyReference.GetOrBuildReference(solution, projectReference, compilation, version, cancellationToken); + } + else + { + var version = await GetDependentSemanticVersionAsync(solution, cancellationToken).ConfigureAwait(false); + return MetadataOnlyReference.GetOrBuildReference(solution, projectReference, compilation, version, cancellationToken); + } } public CompilationReference? GetPartialMetadataReference(ProjectState fromProject, ProjectReference projectReference) From d1f781cfff20d6b4e6a121e49f21389062de655e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Oct 2021 16:31:18 -0700 Subject: [PATCH 05/14] Delete validaiton --- .../Core/Portable/Workspace/Workspace.cs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index a5be90fcee5e8..cbdee6ae54234 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -1191,13 +1191,8 @@ internal virtual bool TryApplyChanges(Solution newSolution, IProgressTracker pro // make sure that newSolution is a branch of the current solution - // the given solution must be a branched one. - // otherwise, there should be no change to apply. if (oldSolution == newSolution) - { - CheckNoChanges(oldSolution, newSolution); return true; - } var solutionChanges = newSolution.GetChanges(oldSolution); this.CheckAllowedSolutionChanges(solutionChanges); @@ -1622,15 +1617,6 @@ private void ApplyChangedDocument( } } - [Conditional("DEBUG")] - private static void CheckNoChanges(Solution oldSolution, Solution newSolution) - { - var changes = newSolution.GetChanges(oldSolution); - Contract.ThrowIfTrue(changes.GetAddedProjects().Any()); - Contract.ThrowIfTrue(changes.GetRemovedProjects().Any()); - Contract.ThrowIfTrue(changes.GetProjectChanges().Any()); - } - private static ProjectInfo CreateProjectInfo(Project project) { return ProjectInfo.Create( From 9aae5f77b4d3de56dbef2e8dfd38373bd6804fc5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Oct 2021 16:39:27 -0700 Subject: [PATCH 06/14] Simplify --- .../Solution/MetadataOnlyReference.cs | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs b/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs index 32d4cd41006a2..07bc3055195d7 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs @@ -17,12 +17,10 @@ namespace Microsoft.CodeAnalysis internal class MetadataOnlyReference { // version based cache - private static readonly ConditionalWeakTable> s_cache = new(); + private static readonly ConditionalWeakTable s_projectIdToReferenceMap = new(); // snapshot based cache - private static readonly ConditionalWeakTable s_snapshotCache = new(); - - private static readonly ConditionalWeakTable>.CreateValueCallback s_createReferenceSetMap = _ => new(); + private static readonly ConditionalWeakTable s_compilationToReferenceMap = new(); internal static MetadataReference GetOrBuildReference( SolutionState solution, @@ -61,12 +59,11 @@ internal static MetadataReference GetOrBuildReference( // okay, proceed with whatever image we have // now, remove existing set - var mapFromBranch = s_cache.GetValue(solution, s_createReferenceSetMap); - mapFromBranch.Remove(projectReference.ProjectId); + s_projectIdToReferenceMap.Remove(projectReference.ProjectId); // create new one var newReferenceSet = new MetadataOnlyReferenceSet(version, image); - var referenceSet = s_snapshotCache.GetValue(finalCompilation, _ => newReferenceSet); + var referenceSet = s_compilationToReferenceMap.GetValue(finalCompilation, _ => newReferenceSet); if (newReferenceSet != referenceSet) { // someone else has beaten us. @@ -83,7 +80,15 @@ internal static MetadataReference GetOrBuildReference( // record it to version based cache as well. snapshot cache always has a higher priority. we don't need to check returned set here // since snapshot based cache will take care of same compilation for us. - mapFromBranch.GetValue(projectReference.ProjectId, _ => referenceSet); +#if NETCOREAPP + s_projectIdToReferenceMap.AddOrUpdate(projectReference.ProjectId, referenceSet); +#else + lock (s_projectIdToReferenceMap) + { + s_projectIdToReferenceMap.Remove(projectReference.ProjectId); + s_projectIdToReferenceMap.Add(projectReference.ProjectId, referenceSet); + } +#endif // return new reference return referenceSet.GetMetadataReference(finalCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes); @@ -93,16 +98,15 @@ internal static bool TryGetReference( SolutionState solution, ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp version, out MetadataReference reference) { // if we have one from snapshot cache, use it. it will make sure same compilation will get same metadata reference always. - if (s_snapshotCache.TryGetValue(finalOrDeclarationCompilation, out var referenceSet)) + if (s_compilationToReferenceMap.TryGetValue(finalOrDeclarationCompilation, out var referenceSet)) { - solution.Workspace.LogTestMessage($"Found already cached metadata in {nameof(s_snapshotCache)} for the exact compilation"); + solution.Workspace.LogTestMessage($"Found already cached metadata in {nameof(s_compilationToReferenceMap)} for the exact compilation"); reference = referenceSet.GetMetadataReference(finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes); return true; } // okay, now use version based cache that can live multiple compilation as long as there is no semantic changes. - // get one for the branch if (TryGetReferenceFromBranch(solution, projectReference, finalOrDeclarationCompilation, version, out reference)) { solution.Workspace.LogTestMessage($"Found already cached metadata for the branch and version {version}"); @@ -117,14 +121,11 @@ internal static bool TryGetReference( private static bool TryGetReferenceFromBranch( SolutionState state, ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp version, out MetadataReference reference) { - // get map for the branch - var mapFromBranch = s_cache.GetValue(state, s_createReferenceSetMap); - // if we have one, return it - if (mapFromBranch.TryGetValue(projectReference.ProjectId, out var referenceSet) && + if (s_projectIdToReferenceMap.TryGetValue(projectReference.ProjectId, out var referenceSet) && (version == VersionStamp.Default || referenceSet.Version == version)) { // record it to snapshot based cache. - var newReferenceSet = s_snapshotCache.GetValue(finalOrDeclarationCompilation, _ => referenceSet); + var newReferenceSet = s_compilationToReferenceMap.GetValue(finalOrDeclarationCompilation, _ => referenceSet); reference = newReferenceSet.GetMetadataReference(finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes); return true; From 7782d4b24905b649e9bfb392797c02b8c5cf3f41 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Oct 2021 16:41:43 -0700 Subject: [PATCH 07/14] Fixup --- .../Portable/Workspace/Solution/MetadataOnlyReference.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs b/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs index 07bc3055195d7..878b6453c6576 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs @@ -107,7 +107,7 @@ internal static bool TryGetReference( // okay, now use version based cache that can live multiple compilation as long as there is no semantic changes. - if (TryGetReferenceFromBranch(solution, projectReference, finalOrDeclarationCompilation, version, out reference)) + if (TryGetReference(projectReference, finalOrDeclarationCompilation, version, out reference)) { solution.Workspace.LogTestMessage($"Found already cached metadata for the branch and version {version}"); return true; @@ -118,8 +118,8 @@ internal static bool TryGetReference( return false; } - private static bool TryGetReferenceFromBranch( - SolutionState state, ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp version, out MetadataReference reference) + private static bool TryGetReference( + ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp version, out MetadataReference reference) { if (s_projectIdToReferenceMap.TryGetValue(projectReference.ProjectId, out var referenceSet) && (version == VersionStamp.Default || referenceSet.Version == version)) From effa0bfcf71d3db4a393703406c9d79d05050358 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Oct 2021 17:31:00 -0700 Subject: [PATCH 08/14] moved cached metadata refs to instance state --- .../Solution/MetadataOnlyReference.cs | 169 ++++++++++++------ .../SolutionState.CompilationTracker.cs | 20 ++- ...eneratedFileReplacingCompilationTracker.cs | 5 +- .../SolutionState.ICompilationTracker.cs | 1 + 4 files changed, 136 insertions(+), 59 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs b/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs index 878b6453c6576..d7d27f2d1aa19 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs @@ -14,23 +14,70 @@ namespace Microsoft.CodeAnalysis { - internal class MetadataOnlyReference + /// + /// Caches the skeleton references produced for a given project/compilation under the varying + /// it might be referenced by. Skeletons are used in the compilation + /// tracker to allow cross-language project references with live semantic updating between VB/C# and vice versa. + /// Specifically, in a cross language case we will build a skeleton ref for the referenced project and have the + /// referrer use that to understand its semantics. + /// + /// This approach works, but has the caveat that live cross language semantics are only possible when the + /// skeleton assembly can be built. This should always be the case for correct code, but it may not be the + /// case for code with errors depending on if the respective language compilat is unable to generate the skeleton + /// in the presence of those errors. In that case though, this type provides mechanisms to fallback to the last + /// successfully built skeleton so that a somewhat reasonable experience can be maintained. If we failed to do this + /// and instead returned nothing, a user would find that practically all semantic experiences that depended on + /// that particular project would fail or be seriously degraded (e.g. diagnostics). To that end, it's better to + /// limp along with stale date, then barrel on ahead with no data. + /// + /// The implementation works by keeping a mapping from to the the metadata references + /// for that project. As long as the for that project + /// is the same, then all the references of it can be reused. When the compilation tracker forks itself, it + /// will also fork this, allow previously computed references to be used by later forks. However, this means + /// that later forks (esp. ones that fail to produce a skeleton, or which produce a skeleton for different + /// semantics) will not leak backward to a prior compilation tracker point, causing it to see a view of the world + /// inapplicable to its current snapshot. + /// + internal class CachedSkeletonReferences { - // version based cache - private static readonly ConditionalWeakTable s_projectIdToReferenceMap = new(); - - // snapshot based cache + /// + /// Mapping from compilation instance to metadata-references for it. Safe to use as a static CWT as anyone + /// with a reference to this compilation (and the same is allowed + /// to reference it using the same . + /// private static readonly ConditionalWeakTable s_compilationToReferenceMap = new(); - internal static MetadataReference GetOrBuildReference( + private readonly object _gate = new(); + + /// + /// Mapping from to the skeleton s for it. + /// + private ImmutableDictionary _projectIdToReferenceMap; + + public static readonly CachedSkeletonReferences Empty = new(ImmutableDictionary.Empty); + + private CachedSkeletonReferences( + ImmutableDictionary projectIdToReferenceMap) + { + _projectIdToReferenceMap = projectIdToReferenceMap; + } + + public CachedSkeletonReferences Clone() + => new(_projectIdToReferenceMap); + + internal MetadataReference GetOrBuildReference( SolutionState solution, ProjectReference projectReference, Compilation finalCompilation, VersionStamp version, CancellationToken cancellationToken) { + // First see if we already have a cached reference for either finalCompilation or for projectReference. + // If we have one for the latter, we'll make sure that it's version matches what we're asking for before + // returning it. solution.Workspace.LogTestMessage($"Looking to see if we already have a skeleton assembly for {projectReference.ProjectId} before we build one..."); - if (TryGetReference(solution, projectReference, finalCompilation, version, out var reference)) + if (TryGetReference( + solution, projectReference, finalCompilation, version, cancellationToken, out var reference)) { solution.Workspace.LogTestMessage($"A reference was found {projectReference.ProjectId} so we're skipping the build."); return reference; @@ -45,23 +92,16 @@ internal static MetadataReference GetOrBuildReference( if (image.IsEmpty) { - // unfortunately, we couldn't create one. do best effort - if (TryGetReference(solution, projectReference, finalCompilation, VersionStamp.Default, out reference)) + // unfortunately, we couldn't create one. see if we have one from previous compilation., it might be + // out-of-date big time, but better than nothing. + if (TryGetReference( + solution, projectReference, finalCompilation, version: null, cancellationToken, out reference)) { solution.Workspace.LogTestMessage($"We failed to create metadata so we're using the one we just found from an earlier version."); - - // we have one from previous compilation!!, it might be out-of-date big time, but better than nothing. - // re-use it return reference; } } - // okay, proceed with whatever image we have - - // now, remove existing set - s_projectIdToReferenceMap.Remove(projectReference.ProjectId); - - // create new one var newReferenceSet = new MetadataOnlyReferenceSet(version, image); var referenceSet = s_compilationToReferenceMap.GetValue(finalCompilation, _ => newReferenceSet); if (newReferenceSet != referenceSet) @@ -71,7 +111,8 @@ internal static MetadataReference GetOrBuildReference( image.Cleanup(); // return new reference - return referenceSet.GetMetadataReference(finalCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes); + return referenceSet.GetMetadataReference( + finalCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken); } else { @@ -80,34 +121,61 @@ internal static MetadataReference GetOrBuildReference( // record it to version based cache as well. snapshot cache always has a higher priority. we don't need to check returned set here // since snapshot based cache will take care of same compilation for us. -#if NETCOREAPP - s_projectIdToReferenceMap.AddOrUpdate(projectReference.ProjectId, referenceSet); -#else - lock (s_projectIdToReferenceMap) + + lock (_gate) { - s_projectIdToReferenceMap.Remove(projectReference.ProjectId); - s_projectIdToReferenceMap.Add(projectReference.ProjectId, referenceSet); + _projectIdToReferenceMap = _projectIdToReferenceMap.Remove(projectReference.ProjectId); + _projectIdToReferenceMap = _projectIdToReferenceMap.Add(projectReference.ProjectId, referenceSet); } -#endif // return new reference - return referenceSet.GetMetadataReference(finalCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes); + return referenceSet.GetMetadataReference( + finalCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken); } - internal static bool TryGetReference( - SolutionState solution, ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp version, out MetadataReference reference) + /// + /// Tries to get the associated with the provided + /// that produced the . + /// + internal bool TryGetReference( + SolutionState solution, + ProjectReference projectReference, + Compilation finalOrDeclarationCompilation, + VersionStamp version, + CancellationToken cancellationToken, + out MetadataReference reference) + { + return TryGetReference(solution, projectReference, finalOrDeclarationCompilation, (VersionStamp?)version, cancellationToken, out reference); + } + + /// + /// . + /// If is , any for that + /// project may be returned, even if it doesn't correspond to that compilation. This is useful in error tolerance + /// cases as building a skeleton assembly may easily fail. In that case it's better to use the last successfully + /// built skeleton than just have no semantic information for that project at all. + /// + private bool TryGetReference( + SolutionState solution, + ProjectReference projectReference, + Compilation finalOrDeclarationCompilation, + VersionStamp? version, + CancellationToken cancellationToken, + out MetadataReference reference) { // if we have one from snapshot cache, use it. it will make sure same compilation will get same metadata reference always. if (s_compilationToReferenceMap.TryGetValue(finalOrDeclarationCompilation, out var referenceSet)) { solution.Workspace.LogTestMessage($"Found already cached metadata in {nameof(s_compilationToReferenceMap)} for the exact compilation"); - reference = referenceSet.GetMetadataReference(finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes); + reference = referenceSet.GetMetadataReference( + finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken); return true; } // okay, now use version based cache that can live multiple compilation as long as there is no semantic changes. - if (TryGetReference(projectReference, finalOrDeclarationCompilation, version, out reference)) + if (TryGetReference( + projectReference, finalOrDeclarationCompilation, version, cancellationToken, out reference)) { solution.Workspace.LogTestMessage($"Found already cached metadata for the branch and version {version}"); return true; @@ -118,16 +186,20 @@ internal static bool TryGetReference( return false; } - private static bool TryGetReference( - ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp version, out MetadataReference reference) + private bool TryGetReference( + ProjectReference projectReference, + Compilation finalOrDeclarationCompilation, + VersionStamp? version, + CancellationToken cancellationToken, + out MetadataReference reference) { - if (s_projectIdToReferenceMap.TryGetValue(projectReference.ProjectId, out var referenceSet) && - (version == VersionStamp.Default || referenceSet.Version == version)) + if (_projectIdToReferenceMap.TryGetValue(projectReference.ProjectId, out var referenceSet) && + (version == null || referenceSet.Version == version)) { // record it to snapshot based cache. var newReferenceSet = s_compilationToReferenceMap.GetValue(finalOrDeclarationCompilation, _ => referenceSet); - - reference = newReferenceSet.GetMetadataReference(finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes); + reference = newReferenceSet.GetMetadataReference( + finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken); return true; } @@ -140,31 +212,26 @@ private class MetadataOnlyReferenceSet // use WeakReference so we don't keep MetadataReference's alive if they are not being consumed private readonly NonReentrantLock _gate = new(useThisInstanceForSynchronization: true); - // here, there is a very small chance of leaking Tuple and WeakReference, but it is so small chance, - // I don't believe it will actually happen in real life situation. basically, for leak to happen, - // every image creation except the first one has to fail so that we end up re-use old reference set. - // and the user creates many different metadata references with multiple combination of the key (tuple). - private readonly Dictionary> _metadataReferences - = new(); - - private readonly VersionStamp _version; + private readonly Dictionary> _metadataReferences = new(); private readonly MetadataOnlyImage _image; + public readonly VersionStamp Version; + public MetadataOnlyReferenceSet(VersionStamp version, MetadataOnlyImage image) { - _version = version; + Version = version; _image = image; } - public VersionStamp Version => _version; - - public MetadataReference GetMetadataReference(Compilation compilation, ImmutableArray aliases, bool embedInteropTypes) + public MetadataReference GetMetadataReference( + Compilation compilation, ImmutableArray aliases, bool embedInteropTypes, CancellationToken cancellationToken) { var key = new MetadataReferenceProperties(MetadataImageKind.Assembly, aliases, embedInteropTypes); - using (_gate.DisposableWait()) + using (_gate.DisposableWait(cancellationToken)) { - if (!_metadataReferences.TryGetValue(key, out var weakMetadata) || !weakMetadata.TryGetTarget(out var metadataReference)) + if (!_metadataReferences.TryGetValue(key, out var weakMetadata) || + !weakMetadata.TryGetTarget(out var metadataReference)) { // here we give out strong reference to compilation. so there is possibility that we end up making 2 compilations for same project alive. // one for final compilation and one for declaration only compilation. but the final compilation will be eventually kicked out from compilation cache diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs index 4d61e7f63c77c..199c6f58e70e5 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs @@ -44,14 +44,18 @@ private partial class CompilationTracker : ICompilationTracker // guarantees only one thread is building at a time private readonly SemaphoreSlim _buildLock = new(initialCount: 1); + public CachedSkeletonReferences CachedSkeletonReferences { get; } + private CompilationTracker( ProjectState project, - CompilationTrackerState state) + CompilationTrackerState state, + CachedSkeletonReferences cachedSkeletonReferences) { Contract.ThrowIfNull(project); this.ProjectState = project; _stateDoNotAccessDirectly = state; + CachedSkeletonReferences = cachedSkeletonReferences; } /// @@ -59,7 +63,7 @@ private CompilationTracker( /// and will have no extra information beyond the project itself. /// public CompilationTracker(ProjectState project) - : this(project, CompilationTrackerState.Empty) + : this(project, CompilationTrackerState.Empty, CachedSkeletonReferences.Empty) { } @@ -143,13 +147,13 @@ public ICompilationTracker Fork( var newState = CompilationTrackerState.Create( solutionServices, baseCompilation, state.GeneratorInfo, state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken), intermediateProjects); - return new CompilationTracker(newProject, newState); + return new CompilationTracker(newProject, newState, this.CachedSkeletonReferences.Clone()); } else { // We have no compilation, but we might have information about generated docs. var newState = new NoCompilationState(state.GeneratorInfo.WithDocumentsAreFinal(false)); - return new CompilationTracker(newProject, newState); + return new CompilationTracker(newProject, newState, this.CachedSkeletonReferences.Clone()); } } @@ -193,7 +197,7 @@ public ICompilationTracker FreezePartialStateWithTree(SolutionState solution, Do this.ProjectState.Id, metadataReferenceToProjectId); - return new CompilationTracker(inProgressProject, finalState); + return new CompilationTracker(inProgressProject, finalState, this.CachedSkeletonReferences.Clone()); } /// @@ -1012,7 +1016,8 @@ private async Task GetMetadataOnlyImageReferenceAsync( var declarationCompilation = await this.GetOrBuildDeclarationCompilationAsync(solution.Services, cancellationToken: cancellationToken).ConfigureAwait(false); solution.Workspace.LogTestMessage($"Looking for a cached skeleton assembly for {projectReference.ProjectId} before taking the lock..."); - if (!MetadataOnlyReference.TryGetReference(solution, projectReference, declarationCompilation, version, out var reference)) + if (!this.CachedSkeletonReferences.TryGetReference( + solution, projectReference, declarationCompilation, version, cancellationToken, out var reference)) { // using async build lock so we don't get multiple consumers attempting to build metadata-only images for the same compilation. using (await _buildLock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) @@ -1021,7 +1026,8 @@ private async Task GetMetadataOnlyImageReferenceAsync( // okay, we still don't have one. bring the compilation to final state since we are going to use it to create skeleton assembly var compilationInfo = await this.GetOrBuildCompilationInfoAsync(solution, lockGate: false, cancellationToken: cancellationToken).ConfigureAwait(false); - reference = MetadataOnlyReference.GetOrBuildReference(solution, projectReference, compilationInfo.Compilation, version, cancellationToken); + reference = this.CachedSkeletonReferences.GetOrBuildReference( + solution, projectReference, compilationInfo.Compilation, version, cancellationToken); } } else diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs index e24a52cbebd7c..1aba5cd6824f2 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs @@ -38,6 +38,7 @@ public GeneratedFileReplacingCompilationTracker(ICompilationTracker underlyingTr } public ProjectState ProjectState => _underlyingTracker.ProjectState; + public CachedSkeletonReferences CachedSkeletonReferences => _underlyingTracker.CachedSkeletonReferences; public bool ContainsAssemblyOrModuleOrDynamic(ISymbol symbol, bool primary) { @@ -141,8 +142,10 @@ public async Task GetMetadataReferenceAsync(SolutionState sol } else { + // Otherwise we need to create a skeleton for this project. See if we can reuse an existing + // one, or create a new one when we can't. var version = await GetDependentSemanticVersionAsync(solution, cancellationToken).ConfigureAwait(false); - return MetadataOnlyReference.GetOrBuildReference(solution, projectReference, compilation, version, cancellationToken); + return _underlyingTracker.CachedSkeletonReferences.GetOrBuildReference(solution, projectReference, compilation, version, cancellationToken); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.ICompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.ICompilationTracker.cs index 0f5ae213cbe9d..e40cab60c7c87 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.ICompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.ICompilationTracker.cs @@ -13,6 +13,7 @@ internal partial class SolutionState private interface ICompilationTracker { ProjectState ProjectState { get; } + CachedSkeletonReferences CachedSkeletonReferences { get; } /// /// Returns if this / could produce the From d83c68640bd228956437648d46178599c8a67c21 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 Oct 2021 12:10:12 -0700 Subject: [PATCH 09/14] Temp --- ...ference.cs => CachedSkeletonReferences.cs} | 147 +++++++++--------- .../Workspace/Solution/ProjectState.cs | 28 +++- .../SolutionState.CompilationTracker.cs | 18 +-- ...eneratedFileReplacingCompilationTracker.cs | 3 +- .../SolutionState.ICompilationTracker.cs | 1 - 5 files changed, 107 insertions(+), 90 deletions(-) rename src/Workspaces/Core/Portable/Workspace/Solution/{MetadataOnlyReference.cs => CachedSkeletonReferences.cs} (69%) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs b/src/Workspaces/Core/Portable/Workspace/Solution/CachedSkeletonReferences.cs similarity index 69% rename from src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs rename to src/Workspaces/Core/Portable/Workspace/Solution/CachedSkeletonReferences.cs index d7d27f2d1aa19..ba8531e8ec736 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/MetadataOnlyReference.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/CachedSkeletonReferences.cs @@ -2,13 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Runtime.CompilerServices; using System.Threading; +using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; using Roslyn.Utilities; @@ -45,27 +44,44 @@ internal class CachedSkeletonReferences /// with a reference to this compilation (and the same is allowed /// to reference it using the same . /// - private static readonly ConditionalWeakTable s_compilationToReferenceMap = new(); + private static readonly ConditionalWeakTable s_compilationToReferenceMap = new(); + + private readonly SemaphoreSlim _gate = new(initialCount: 1); - private readonly object _gate = new(); + /// + /// The version of the project that these correspond to. + /// + private VersionStamp? _version; /// - /// Mapping from to the skeleton s for it. + /// Mapping from metadata-reference-properties to the actual metadata reference for them. /// - private ImmutableDictionary _projectIdToReferenceMap; + private SkeletonReferenceSet? _skeletonReferenceSet; - public static readonly CachedSkeletonReferences Empty = new(ImmutableDictionary.Empty); + public static readonly CachedSkeletonReferences Empty = + new(version: null, skeletonReferenceSet: null); private CachedSkeletonReferences( - ImmutableDictionary projectIdToReferenceMap) + VersionStamp? version, + SkeletonReferenceSet? skeletonReferenceSet) { - _projectIdToReferenceMap = projectIdToReferenceMap; + _version = version; + _skeletonReferenceSet = skeletonReferenceSet; } public CachedSkeletonReferences Clone() - => new(_projectIdToReferenceMap); + { + lock (_gate) + { + // pass along the best version/reference-set we computed for ourselves. That way future ProjectStates + // can use this data if either the version changed, or they weren't able to build a skeleton for themselves. + // By passing along a copy we ensure that if they have a different version, they'll end up producing a new + // SkeletonReferenceSet where they'll store their own data in which will not affect prior ProjectStates. + return new CachedSkeletonReferences(_version, _skeletonReferenceSet); + } + } - internal MetadataReference GetOrBuildReference( + public async Task GetOrBuildReferenceAsync( SolutionState solution, ProjectReference projectReference, Compilation finalCompilation, @@ -76,8 +92,8 @@ internal MetadataReference GetOrBuildReference( // If we have one for the latter, we'll make sure that it's version matches what we're asking for before // returning it. solution.Workspace.LogTestMessage($"Looking to see if we already have a skeleton assembly for {projectReference.ProjectId} before we build one..."); - if (TryGetReference( - solution, projectReference, finalCompilation, version, cancellationToken, out var reference)) + var reference = await TryGetReferenceAsync(solution, projectReference, finalCompilation, version, cancellationToken).ConfigureAwait(false); + if (reference != null) { solution.Workspace.LogTestMessage($"A reference was found {projectReference.ProjectId} so we're skipping the build."); return reference; @@ -94,141 +110,122 @@ internal MetadataReference GetOrBuildReference( { // unfortunately, we couldn't create one. see if we have one from previous compilation., it might be // out-of-date big time, but better than nothing. - if (TryGetReference( - solution, projectReference, finalCompilation, version: null, cancellationToken, out reference)) + reference = await TryGetReferenceAsync(solution, projectReference, finalCompilation, version: null, cancellationToken).ConfigureAwait(false); + if (reference != null) { solution.Workspace.LogTestMessage($"We failed to create metadata so we're using the one we just found from an earlier version."); return reference; } } - var newReferenceSet = new MetadataOnlyReferenceSet(version, image); - var referenceSet = s_compilationToReferenceMap.GetValue(finalCompilation, _ => newReferenceSet); - if (newReferenceSet != referenceSet) + var tempReferenceSet = new SkeletonReferenceSet(image); + var referenceSet = s_compilationToReferenceMap.GetValue(finalCompilation, _ => tempReferenceSet); + if (tempReferenceSet != referenceSet) { // someone else has beaten us. // let image go eagerly. otherwise, finalizer in temporary storage will take care of it image.Cleanup(); - - // return new reference - return referenceSet.GetMetadataReference( - finalCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken); } - else - { - solution.Workspace.LogTestMessage($"Successfully stored the metadata generated for {projectReference.ProjectId}"); - } - - // record it to version based cache as well. snapshot cache always has a higher priority. we don't need to check returned set here - // since snapshot based cache will take care of same compilation for us. - lock (_gate) + await (_gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { - _projectIdToReferenceMap = _projectIdToReferenceMap.Remove(projectReference.ProjectId); - _projectIdToReferenceMap = _projectIdToReferenceMap.Add(projectReference.ProjectId, referenceSet); + _version = version; + _skeletonReferenceSet = referenceSet; } - // return new reference - return referenceSet.GetMetadataReference( - finalCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken); + return await referenceSet.GetMetadataReferenceAsync( + finalCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken).ConfigureAwait(false); } /// /// Tries to get the associated with the provided /// that produced the . /// - internal bool TryGetReference( + public Task TryGetReferenceAsync( SolutionState solution, ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp version, - CancellationToken cancellationToken, - out MetadataReference reference) + CancellationToken cancellationToken) { - return TryGetReference(solution, projectReference, finalOrDeclarationCompilation, (VersionStamp?)version, cancellationToken, out reference); + return TryGetReferenceAsync(solution, projectReference, finalOrDeclarationCompilation, (VersionStamp?)version, cancellationToken); } /// - /// . + /// . /// If is , any for that /// project may be returned, even if it doesn't correspond to that compilation. This is useful in error tolerance /// cases as building a skeleton assembly may easily fail. In that case it's better to use the last successfully /// built skeleton than just have no semantic information for that project at all. /// - private bool TryGetReference( + private async Task TryGetReferenceAsync( SolutionState solution, ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp? version, - CancellationToken cancellationToken, - out MetadataReference reference) + CancellationToken cancellationToken) { // if we have one from snapshot cache, use it. it will make sure same compilation will get same metadata reference always. if (s_compilationToReferenceMap.TryGetValue(finalOrDeclarationCompilation, out var referenceSet)) { solution.Workspace.LogTestMessage($"Found already cached metadata in {nameof(s_compilationToReferenceMap)} for the exact compilation"); - reference = referenceSet.GetMetadataReference( - finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken); - return true; + return await referenceSet.GetMetadataReferenceAsync( + finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken).ConfigureAwait(false); } // okay, now use version based cache that can live multiple compilation as long as there is no semantic changes. - if (TryGetReference( - projectReference, finalOrDeclarationCompilation, version, cancellationToken, out reference)) - { + var result = await TryGetReferenceAsync(projectReference, finalOrDeclarationCompilation, version, cancellationToken).ConfigureAwait(false); + if (result != null) solution.Workspace.LogTestMessage($"Found already cached metadata for the branch and version {version}"); - return true; - } - // noop, we don't have any - reference = null; - return false; + return result; } - private bool TryGetReference( + private async Task TryGetReferenceAsync( ProjectReference projectReference, Compilation finalOrDeclarationCompilation, VersionStamp? version, - CancellationToken cancellationToken, - out MetadataReference reference) + CancellationToken cancellationToken) { - if (_projectIdToReferenceMap.TryGetValue(projectReference.ProjectId, out var referenceSet) && - (version == null || referenceSet.Version == version)) + SkeletonReferenceSet set; + using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { - // record it to snapshot based cache. - var newReferenceSet = s_compilationToReferenceMap.GetValue(finalOrDeclarationCompilation, _ => referenceSet); - reference = newReferenceSet.GetMetadataReference( - finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken); - return true; + if (_skeletonReferenceSet != null && + (version == null || _version == version)) + { + // record it to snapshot based cache. + set = s_compilationToReferenceMap.GetValue(finalOrDeclarationCompilation, _ => _skeletonReferenceSet); + } + else + { + return null; + } } - reference = null; - return false; + return await set.GetMetadataReferenceAsync( + finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken).ConfigureAwait(false); } - private class MetadataOnlyReferenceSet + private class SkeletonReferenceSet { - // use WeakReference so we don't keep MetadataReference's alive if they are not being consumed - private readonly NonReentrantLock _gate = new(useThisInstanceForSynchronization: true); + private readonly SemaphoreSlim _gate = new(initialCount: 1); + // use WeakReference so we don't keep MetadataReference's alive if they are not being consumed private readonly Dictionary> _metadataReferences = new(); private readonly MetadataOnlyImage _image; - public readonly VersionStamp Version; - - public MetadataOnlyReferenceSet(VersionStamp version, MetadataOnlyImage image) + public SkeletonReferenceSet(MetadataOnlyImage image) { - Version = version; _image = image; } - public MetadataReference GetMetadataReference( + public async Task GetMetadataReferenceAsync( Compilation compilation, ImmutableArray aliases, bool embedInteropTypes, CancellationToken cancellationToken) { var key = new MetadataReferenceProperties(MetadataImageKind.Assembly, aliases, embedInteropTypes); - using (_gate.DisposableWait(cancellationToken)) + using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { if (!_metadataReferences.TryGetValue(key, out var weakMetadata) || !weakMetadata.TryGetTarget(out var metadataReference)) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs index cbb722776a0bb..2cc40fced54f1 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs @@ -61,6 +61,8 @@ internal partial class ProjectState /// private ImmutableArray _lazySourceGenerators; + private readonly CachedSkeletonReferences _cachedSkeletonReferences; + private ProjectState( ProjectInfo projectInfo, HostLanguageServices languageServices, @@ -89,7 +91,10 @@ private ProjectState( _lazyChecksums = new AsyncLazy(ComputeChecksumsAsync, cacheResult: true); } - public ProjectState(ProjectInfo projectInfo, HostLanguageServices languageServices, SolutionServices solutionServices) + public ProjectState( + ProjectInfo projectInfo, + HostLanguageServices languageServices, + SolutionServices solutionServices) { Contract.ThrowIfNull(projectInfo); Contract.ThrowIfNull(languageServices); @@ -781,5 +786,26 @@ private void GetLatestDependentVersions( CreateLazyLatestDocumentTopLevelChangeVersion(newDocument, newDocumentStates, newAdditionalDocumentStates) : _lazyLatestDocumentTopLevelChangeVersion; } + + public MetadataReference GetOrBuildSkeletonReference( + SolutionState solution, + ProjectReference projectReference, + Compilation finalCompilation, + VersionStamp version, + CancellationToken cancellationToken) + { + + } + + public bool TryGetSkeletonReference( + SolutionState solution, + ProjectReference projectReference, + Compilation finalOrDeclarationCompilation, + VersionStamp version, + CancellationToken cancellationToken, + out MetadataReference reference) + { + + } } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs index 199c6f58e70e5..51e524b11baa9 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs @@ -44,18 +44,14 @@ private partial class CompilationTracker : ICompilationTracker // guarantees only one thread is building at a time private readonly SemaphoreSlim _buildLock = new(initialCount: 1); - public CachedSkeletonReferences CachedSkeletonReferences { get; } - private CompilationTracker( ProjectState project, - CompilationTrackerState state, - CachedSkeletonReferences cachedSkeletonReferences) + CompilationTrackerState state) { Contract.ThrowIfNull(project); this.ProjectState = project; _stateDoNotAccessDirectly = state; - CachedSkeletonReferences = cachedSkeletonReferences; } /// @@ -63,7 +59,7 @@ private CompilationTracker( /// and will have no extra information beyond the project itself. /// public CompilationTracker(ProjectState project) - : this(project, CompilationTrackerState.Empty, CachedSkeletonReferences.Empty) + : this(project, CompilationTrackerState.Empty) { } @@ -147,13 +143,13 @@ public ICompilationTracker Fork( var newState = CompilationTrackerState.Create( solutionServices, baseCompilation, state.GeneratorInfo, state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken), intermediateProjects); - return new CompilationTracker(newProject, newState, this.CachedSkeletonReferences.Clone()); + return new CompilationTracker(newProject, newState); } else { // We have no compilation, but we might have information about generated docs. var newState = new NoCompilationState(state.GeneratorInfo.WithDocumentsAreFinal(false)); - return new CompilationTracker(newProject, newState, this.CachedSkeletonReferences.Clone()); + return new CompilationTracker(newProject, newState); } } @@ -197,7 +193,7 @@ public ICompilationTracker FreezePartialStateWithTree(SolutionState solution, Do this.ProjectState.Id, metadataReferenceToProjectId); - return new CompilationTracker(inProgressProject, finalState, this.CachedSkeletonReferences.Clone()); + return new CompilationTracker(inProgressProject, finalState); } /// @@ -1016,7 +1012,7 @@ private async Task GetMetadataOnlyImageReferenceAsync( var declarationCompilation = await this.GetOrBuildDeclarationCompilationAsync(solution.Services, cancellationToken: cancellationToken).ConfigureAwait(false); solution.Workspace.LogTestMessage($"Looking for a cached skeleton assembly for {projectReference.ProjectId} before taking the lock..."); - if (!this.CachedSkeletonReferences.TryGetReference( + if (!this.ProjectState.TryGetSkeletonReference( solution, projectReference, declarationCompilation, version, cancellationToken, out var reference)) { // using async build lock so we don't get multiple consumers attempting to build metadata-only images for the same compilation. @@ -1026,7 +1022,7 @@ private async Task GetMetadataOnlyImageReferenceAsync( // okay, we still don't have one. bring the compilation to final state since we are going to use it to create skeleton assembly var compilationInfo = await this.GetOrBuildCompilationInfoAsync(solution, lockGate: false, cancellationToken: cancellationToken).ConfigureAwait(false); - reference = this.CachedSkeletonReferences.GetOrBuildReference( + reference = this.ProjectState.GetOrBuildSkeletonReference( solution, projectReference, compilationInfo.Compilation, version, cancellationToken); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs index 1aba5cd6824f2..cb13374ae2fc5 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs @@ -38,7 +38,6 @@ public GeneratedFileReplacingCompilationTracker(ICompilationTracker underlyingTr } public ProjectState ProjectState => _underlyingTracker.ProjectState; - public CachedSkeletonReferences CachedSkeletonReferences => _underlyingTracker.CachedSkeletonReferences; public bool ContainsAssemblyOrModuleOrDynamic(ISymbol symbol, bool primary) { @@ -145,7 +144,7 @@ public async Task GetMetadataReferenceAsync(SolutionState sol // Otherwise we need to create a skeleton for this project. See if we can reuse an existing // one, or create a new one when we can't. var version = await GetDependentSemanticVersionAsync(solution, cancellationToken).ConfigureAwait(false); - return _underlyingTracker.CachedSkeletonReferences.GetOrBuildReference(solution, projectReference, compilation, version, cancellationToken); + return this.ProjectState.GetOrBuildSkeletonReference(solution, projectReference, compilation, version, cancellationToken); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.ICompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.ICompilationTracker.cs index e40cab60c7c87..0f5ae213cbe9d 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.ICompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.ICompilationTracker.cs @@ -13,7 +13,6 @@ internal partial class SolutionState private interface ICompilationTracker { ProjectState ProjectState { get; } - CachedSkeletonReferences CachedSkeletonReferences { get; } /// /// Returns if this / could produce the From fea7fc9a31aeb25f61d58f6d2a9165d0b63508a5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 Oct 2021 12:21:24 -0700 Subject: [PATCH 10/14] Temp --- .../Solution/CachedSkeletonReferences.cs | 108 ++++++++++-------- 1 file changed, 61 insertions(+), 47 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/CachedSkeletonReferences.cs b/src/Workspaces/Core/Portable/Workspace/Solution/CachedSkeletonReferences.cs index ba8531e8ec736..c402e876c5371 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/CachedSkeletonReferences.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/CachedSkeletonReferences.cs @@ -46,6 +46,8 @@ internal class CachedSkeletonReferences /// private static readonly ConditionalWeakTable s_compilationToReferenceMap = new(); + private readonly ProjectId _projectId; + private readonly SemaphoreSlim _gate = new(initialCount: 1); /// @@ -58,13 +60,17 @@ internal class CachedSkeletonReferences /// private SkeletonReferenceSet? _skeletonReferenceSet; - public static readonly CachedSkeletonReferences Empty = - new(version: null, skeletonReferenceSet: null); + public CachedSkeletonReferences(ProjectId projectId) + : this(projectId, version: null, skeletonReferenceSet: null) + { + } private CachedSkeletonReferences( + ProjectId projectId, VersionStamp? version, SkeletonReferenceSet? skeletonReferenceSet) { + _projectId = projectId; _version = version; _skeletonReferenceSet = skeletonReferenceSet; } @@ -77,13 +83,13 @@ public CachedSkeletonReferences Clone() // can use this data if either the version changed, or they weren't able to build a skeleton for themselves. // By passing along a copy we ensure that if they have a different version, they'll end up producing a new // SkeletonReferenceSet where they'll store their own data in which will not affect prior ProjectStates. - return new CachedSkeletonReferences(_version, _skeletonReferenceSet); + return new CachedSkeletonReferences(_projectId, _version, _skeletonReferenceSet); } } public async Task GetOrBuildReferenceAsync( - SolutionState solution, - ProjectReference projectReference, + Workspace workspace, + MetadataReferenceProperties properties, Compilation finalCompilation, VersionStamp version, CancellationToken cancellationToken) @@ -91,11 +97,11 @@ public async Task GetOrBuildReferenceAsync( // First see if we already have a cached reference for either finalCompilation or for projectReference. // If we have one for the latter, we'll make sure that it's version matches what we're asking for before // returning it. - solution.Workspace.LogTestMessage($"Looking to see if we already have a skeleton assembly for {projectReference.ProjectId} before we build one..."); - var reference = await TryGetReferenceAsync(solution, projectReference, finalCompilation, version, cancellationToken).ConfigureAwait(false); + workspace.LogTestMessage($"Looking to see if we already have a skeleton assembly for {_projectId} before we build one..."); + var reference = await TryGetReferenceAsync(workspace, properties, finalCompilation, version, cancellationToken).ConfigureAwait(false); if (reference != null) { - solution.Workspace.LogTestMessage($"A reference was found {projectReference.ProjectId} so we're skipping the build."); + workspace.LogTestMessage($"A reference was found {_projectId} so we're skipping the build."); return reference; } @@ -103,21 +109,24 @@ public async Task GetOrBuildReferenceAsync( // first, prepare image // * NOTE * image is cancellable, do not create it inside of conditional weak table. - var service = solution.Workspace.Services.GetService(); - var image = MetadataOnlyImage.Create(solution.Workspace, service, finalCompilation, cancellationToken); + var service = workspace.Services.GetService(); + var image = MetadataOnlyImage.Create(workspace, service, finalCompilation, cancellationToken); if (image.IsEmpty) { // unfortunately, we couldn't create one. see if we have one from previous compilation., it might be // out-of-date big time, but better than nothing. - reference = await TryGetReferenceAsync(solution, projectReference, finalCompilation, version: null, cancellationToken).ConfigureAwait(false); + reference = await TryGetReferenceAsync(workspace, properties, finalCompilation, version: null, cancellationToken).ConfigureAwait(false); if (reference != null) { - solution.Workspace.LogTestMessage($"We failed to create metadata so we're using the one we just found from an earlier version."); + workspace.LogTestMessage($"We failed to create metadata so we're using the one we just found from an earlier version."); return reference; } } + // We either had an image, or we didn't have an image and we didn't have a previous value we could reuse. + // Just store this image against this version for us, and any future ProjectStates that fork from us. + var tempReferenceSet = new SkeletonReferenceSet(image); var referenceSet = s_compilationToReferenceMap.GetValue(finalCompilation, _ => tempReferenceSet); if (tempReferenceSet != referenceSet) @@ -127,40 +136,40 @@ public async Task GetOrBuildReferenceAsync( image.Cleanup(); } - await (_gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) + // Store this for ourselves, and for any project states that clone from us from this point onwards. + using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { _version = version; _skeletonReferenceSet = referenceSet; } - return await referenceSet.GetMetadataReferenceAsync( - finalCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken).ConfigureAwait(false); + return await referenceSet.GetMetadataReferenceAsync(finalCompilation, properties, cancellationToken).ConfigureAwait(false); } /// - /// Tries to get the associated with the provided - /// that produced the . + /// Tries to get the with the given + /// for the . /// public Task TryGetReferenceAsync( - SolutionState solution, - ProjectReference projectReference, + Workspace workspace, + MetadataReferenceProperties properties, Compilation finalOrDeclarationCompilation, VersionStamp version, CancellationToken cancellationToken) { - return TryGetReferenceAsync(solution, projectReference, finalOrDeclarationCompilation, (VersionStamp?)version, cancellationToken); + return TryGetReferenceAsync(workspace, properties, finalOrDeclarationCompilation, (VersionStamp?)version, cancellationToken); } /// - /// . + /// . /// If is , any for that /// project may be returned, even if it doesn't correspond to that compilation. This is useful in error tolerance /// cases as building a skeleton assembly may easily fail. In that case it's better to use the last successfully /// built skeleton than just have no semantic information for that project at all. /// private async Task TryGetReferenceAsync( - SolutionState solution, - ProjectReference projectReference, + Workspace workspace, + MetadataReferenceProperties properties, Compilation finalOrDeclarationCompilation, VersionStamp? version, CancellationToken cancellationToken) @@ -168,22 +177,22 @@ public async Task GetOrBuildReferenceAsync( // if we have one from snapshot cache, use it. it will make sure same compilation will get same metadata reference always. if (s_compilationToReferenceMap.TryGetValue(finalOrDeclarationCompilation, out var referenceSet)) { - solution.Workspace.LogTestMessage($"Found already cached metadata in {nameof(s_compilationToReferenceMap)} for the exact compilation"); + workspace.LogTestMessage($"Found already cached metadata in {nameof(s_compilationToReferenceMap)} for the exact compilation"); return await referenceSet.GetMetadataReferenceAsync( - finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken).ConfigureAwait(false); + finalOrDeclarationCompilation, properties, cancellationToken).ConfigureAwait(false); } // okay, now use version based cache that can live multiple compilation as long as there is no semantic changes. - var result = await TryGetReferenceAsync(projectReference, finalOrDeclarationCompilation, version, cancellationToken).ConfigureAwait(false); + var result = await TryGetReferenceAsync(properties, finalOrDeclarationCompilation, version, cancellationToken).ConfigureAwait(false); if (result != null) - solution.Workspace.LogTestMessage($"Found already cached metadata for the branch and version {version}"); + workspace.LogTestMessage($"Found already cached metadata for the branch and version {version}"); return result; } private async Task TryGetReferenceAsync( - ProjectReference projectReference, + MetadataReferenceProperties properties, Compilation finalOrDeclarationCompilation, VersionStamp? version, CancellationToken cancellationToken) @@ -203,8 +212,7 @@ public async Task GetOrBuildReferenceAsync( } } - return await set.GetMetadataReferenceAsync( - finalOrDeclarationCompilation, projectReference.Aliases, projectReference.EmbedInteropTypes, cancellationToken).ConfigureAwait(false); + return await set.GetMetadataReferenceAsync(finalOrDeclarationCompilation, properties, cancellationToken).ConfigureAwait(false); } private class SkeletonReferenceSet @@ -221,27 +229,33 @@ public SkeletonReferenceSet(MetadataOnlyImage image) } public async Task GetMetadataReferenceAsync( - Compilation compilation, ImmutableArray aliases, bool embedInteropTypes, CancellationToken cancellationToken) + Compilation compilation, MetadataReferenceProperties properties, CancellationToken cancellationToken) { - var key = new MetadataReferenceProperties(MetadataImageKind.Assembly, aliases, embedInteropTypes); - + // lookup first and eagerly return cached value if we have it. using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { - if (!_metadataReferences.TryGetValue(key, out var weakMetadata) || - !weakMetadata.TryGetTarget(out var metadataReference)) + if (_metadataReferences.TryGetValue(properties, out var weakMetadata) && + weakMetadata.TryGetTarget(out var metadataReference)) { - // here we give out strong reference to compilation. so there is possibility that we end up making 2 compilations for same project alive. - // one for final compilation and one for declaration only compilation. but the final compilation will be eventually kicked out from compilation cache - // if there is no activity on the project. or the declaration compilation will go away if the project that depends on the reference doesn't have any - // activity when it is kicked out from compilation cache. if there is an activity, then both will updated as activity happens. - // so simply put, things will go away when compilations are kicked out from the cache or due to user activity. - // - // there is one case where we could have 2 compilations for same project alive. if a user opens a file that requires a skeleton assembly when the skeleton - // assembly project didn't reach the final stage yet and then the user opens another document that is part of the skeleton assembly project - // and then never change it. declaration compilation will be alive by skeleton assembly and final compilation will be alive by background compiler. - metadataReference = _image.CreateReference(aliases, embedInteropTypes, new DeferredDocumentationProvider(compilation)); - weakMetadata = new WeakReference(metadataReference); - _metadataReferences[key] = weakMetadata; + return metadataReference; + } + } + + // otherwise, create the metadata outside of the lock, and then try to assign it if no one else beat us + { + var metadataReference = _image.CreateReference(properties.Aliases, properties.EmbedInteropTypes, new DeferredDocumentationProvider(compilation)); + var weakMetadata = new WeakReference(metadataReference); + + using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) + { + if (_metadataReferences.TryGetValue(properties, out var innherWeakMetadata) && + weakMetadata.TryGetTarget(out var innerMetadataReference)) + { + // someone beat us to writing this. + return innerMetadataReference; + } + + _metadataReferences[properties] = weakMetadata; } return metadataReference; From ff2362bce774b1d8394eea154838ba192c569233 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 Oct 2021 12:28:35 -0700 Subject: [PATCH 11/14] Move skeleton refs to ProjectState --- .../Workspace/Solution/ProjectState.cs | 30 +++++++++++-------- .../SolutionState.CompilationTracker.cs | 23 +++++++------- ...eneratedFileReplacingCompilationTracker.cs | 16 +++++----- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs index 2cc40fced54f1..b9d9c399897d6 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs @@ -72,7 +72,8 @@ private ProjectState( TextDocumentStates analyzerConfigDocumentStates, AsyncLazy lazyLatestDocumentVersion, AsyncLazy lazyLatestDocumentTopLevelChangeVersion, - ValueSource lazyAnalyzerConfigSet) + ValueSource lazyAnalyzerConfigSet, + CachedSkeletonReferences cachedSkeletonReferences) { _solutionServices = solutionServices; _languageServices = languageServices; @@ -82,6 +83,7 @@ private ProjectState( _lazyLatestDocumentVersion = lazyLatestDocumentVersion; _lazyLatestDocumentTopLevelChangeVersion = lazyLatestDocumentTopLevelChangeVersion; _lazyAnalyzerConfigSet = lazyAnalyzerConfigSet; + _cachedSkeletonReferences = cachedSkeletonReferences; // ownership of information on document has moved to project state. clear out documentInfo the state is // holding on. otherwise, these information will be held onto unnecessarily by projectInfo even after @@ -102,6 +104,7 @@ public ProjectState( _languageServices = languageServices; _solutionServices = solutionServices; + _cachedSkeletonReferences = new CachedSkeletonReferences(this.Id); var projectInfoFixed = FixProjectInfo(projectInfo); @@ -481,7 +484,11 @@ private ProjectState With( analyzerConfigDocumentStates ?? AnalyzerConfigDocumentStates, latestDocumentVersion ?? _lazyLatestDocumentVersion, latestDocumentTopLevelChangeVersion ?? _lazyLatestDocumentTopLevelChangeVersion, - analyzerConfigSet ?? _lazyAnalyzerConfigSet); + analyzerConfigSet ?? _lazyAnalyzerConfigSet, + // Ensure this fork get's a clone of whatever cached skeletons we point at. It can reuse them if its + // top level semantic version is the same, or if it fails to produce its own skeletons due to errors. + // if it produces new skeletons, it won't affect us though as it will only work on its own copy. + _cachedSkeletonReferences.Clone()); } private ProjectInfo.ProjectAttributes Attributes @@ -787,25 +794,24 @@ private void GetLatestDependentVersions( _lazyLatestDocumentTopLevelChangeVersion; } - public MetadataReference GetOrBuildSkeletonReference( - SolutionState solution, - ProjectReference projectReference, + public Task GetOrBuildSkeletonReferenceAsync( + Workspace workspace, + MetadataReferenceProperties properties, Compilation finalCompilation, VersionStamp version, CancellationToken cancellationToken) { - + return _cachedSkeletonReferences.GetOrBuildReferenceAsync(workspace, properties, finalCompilation, version, cancellationToken); } - public bool TryGetSkeletonReference( - SolutionState solution, - ProjectReference projectReference, + public Task TryGetSkeletonReferenceAsync( + Workspace workspace, + MetadataReferenceProperties properties, Compilation finalOrDeclarationCompilation, VersionStamp version, - CancellationToken cancellationToken, - out MetadataReference reference) + CancellationToken cancellationToken) { - + return _cachedSkeletonReferences.TryGetReferenceAsync(workspace, properties, finalOrDeclarationCompilation, version, cancellationToken); } } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs index 51e524b11baa9..ca9f41e894dce 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs @@ -1006,30 +1006,33 @@ private async Task GetMetadataOnlyImageReferenceAsync( { using (Logger.LogBlock(FunctionId.Workspace_SkeletonAssembly_GetMetadataOnlyImage, cancellationToken)) { + var workspace = solution.Workspace; var version = await this.GetDependentSemanticVersionAsync(solution, cancellationToken).ConfigureAwait(false); // get or build compilation up to declaration state. this compilation will be used to provide live xml doc comment var declarationCompilation = await this.GetOrBuildDeclarationCompilationAsync(solution.Services, cancellationToken: cancellationToken).ConfigureAwait(false); - solution.Workspace.LogTestMessage($"Looking for a cached skeleton assembly for {projectReference.ProjectId} before taking the lock..."); + workspace.LogTestMessage($"Looking for a cached skeleton assembly for {projectReference.ProjectId} before taking the lock..."); - if (!this.ProjectState.TryGetSkeletonReference( - solution, projectReference, declarationCompilation, version, cancellationToken, out var reference)) + var properties = new MetadataReferenceProperties(aliases: projectReference.Aliases, embedInteropTypes: projectReference.EmbedInteropTypes); + var reference = await this.ProjectState.TryGetSkeletonReferenceAsync( + workspace, properties, declarationCompilation, version, cancellationToken).ConfigureAwait(false); + if (reference != null) + { + workspace.LogTestMessage($"Reusing the already cached skeleton assembly for {projectReference.ProjectId}"); + } + else { // using async build lock so we don't get multiple consumers attempting to build metadata-only images for the same compilation. using (await _buildLock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { - solution.Workspace.LogTestMessage($"Build lock taken for {ProjectState.Id}..."); + workspace.LogTestMessage($"Build lock taken for {ProjectState.Id}..."); // okay, we still don't have one. bring the compilation to final state since we are going to use it to create skeleton assembly var compilationInfo = await this.GetOrBuildCompilationInfoAsync(solution, lockGate: false, cancellationToken: cancellationToken).ConfigureAwait(false); - reference = this.ProjectState.GetOrBuildSkeletonReference( - solution, projectReference, compilationInfo.Compilation, version, cancellationToken); + reference = await this.ProjectState.GetOrBuildSkeletonReferenceAsync( + workspace, properties, compilationInfo.Compilation, version, cancellationToken).ConfigureAwait(false); } } - else - { - solution.Workspace.LogTestMessage($"Reusing the already cached skeleton assembly for {projectReference.ProjectId}"); - } return reference; } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs index cb13374ae2fc5..fce3b7f645ddd 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs @@ -136,16 +136,14 @@ public async Task GetMetadataReferenceAsync(SolutionState sol // If it's the same language we can just make a CompilationReference if (this.ProjectState.LanguageServices == fromProject.LanguageServices) - { return compilation.ToMetadataReference(projectReference.Aliases, projectReference.EmbedInteropTypes); - } - else - { - // Otherwise we need to create a skeleton for this project. See if we can reuse an existing - // one, or create a new one when we can't. - var version = await GetDependentSemanticVersionAsync(solution, cancellationToken).ConfigureAwait(false); - return this.ProjectState.GetOrBuildSkeletonReference(solution, projectReference, compilation, version, cancellationToken); - } + + // Otherwise we need to create a skeleton for this project. See if we can reuse an existing + // one, or create a new one when we can't. + var version = await GetDependentSemanticVersionAsync(solution, cancellationToken).ConfigureAwait(false); + var properties = new MetadataReferenceProperties(aliases: projectReference.Aliases, embedInteropTypes: projectReference.EmbedInteropTypes); + return await this.ProjectState.GetOrBuildSkeletonReferenceAsync( + solution.Workspace, properties, compilation, version, cancellationToken).ConfigureAwait(false); } public CompilationReference? GetPartialMetadataReference(ProjectState fromProject, ProjectReference projectReference) From 19748d1aad6d7f84b0d320e58bb75d6e232ef926 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 22 Oct 2021 13:25:44 -0700 Subject: [PATCH 12/14] revert' --- .../Core/Portable/Workspace/Solution/ProjectState.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs index c18e82629d118..cbb722776a0bb 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs @@ -89,10 +89,7 @@ private ProjectState( _lazyChecksums = new AsyncLazy(ComputeChecksumsAsync, cacheResult: true); } - public ProjectState( - ProjectInfo projectInfo, - HostLanguageServices languageServices, - SolutionServices solutionServices) + public ProjectState(ProjectInfo projectInfo, HostLanguageServices languageServices, SolutionServices solutionServices) { Contract.ThrowIfNull(projectInfo); Contract.ThrowIfNull(languageServices); From e3f9cbc8924c2b45f54b462aa2a5140e82ce6649 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 25 Oct 2021 13:28:23 -0700 Subject: [PATCH 13/14] Add comment --- src/Workspaces/Remote/Core/SolutionAssetStorage.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs index 94125b1473616..b4c973774acb9 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs @@ -50,6 +50,14 @@ private async ValueTask StoreAssetsAsync(Solution solution, ProjectId? pr : await solutionState.GetChecksumAsync(projectId, cancellationToken).ConfigureAwait(false); var context = SolutionReplicationContext.Create(); + // The check of `fromPrimaryBranch: solution == solution.Workspace.CurrentSolution` may result in + // non-deterministic behavior. Specifically, because this syncing is done in the BG, but change + // to .CurrentSolution can happen on another thread, it may be the case that 'fromPrimaryBranch' + // is true/false depending on the ordering of those ops. That's ok sa this is just a hint to the + // remote side if they should cache this snapshot or not, and fork future syncs off of it. We + // will always reach a fixed point here when the user pauses (which effectively always happens), so + // we're always going to get to the point that we do send `true` here and we cache a recent enough + // solution that we will then fork off of on the OOP side. var id = Interlocked.Increment(ref s_scopeId); var solutionInfo = new PinnedSolutionInfo( id, From 48c965288d36b971865b47faaaed4c2e234b6f70 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 25 Oct 2021 13:31:27 -0700 Subject: [PATCH 14/14] Simplify since we're under a lock --- .../Workspace/CompileTimeSolutionProvider.cs | 56 ++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs b/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs index 3c15f6050a98a..ad706d0de4aa8 100644 --- a/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs +++ b/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs @@ -83,51 +83,43 @@ public Solution GetCompileTimeSolution(Solution designTimeSolution) { lock (_gate) { - if (_designTimeToCompileTimeSoution.TryGetValue(designTimeSolution, out var cachedCompileTimeSolution) && - cachedCompileTimeSolution != null) + if (!_designTimeToCompileTimeSoution.TryGetValue(designTimeSolution, out var compileTimeSolution)) { - return cachedCompileTimeSolution; - } - - using var _1 = ArrayBuilder.GetInstance(out var configIdsToRemove); - using var _2 = ArrayBuilder.GetInstance(out var documentIdsToRemove); - - foreach (var (_, projectState) in designTimeSolution.State.ProjectStates) - { - var anyConfigs = false; + using var _1 = ArrayBuilder.GetInstance(out var configIdsToRemove); + using var _2 = ArrayBuilder.GetInstance(out var documentIdsToRemove); - foreach (var (_, configState) in projectState.AnalyzerConfigDocumentStates.States) + foreach (var (_, projectState) in designTimeSolution.State.ProjectStates) { - if (IsRazorAnalyzerConfig(configState)) + var anyConfigs = false; + + foreach (var (_, configState) in projectState.AnalyzerConfigDocumentStates.States) { - configIdsToRemove.Add(configState.Id); - anyConfigs = true; + if (IsRazorAnalyzerConfig(configState)) + { + configIdsToRemove.Add(configState.Id); + anyConfigs = true; + } } - } - // only remove design-time only documents when source-generated ones replace them - if (anyConfigs) - { - foreach (var (_, documentState) in projectState.DocumentStates.States) + // only remove design-time only documents when source-generated ones replace them + if (anyConfigs) { - if (documentState.Attributes.DesignTimeOnly) + foreach (var (_, documentState) in projectState.DocumentStates.States) { - documentIdsToRemove.Add(documentState.Id); + if (documentState.Attributes.DesignTimeOnly) + { + documentIdsToRemove.Add(documentState.Id); + } } } } - } - var compileTimeSolution = designTimeSolution - .RemoveAnalyzerConfigDocuments(configIdsToRemove.ToImmutable()) - .RemoveDocuments(documentIdsToRemove.ToImmutable()); + compileTimeSolution = designTimeSolution + .RemoveAnalyzerConfigDocuments(configIdsToRemove.ToImmutable()) + .RemoveDocuments(documentIdsToRemove.ToImmutable()); -#if NETCOREAPP - _designTimeToCompileTimeSoution.AddOrUpdate(designTimeSolution, compileTimeSolution); -#else - _designTimeToCompileTimeSoution.Remove(designTimeSolution); - _designTimeToCompileTimeSoution.Add(designTimeSolution, compileTimeSolution); -#endif + _designTimeToCompileTimeSoution.Add(designTimeSolution, compileTimeSolution); + } return compileTimeSolution; }