Skip to content

Commit

Permalink
Merge pull request #58856 from CyrusNajmabadi/revertBranchIdBackport
Browse files Browse the repository at this point in the history
Revert "Merge pull request #57132 from CyrusNajmabadi/branchId3"
  • Loading branch information
CyrusNajmabadi authored Jan 20, 2022
2 parents aba7c2a + a6ecada commit 30573e1
Show file tree
Hide file tree
Showing 12 changed files with 187 additions and 53 deletions.
111 changes: 74 additions & 37 deletions src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -45,15 +44,20 @@ public Factory()
private const string RazorSourceGeneratorTypeName = "Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator";
private static readonly string s_razorSourceGeneratorFileNamePrefix = Path.Combine(RazorSourceGeneratorAssemblyName, RazorSourceGeneratorTypeName);

private readonly Workspace _workspace;

private readonly object _gate = new();

#if NETCOREAPP
private readonly ConditionalWeakTable<Solution, Solution> _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<Solution, Solution> _designTimeToCompileTimeSoution = new();
#endif
/// <summary>
/// Cached compile time solution corresponding to the <see cref="Workspace.PrimaryBranchId"/>
/// </summary>
private (int DesignTimeSolutionVersion, BranchId DesignTimeSolutionBranch, Solution CompileTimeSolution)? _primaryBranchCompileTimeCache;

/// <summary>
/// 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.
/// </summary>
private (int DesignTimeSolutionVersion, BranchId DesignTimeSolutionBranch, Solution CompileTimeSolution)? _forkedBranchCompileTimeCache;

public CompileTimeSolutionProvider(Workspace workspace)
{
Expand All @@ -63,14 +67,12 @@ public CompileTimeSolutionProvider(Workspace workspace)
{
lock (_gate)
{
#if NETCOREAPP
_designTimeToCompileTimeSoution.Clear();
#else
_designTimeToCompileTimeSoution = new();
#endif
_primaryBranchCompileTimeCache = null;
_forkedBranchCompileTimeCache = null;
}
}
};
_workspace = workspace;
}

private static bool IsRazorAnalyzerConfig(TextDocumentState documentState)
Expand All @@ -80,48 +82,83 @@ public Solution GetCompileTimeSolution(Solution designTimeSolution)
{
lock (_gate)
{
if (!_designTimeToCompileTimeSoution.TryGetValue(designTimeSolution, out var compileTimeSolution))
var cachedCompileTimeSolution = GetCachedCompileTimeSolution(designTimeSolution);

// Design time solution hasn't changed since we calculated the last compile-time solution:
if (cachedCompileTimeSolution != null)
{
return cachedCompileTimeSolution;
}

using var _1 = ArrayBuilder<DocumentId>.GetInstance(out var configIdsToRemove);
using var _2 = ArrayBuilder<DocumentId>.GetInstance(out var documentIdsToRemove);

foreach (var (_, projectState) in designTimeSolution.State.ProjectStates)
{
using var _1 = ArrayBuilder<DocumentId>.GetInstance(out var configIdsToRemove);
using var _2 = ArrayBuilder<DocumentId>.GetInstance(out var documentIdsToRemove);
var anyConfigs = false;

foreach (var (_, projectState) in designTimeSolution.State.ProjectStates)
foreach (var (_, configState) in projectState.AnalyzerConfigDocumentStates.States)
{
var anyConfigs = false;

foreach (var (_, configState) in projectState.AnalyzerConfigDocumentStates.States)
if (IsRazorAnalyzerConfig(configState))
{
if (IsRazorAnalyzerConfig(configState))
{
configIdsToRemove.Add(configState.Id);
anyConfigs = true;
}
configIdsToRemove.Add(configState.Id);
anyConfigs = true;
}
}

// only remove design-time only documents when source-generated ones replace them
if (anyConfigs)
// only remove design-time only documents when source-generated ones replace them
if (anyConfigs)
{
foreach (var (_, documentState) in projectState.DocumentStates.States)
{
foreach (var (_, documentState) in projectState.DocumentStates.States)
if (documentState.Attributes.DesignTimeOnly)
{
if (documentState.Attributes.DesignTimeOnly)
{
documentIdsToRemove.Add(documentState.Id);
}
documentIdsToRemove.Add(documentState.Id);
}
}
}
}

compileTimeSolution = designTimeSolution
.RemoveAnalyzerConfigDocuments(configIdsToRemove.ToImmutable())
.RemoveDocuments(documentIdsToRemove.ToImmutable());
var compileTimeSolution = designTimeSolution
.RemoveAnalyzerConfigDocuments(configIdsToRemove.ToImmutable())
.RemoveDocuments(documentIdsToRemove.ToImmutable());

_designTimeToCompileTimeSoution.Add(designTimeSolution, compileTimeSolution);
}
UpdateCachedCompileTimeSolution(designTimeSolution, compileTimeSolution);

return compileTimeSolution;
}
}

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
{
_forkedBranchCompileTimeCache = (designTimeSolution.WorkspaceVersion, designTimeSolution.BranchId, compileTimeSolution);
}
}

// Copied from
// https://github.com/dotnet/sdk/blob/main/src/RazorSdk/SourceGenerators/RazorSourceGenerator.Helpers.cs#L32
private static string GetIdentifierFromPath(string filePath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,7 @@ private static async Task VerifySolutionUpdate(
Assert.IsAssignableFrom<RemoteWorkspace>(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);
Expand All @@ -806,12 +807,14 @@ 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ 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<SyntaxTreeIndex> GetSyntaxTreeIndexAsync(this Document document, CancellationToken cancellationToken)
{
var result = await SyntaxTreeIndex.GetIndexAsync(document, loadOnly: false, cancellationToken).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ 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)!;

Expand Down
28 changes: 28 additions & 0 deletions src/Workspaces/Core/Portable/Workspace/Solution/BranchId.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// 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
{
/// <summary>
/// solution branch Id
/// </summary>
[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));
}
}
4 changes: 3 additions & 1 deletion src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ private Solution(SolutionState state)
}

internal Solution(Workspace workspace, SolutionInfo.SolutionAttributes solutionAttributes, SerializableOptionSet options, IReadOnlyList<AnalyzerReference> analyzerReferences)
: this(new SolutionState(new SolutionServices(workspace), solutionAttributes, options, analyzerReferences))
: this(new SolutionState(workspace.PrimaryBranchId, new SolutionServices(workspace), solutionAttributes, options, analyzerReferences))
{
}

Expand All @@ -48,6 +48,8 @@ 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);

/// <summary>
Expand Down
42 changes: 39 additions & 3 deletions src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ namespace Microsoft.CodeAnalysis
/// </summary>
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;

Expand Down Expand Up @@ -71,6 +74,7 @@ internal partial class SolutionState
private readonly SourceGeneratedDocumentState? _frozenSourceGeneratedDocumentState;

private SolutionState(
BranchId branchId,
int workspaceVersion,
SolutionServices solutionServices,
SolutionInfo.SolutionAttributes solutionAttributes,
Expand All @@ -85,6 +89,7 @@ private SolutionState(
Lazy<HostDiagnosticAnalyzers>? lazyAnalyzers,
SourceGeneratedDocumentState? frozenSourceGeneratedDocument)
{
_branchId = branchId;
_workspaceVersion = workspaceVersion;
_solutionAttributes = solutionAttributes;
_solutionServices = solutionServices;
Expand All @@ -111,11 +116,13 @@ static Lazy<HostDiagnosticAnalyzers> CreateLazyHostDiagnosticAnalyzers(IReadOnly
}

public SolutionState(
BranchId primaryBranchId,
SolutionServices solutionServices,
SolutionInfo.SolutionAttributes solutionAttributes,
SerializableOptionSet options,
IReadOnlyList<AnalyzerReference> analyzerReferences)
: this(
primaryBranchId,
workspaceVersion: 0,
solutionServices,
solutionAttributes,
Expand All @@ -140,7 +147,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(workspaceVersion: workspaceVersion, services: services);
return CreatePrimarySolution(branchId: workspace.PrimaryBranchId, workspaceVersion: workspaceVersion, services: services);
}

public HostDiagnosticAnalyzers Analyzers => _lazyAnalyzers.Value;
Expand All @@ -157,6 +164,19 @@ public SolutionState WithNewWorkspace(Workspace workspace, int workspaceVersion)

public SerializableOptionSet Options { get; }

/// <summary>
/// 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.
/// </summary>
public BranchId BranchId => _branchId;

/// <summary>
/// The Workspace this solution is associated with.
/// </summary>
Expand Down Expand Up @@ -216,6 +236,8 @@ private SolutionState Branch(
ProjectDependencyGraph? dependencyGraph = null,
Optional<SourceGeneratedDocumentState?> frozenSourceGeneratedDocument = default)
{
var branchId = GetBranchId();

if (idToProjectStateMap is not null)
{
Contract.ThrowIfNull(remoteSupportedProjectLanguages);
Expand All @@ -235,7 +257,8 @@ private SolutionState Branch(

var analyzerReferencesEqual = AnalyzerReferences.SequenceEqual(analyzerReferences);

if (solutionAttributes == _solutionAttributes &&
if (branchId == _branchId &&
solutionAttributes == _solutionAttributes &&
projectIds == ProjectIds &&
options == Options &&
analyzerReferencesEqual &&
Expand All @@ -249,6 +272,7 @@ private SolutionState Branch(
}

return new SolutionState(
branchId,
_workspaceVersion,
_solutionServices,
solutionAttributes,
Expand All @@ -265,16 +289,19 @@ private SolutionState Branch(
}

private SolutionState CreatePrimarySolution(
BranchId branchId,
int workspaceVersion,
SolutionServices services)
{
if (workspaceVersion == _workspaceVersion &&
if (branchId == _branchId &&
workspaceVersion == _workspaceVersion &&
services == _solutionServices)
{
return this;
}

return new SolutionState(
branchId,
workspaceVersion,
services,
_solutionAttributes,
Expand All @@ -290,6 +317,15 @@ 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;
}

/// <summary>
/// The version of the most recently modified project.
/// </summary>
Expand Down
Loading

0 comments on commit 30573e1

Please sign in to comment.