Skip to content

Commit

Permalink
Merge pull request #57132 from CyrusNajmabadi/branchId3
Browse files Browse the repository at this point in the history 
Remove the BranchId concept from teh workspace
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored Oct 25, 2021
2 parents a4ec465 + 48c9652 commit 83a728a
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 184 deletions.
108 changes: 37 additions & 71 deletions src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -48,16 +49,13 @@ public Factory()

private readonly object _gate = new();

/// <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;
#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

public CompileTimeSolutionProvider(Workspace workspace)
{
Expand All @@ -67,8 +65,11 @@ public CompileTimeSolutionProvider(Workspace workspace)
{
lock (_gate)
{
_primaryBranchCompileTimeCache = null;
_forkedBranchCompileTimeCache = null;
#if NETCOREAPP
_designTimeToCompileTimeSoution.Clear();
#else
_designTimeToCompileTimeSoution = new();
#endif
}
}
};
Expand All @@ -82,83 +83,48 @@ 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 compileTimeSolution))
{
return cachedCompileTimeSolution;
}

using var _1 = ArrayBuilder<DocumentId>.GetInstance(out var configIdsToRemove);
using var _2 = ArrayBuilder<DocumentId>.GetInstance(out var documentIdsToRemove);
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)
{
var anyConfigs = false;

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());

UpdateCachedCompileTimeSolution(designTimeSolution, compileTimeSolution);
_designTimeToCompileTimeSoution.Add(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
3 changes: 0 additions & 3 deletions src/VisualStudio/Core/Test.Next/Services/SolutionServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,6 @@ 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 @@ -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);
}
Expand Down
2 changes: 0 additions & 2 deletions src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions src/Workspaces/Core/Portable/Shared/Extensions/DocumentExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<SyntaxTreeIndex> GetSyntaxTreeIndexAsync(this Document document, CancellationToken cancellationToken)
{
var result = await SyntaxTreeIndex.GetIndexAsync(document, loadOnly: false, cancellationToken).ConfigureAwait(false);
Expand Down
3 changes: 0 additions & 3 deletions src/Workspaces/Core/Portable/Shared/Extensions/ProjectExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)!;

Expand Down
28 changes: 0 additions & 28 deletions src/Workspaces/Core/Portable/Workspace/Solution/BranchId.cs
View file

This file was deleted.

4 changes: 1 addition & 3 deletions src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs
View file
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(workspace.PrimaryBranchId, new SolutionServices(workspace), solutionAttributes, options, analyzerReferences))
: this(new SolutionState(new SolutionServices(workspace), solutionAttributes, options, analyzerReferences))
{
}

Expand All @@ -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);

/// <summary>
Expand Down
42 changes: 3 additions & 39 deletions src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ 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 @@ -73,7 +70,6 @@ internal partial class SolutionState
private readonly SourceGeneratedDocumentState? _frozenSourceGeneratedDocumentState;

private SolutionState(
BranchId branchId,
int workspaceVersion,
SolutionServices solutionServices,
SolutionInfo.SolutionAttributes solutionAttributes,
Expand All @@ -88,7 +84,6 @@ private SolutionState(
Lazy<HostDiagnosticAnalyzers>? lazyAnalyzers,
SourceGeneratedDocumentState? frozenSourceGeneratedDocument)
{
_branchId = branchId;
_workspaceVersion = workspaceVersion;
_solutionAttributes = solutionAttributes;
_solutionServices = solutionServices;
Expand All @@ -115,13 +110,11 @@ 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 @@ -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;
Expand All @@ -163,19 +156,6 @@ 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 @@ -235,8 +215,6 @@ private SolutionState Branch(
ProjectDependencyGraph? dependencyGraph = null,
Optional<SourceGeneratedDocumentState?> frozenSourceGeneratedDocument = default)
{
var branchId = GetBranchId();

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

var analyzerReferencesEqual = AnalyzerReferences.SequenceEqual(analyzerReferences);

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

return new SolutionState(
branchId,
_workspaceVersion,
_solutionServices,
solutionAttributes,
Expand All @@ -288,19 +264,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,
Expand All @@ -316,15 +289,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;
}

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

0 comments on commit 83a728a

Please sign in to comment.