Skip to content

Commit

Permalink
Reduce project forks on solution open (#73839)
Browse files Browse the repository at this point in the history 
* Reduce project forks on solution open

This doesn't have a huge impact on solution open time, so I'm on the fence about whether this is worth the churn. I *think* the new code is better, but I'm flexible about whether to pursue this.
  • Loading branch information
@ToddGrun
ToddGrun authored Jun 5, 2024
1 parent 1102c23 commit 24d3887
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 115 deletions.
4 changes: 2 additions & 2 deletions src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ await getFixedDocumentsAsync(
// expensive as we'd fork, produce semantics, fork, produce semantics, etc. etc.). Instead, by
// adding all the changed documents to one solution, and then cleaning *those* we only perform
// cleanup semantics on one forked solution.
var changedRoots = changedRootsAndTexts.SelectAsArray(t => t.Item2.node != null, t => (t.documentId, t.Item2.node!, PreservationMode.PreserveValue));
var changedTexts = changedRootsAndTexts.SelectAsArray(t => t.Item2.text != null, t => (t.documentId, t.Item2.text!, PreservationMode.PreserveValue));
var changedRoots = changedRootsAndTexts.SelectAsArray(t => t.Item2.node != null, t => (t.documentId, t.Item2.node!));
var changedTexts = changedRootsAndTexts.SelectAsArray(t => t.Item2.text != null, t => (t.documentId, t.Item2.text!));

return originalSolution
.WithDocumentSyntaxRoots(changedRoots)
Expand Down
34 changes: 15 additions & 19 deletions src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1181,14 +1181,11 @@ public Solution WithDocumentFilePath(DocumentId documentId, string filePath)
/// specified.
/// </summary>
public Solution WithDocumentText(DocumentId documentId, SourceText text, PreservationMode mode = PreservationMode.PreserveValue)
=> WithDocumentTexts([(documentId, text, mode)]);
=> WithDocumentTexts([(documentId, text)], mode);

internal Solution WithDocumentTexts(ImmutableArray<(DocumentId documentId, SourceText text)> texts)
=> WithDocumentTexts(texts.SelectAsArray(t => (t.documentId, t.text, PreservationMode.PreserveValue)));

internal Solution WithDocumentTexts(ImmutableArray<(DocumentId documentId, SourceText text, PreservationMode mode)> texts)
internal Solution WithDocumentTexts(ImmutableArray<(DocumentId documentId, SourceText text)> texts, PreservationMode mode = PreservationMode.PreserveValue)
{
foreach (var (documentId, text, mode) in texts)
foreach (var (documentId, text) in texts)
{
CheckContainsDocument(documentId);

Expand All @@ -1199,7 +1196,7 @@ internal Solution WithDocumentTexts(ImmutableArray<(DocumentId documentId, Sourc
throw new ArgumentOutOfRangeException(nameof(mode));
}

return WithCompilationState(_compilationState.WithDocumentTexts(texts));
return WithCompilationState(_compilationState.WithDocumentTexts(texts, mode));
}

/// <summary>
Expand Down Expand Up @@ -1312,31 +1309,30 @@ public Solution WithAnalyzerConfigDocumentText(DocumentId documentId, TextAndVer
/// rooted by the specified syntax node.
/// </summary>
public Solution WithDocumentSyntaxRoot(DocumentId documentId, SyntaxNode root, PreservationMode mode = PreservationMode.PreserveValue)
=> WithDocumentSyntaxRoots([(documentId, root, mode)]);

/// <inheritdoc cref="WithDocumentSyntaxRoot"/>.
internal Solution WithDocumentSyntaxRoots(ImmutableArray<(DocumentId documentId, SyntaxNode root)> syntaxRoots)
=> WithDocumentSyntaxRoots(syntaxRoots.SelectAsArray(t => (t.documentId, t.root, PreservationMode.PreserveValue)));
=> WithDocumentSyntaxRoots([(documentId, root)], mode);

/// <inheritdoc cref="WithDocumentSyntaxRoot"/>.
internal Solution WithDocumentSyntaxRoots(ImmutableArray<(DocumentId documentId, SyntaxNode root, PreservationMode mode)> syntaxRoots)
internal Solution WithDocumentSyntaxRoots(ImmutableArray<(DocumentId documentId, SyntaxNode root)> syntaxRoots, PreservationMode mode = PreservationMode.PreserveValue)
{
foreach (var (documentId, root, mode) in syntaxRoots)
if (!mode.IsValid())
throw new ArgumentOutOfRangeException(nameof(mode));

foreach (var (documentId, root) in syntaxRoots)
{
CheckContainsDocument(documentId);

if (root == null)
throw new ArgumentNullException(nameof(root));

if (!mode.IsValid())
throw new ArgumentOutOfRangeException(nameof(mode));
}

return WithCompilationState(_compilationState.WithDocumentSyntaxRoots(syntaxRoots));
return WithCompilationState(_compilationState.WithDocumentSyntaxRoots(syntaxRoots, mode));
}

internal Solution WithDocumentContentsFrom(DocumentId documentId, DocumentState documentState, bool forceEvenIfTreesWouldDiffer)
=> WithCompilationState(_compilationState.WithDocumentContentsFrom(documentId, documentState, forceEvenIfTreesWouldDiffer));
=> WithCompilationState(_compilationState.WithDocumentContentsFrom([(documentId, documentState)], forceEvenIfTreesWouldDiffer));

internal Solution WithDocumentContentsFrom(ImmutableArray<(DocumentId documentId, DocumentState documentState)> documentIdsAndStates, bool forceEvenIfTreesWouldDiffer)
=> WithCompilationState(_compilationState.WithDocumentContentsFrom(documentIdsAndStates, forceEvenIfTreesWouldDiffer));

/// <summary>
/// Creates a new solution instance with the document specified updated to have the source
Expand Down
57 changes: 34 additions & 23 deletions src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -708,18 +708,20 @@ public SolutionCompilationState WithDocumentFilePath(
this.SolutionState.WithDocumentFilePath(documentId, filePath), documentId);
}

internal SolutionCompilationState WithDocumentTexts(ImmutableArray<(DocumentId documentId, SourceText text, PreservationMode mode)> texts)
internal SolutionCompilationState WithDocumentTexts(ImmutableArray<(DocumentId documentId, SourceText text)> texts, PreservationMode mode)
=> WithDocumentContents(
texts, SourceTextIsUnchanged,
static (documentState, text, mode) => documentState.UpdateText(text, mode));
static (documentState, text, mode) => documentState.UpdateText(text, mode),
data: mode);

private static bool SourceTextIsUnchanged(DocumentState oldDocument, SourceText text)
private static bool SourceTextIsUnchanged(DocumentState oldDocument, SourceText text, PreservationMode mode)
=> oldDocument.TryGetText(out var oldText) && text == oldText;

private SolutionCompilationState WithDocumentContents<TContent>(
ImmutableArray<(DocumentId documentId, TContent content, PreservationMode mode)> texts,
Func<DocumentState, TContent, bool> isUnchanged,
Func<DocumentState, TContent, PreservationMode, DocumentState> updateContent)
private SolutionCompilationState WithDocumentContents<TContent, TData>(
ImmutableArray<(DocumentId documentId, TContent content)> texts,
Func<DocumentState, TContent, TData, bool> isUnchanged,
Func<DocumentState, TContent, TData, DocumentState> updateContent,
TData data)
{
return UpdateDocumentsInMultipleProjects(
texts.GroupBy(d => d.documentId.ProjectId).Select(g =>
Expand All @@ -728,13 +730,13 @@ private SolutionCompilationState WithDocumentContents<TContent>(
var projectState = this.SolutionState.GetRequiredProjectState(projectId);
using var _ = ArrayBuilder<DocumentState>.GetInstance(out var newDocumentStates);
foreach (var (documentId, content, mode) in g)
foreach (var (documentId, content) in g)
{
var documentState = projectState.DocumentStates.GetRequiredState(documentId);
if (isUnchanged(documentState, content))
if (isUnchanged(documentState, content, data))
continue;
newDocumentStates.Add(updateContent(documentState, content, mode));
newDocumentStates.Add(updateContent(documentState, content, data));
}
return (projectId, newDocumentStates.ToImmutableAndClear());
Expand Down Expand Up @@ -794,14 +796,15 @@ public SolutionCompilationState WithAnalyzerConfigDocumentText(
this.SolutionState.WithAnalyzerConfigDocumentText(documentId, textAndVersion, mode));
}

/// <inheritdoc cref="Solution.WithDocumentSyntaxRoots(ImmutableArray{ValueTuple{DocumentId, SyntaxNode, PreservationMode}})"/>
public SolutionCompilationState WithDocumentSyntaxRoots(ImmutableArray<(DocumentId documentId, SyntaxNode root, PreservationMode mode)> syntaxRoots)
/// <inheritdoc cref="Solution.WithDocumentSyntaxRoots(ImmutableArray{ValueTuple{DocumentId, SyntaxNode}}, PreservationMode)"/>
public SolutionCompilationState WithDocumentSyntaxRoots(ImmutableArray<(DocumentId documentId, SyntaxNode root)> syntaxRoots, PreservationMode mode)
{
return WithDocumentContents(
syntaxRoots, IsUnchanged,
static (documentState, root, mode) => documentState.UpdateTree(root, mode));
static (documentState, root, mode) => documentState.UpdateTree(root, mode),
data: mode);

static bool IsUnchanged(DocumentState oldDocument, SyntaxNode root)
static bool IsUnchanged(DocumentState oldDocument, SyntaxNode root, PreservationMode _)
{
return oldDocument.TryGetSyntaxTree(out var oldTree) &&
oldTree.TryGetRoot(out var oldRoot) &&
Expand All @@ -810,10 +813,18 @@ static bool IsUnchanged(DocumentState oldDocument, SyntaxNode root)
}

public SolutionCompilationState WithDocumentContentsFrom(
DocumentId documentId, DocumentState documentState, bool forceEvenIfTreesWouldDiffer)
ImmutableArray<(DocumentId documentId, DocumentState documentState)> documentIdsAndStates, bool forceEvenIfTreesWouldDiffer)
{
return UpdateDocumentState(
this.SolutionState.WithDocumentContentsFrom(documentId, documentState, forceEvenIfTreesWouldDiffer), documentId);
return WithDocumentContents(
documentIdsAndStates,
isUnchanged: static (oldDocumentState, documentState, forceEvenIfTreesWouldDiffer) =>
{
return oldDocumentState.TextAndVersionSource == documentState.TextAndVersionSource
&& oldDocumentState.TreeSource == documentState.TreeSource;
},
static (oldDocumentState, documentState, forceEvenIfTreesWouldDiffer) =>
oldDocumentState.UpdateTextAndTreeContents(documentState.TextAndVersionSource, documentState.TreeSource, forceEvenIfTreesWouldDiffer),
data: forceEvenIfTreesWouldDiffer);
}

/// <inheritdoc cref="SolutionState.WithDocumentSourceCodeKind"/>
Expand Down Expand Up @@ -1391,8 +1402,8 @@ public SolutionCompilationState WithFrozenPartialCompilationIncludingSpecificDoc
// compilation state instance. So in the case where there are no linked documents, there is no cost here. And
// there is no additional cost processing the initiating document in this loop.
var allDocumentIds = this.SolutionState.GetRelatedDocumentIds(documentId);
foreach (var siblingId in allDocumentIds)
currentCompilationState = currentCompilationState.WithDocumentContentsFrom(siblingId, currentDocumentState, forceEvenIfTreesWouldDiffer: true);
var allDocumentIdsWithCurrentDocumentState = allDocumentIds.SelectAsArray(static (docId, currentDocumentState) => (docId, currentDocumentState), currentDocumentState);
currentCompilationState = currentCompilationState.WithDocumentContentsFrom(allDocumentIdsWithCurrentDocumentState, forceEvenIfTreesWouldDiffer: true);

return WithFrozenPartialCompilationIncludingSpecificDocumentWorker(currentCompilationState, documentId, cancellationToken);

Expand Down Expand Up @@ -1651,7 +1662,7 @@ public SolutionCompilationState WithCachedSourceGeneratorState(ProjectId project
/// </summary>
public SolutionCompilationState WithDocumentText(IEnumerable<DocumentId?> documentIds, SourceText text, PreservationMode mode)
{
using var _ = ArrayBuilder<(DocumentId, SourceText, PreservationMode)>.GetInstance(out var changedDocuments);
using var _ = ArrayBuilder<(DocumentId, SourceText)>.GetInstance(out var changedDocuments);

foreach (var documentId in documentIds)
{
Expand All @@ -1670,15 +1681,15 @@ public SolutionCompilationState WithDocumentText(IEnumerable<DocumentId?> docume
// the same text (for example, when GetOpenDocumentInCurrentContextWithChanges) is called.
//
// The use of GetRequiredState mirrors what happens in WithDocumentTexts
if (!SourceTextIsUnchanged(documentState, text))
changedDocuments.Add((documentId, text, mode));
if (!SourceTextIsUnchanged(documentState, text, mode))
changedDocuments.Add((documentId, text));
}
}

if (changedDocuments.Count == 0)
return this;

return this.WithDocumentTexts(changedDocuments.ToImmutableAndClear());
return this.WithDocumentTexts(changedDocuments.ToImmutableAndClear(), mode);
}

internal TestAccessor GetTestAccessor()
Expand Down
27 changes: 0 additions & 27 deletions src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -988,33 +988,6 @@ public StateChange WithAnalyzerConfigDocumentText(DocumentId documentId, TextAnd
return UpdateAnalyzerConfigDocumentState(oldDocument.UpdateText(textAndVersion, mode));
}

/// <param name="forceEvenIfTreesWouldDiffer">Whether or not the specified document is forced to have the same text and
/// green-tree-root from <paramref name="documentState"/>. If <see langword="true"/>, then they will share
/// these values. If <see langword="false"/>, then they will only be shared when safe to do so (for example,
/// when parse-options and pp-directives would not cause issues.</param>
/// <remarks>
/// Forcing should only happen in frozen-partial snapshots, where we are ok with inaccuracies in the trees we
/// get back and want perf to be very high. Any codepaths from frozen-partial should pass <see
/// langword="true"/> for this. Any codepaths from Workspace.UnifyLinkedDocumentContents should pass <see
/// langword="false"/>.</remarks>
public StateChange WithDocumentContentsFrom(DocumentId documentId, DocumentState documentState, bool forceEvenIfTreesWouldDiffer)
{
var oldDocument = GetRequiredDocumentState(documentId);
var oldProject = GetRequiredProjectState(documentId.ProjectId);
if (oldDocument == documentState)
return new(this, oldProject, oldProject);

if (oldDocument.TextAndVersionSource == documentState.TextAndVersionSource &&
oldDocument.TreeSource == documentState.TreeSource)
{
return new(this, oldProject, oldProject);
}

return UpdateDocumentState(
oldDocument.UpdateTextAndTreeContents(documentState.TextAndVersionSource, documentState.TreeSource, forceEvenIfTreesWouldDiffer),
contentChanged: true);
}

/// <summary>
/// Creates a new solution instance with the document specified updated to have the source
/// code kind specified.
Expand Down
Loading

0 comments on commit 24d3887

Please sign in to comment.