Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce project forks on solution open #73839

Merged
merged 2 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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);
}
ToddGrun marked this conversation as resolved.
Show resolved Hide resolved

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