Skip to content

Commit

Permalink
Merge pull request dotnet#72792 from CyrusNajmabadi/lazyMapAlternate
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi authored Mar 30, 2024
2 parents 9b9b115 + e0d809b commit 0288056
Show file tree
Hide file tree
Showing 11 changed files with 247 additions and 322 deletions.
5 changes: 3 additions & 2 deletions src/EditorFeatures/Test/TextEditor/OpenDocumentTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ public void LinkedFiles()

// Confirm the files have been linked by file path. This isn't the core part of this test but without it
// nothing else will work.
Assert.Equal(documentIds, workspace.CurrentSolution.GetDocumentIdsWithFilePath(FilePath));
Assert.Equal(new[] { documentIds.Last() }, workspace.CurrentSolution.GetDocument(documentIds.First()).GetLinkedDocumentIds());
AssertEx.SetEqual(documentIds, workspace.CurrentSolution.GetDocumentIdsWithFilePath(FilePath));
Assert.Equal(documentIds.Last(), workspace.CurrentSolution.GetDocument(documentIds.First()).GetLinkedDocumentIds().Single());
Assert.Equal(documentIds.First(), workspace.CurrentSolution.GetDocument(documentIds.Last()).GetLinkedDocumentIds().Single());

// Now the core test: first, if we make a modified version of the source text, and attempt to get the document for it,
// both copies should be updated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,30 @@ internal sealed override async Task<CompletionDescription> GetDescriptionWorkerA
var name = SymbolCompletionItem.GetSymbolName(item);
var kind = SymbolCompletionItem.GetKind(item);
var isGeneric = SymbolCompletionItem.GetSymbolIsGeneric(item);
var relatedDocumentIds = document.Project.Solution.GetRelatedDocumentIds(document.Id);
var typeConvertibilityCache = new Dictionary<ITypeSymbol, bool>(SymbolEqualityComparer.Default);

// First try with the document we're currently within.
var description = await TryGetDescriptionAsync(document.Id).ConfigureAwait(false);
if (description != null)
return description;

// If that didn't work, see about any related documents.
var relatedDocumentIds = document.Project.Solution.GetRelatedDocumentIds(document.Id);
foreach (var relatedId in relatedDocumentIds)
{
var relatedDocument = document.Project.Solution.GetRequiredDocument(relatedId);
if (relatedId == document.Id)
continue;

description = await TryGetDescriptionAsync(relatedId).ConfigureAwait(false);
if (description != null)
return description;
}

return CompletionDescription.Empty;

async Task<CompletionDescription?> TryGetDescriptionAsync(DocumentId documentId)
{
var relatedDocument = document.Project.Solution.GetRequiredDocument(documentId);
var context = await Utilities.CreateSyntaxContextWithExistingSpeculativeModelAsync(relatedDocument, position, cancellationToken).ConfigureAwait(false) as TSyntaxContext;
Contract.ThrowIfNull(context);
var symbols = await TryGetSymbolsForContextAsync(completionContext: null, context, options, cancellationToken).ConfigureAwait(false);
Expand All @@ -235,9 +253,9 @@ internal sealed override async Task<CompletionDescription> GetDescriptionWorkerA
return await SymbolCompletionItem.GetDescriptionAsync(item, bestSymbols.SelectAsArray(t => t.Symbol), document, context.SemanticModel, displayOptions, cancellationToken).ConfigureAwait(false);
}
}
}

return CompletionDescription.Empty;
return null;
}

static bool SymbolMatches(SymbolAndSelectionInfo info, string? name, SymbolKind? kind, bool isGeneric)
{
Expand Down

This file was deleted.

19 changes: 17 additions & 2 deletions src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Collections.Concurrent;
using System.Collections.Frozen;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
Expand All @@ -14,11 +15,11 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Serialization;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

Expand Down Expand Up @@ -976,4 +977,18 @@ private void GetLatestDependentVersions(
? CreateLazyLatestDocumentTopLevelChangeVersion(newDocument, newDocumentStates, newAdditionalDocumentStates)
: _lazyLatestDocumentTopLevelChangeVersion;
}

public void AddDocumentIdsWithFilePath(ref TemporaryArray<DocumentId> temporaryArray, string filePath)
{
this.DocumentStates.AddDocumentIdsWithFilePath(ref temporaryArray, filePath);
this.AdditionalDocumentStates.AddDocumentIdsWithFilePath(ref temporaryArray, filePath);
this.AnalyzerConfigDocumentStates.AddDocumentIdsWithFilePath(ref temporaryArray, filePath);
}

public DocumentId? GetFirstDocumentIdWithFilePath(string filePath)
{
return this.DocumentStates.GetFirstDocumentIdWithFilePath(filePath) ??
this.AdditionalDocumentStates.GetFirstDocumentIdWithFilePath(filePath) ??
this.AnalyzerConfigDocumentStates.GetFirstDocumentIdWithFilePath(filePath);
}
}
14 changes: 11 additions & 3 deletions src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1536,9 +1536,17 @@ internal async Task<Solution> WithMergedLinkedFileChangesAsync(
}

internal ImmutableArray<DocumentId> GetRelatedDocumentIds(DocumentId documentId)
{
return this.SolutionState.GetRelatedDocumentIds(documentId);
}
=> this.SolutionState.GetRelatedDocumentIds(documentId);

/// <summary>
/// Returns one of any of the related documents of <paramref name="documentId"/>. Importantly, this will never
/// return <paramref name="documentId"/> (unlike <see cref="GetRelatedDocumentIds"/> which includes the original
/// file in the result).
/// </summary>
/// <param name="relatedProjectIdHint">A hint on the first project to search when looking for related
/// documents. Must not be the project that <paramref name="documentId"/> is from.</param>
internal DocumentId? GetFirstRelatedDocumentId(DocumentId documentId, ProjectId? relatedProjectIdHint)
=> this.SolutionState.GetFirstRelatedDocumentId(documentId, relatedProjectIdHint);

internal Solution WithNewWorkspace(string? workspaceKind, int workspaceVersion, SolutionServices services)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ private SolutionCompilationState Branch(

if (newSolutionState == this.SolutionState &&
projectIdToTrackerMap == _projectIdToTrackerMap &&
newFrozenSourceGeneratedDocumentStates.Equals(FrozenSourceGeneratedDocumentStates))
Equals(newFrozenSourceGeneratedDocumentStates, FrozenSourceGeneratedDocumentStates))
{
return this;
}
Expand Down Expand Up @@ -1010,7 +1010,7 @@ public SolutionCompilationState WithoutFrozenSourceGeneratedDocuments()
if (FrozenSourceGeneratedDocumentStates == null)
return this;

var projectIdsToUnfreeze = FrozenSourceGeneratedDocumentStates.Value.States.Values
var projectIdsToUnfreeze = FrozenSourceGeneratedDocumentStates.States.Values
.Select(static state => state.Identity.DocumentId.ProjectId)
.Distinct()
.ToImmutableArray();
Expand Down Expand Up @@ -1144,19 +1144,6 @@ private SolutionCompilationState ComputeFrozenSnapshot(CancellationToken cancell
var newIdToProjectStateMapBuilder = this.SolutionState.ProjectStates.ToBuilder();
var newIdToTrackerMapBuilder = _projectIdToTrackerMap.ToBuilder();

// Keep track of the files that were potentially added between the last frozen snapshot point we have for a
// project and now. Specifically, if a file was removed in reality, it may show us as an add as we are
// effectively jumping back to a prior point in time for a particular project. We want all those files (and
// related doc ids) to be present in the frozen solution we hand back.
//
// Note: we only keep track of added files. We do not keep track of removed files. This is intentionally done
// for performance reasons. Specifically, it is quite normal for a project to drop all documents when frozen
// (for example, when no documents have been parsed in it). Actually dropping all these files from this map is
// very expensive. This does mean that the FilePathToDocumentIdsMap will be a superset of all files. That's
// ok. We'll mark this map as being frozen (and thus potentially containing a superset of legal ids), and later
// on our helpers will check for that and filter down to the set that is in a solution when queried.
var filePathToDocumentIdsMapBuilder = this.SolutionState.FilePathToDocumentIdsMap.ToFrozen().ToBuilder();

foreach (var projectId in this.SolutionState.ProjectIds)
{
cancellationToken.ThrowIfCancellationRequested();
Expand All @@ -1177,39 +1164,15 @@ private SolutionCompilationState ComputeFrozenSnapshot(CancellationToken cancell

newIdToProjectStateMapBuilder[projectId] = newProjectState;
newIdToTrackerMapBuilder[projectId] = newTracker;

// Freezing projects can cause them to have an entirely different set of documents (since it effectively
// rewinds the project back to the last time it produced a compilation). Ensure we keep track of the docs
// added or removed from the project states to keep the final filepath-to-documentid map accurate.
//
// Note: we only have to do this if the actual project-state changed. If we were able to use the same
// instance (common if we already got the compilation for a project), then nothing changes with the set
// of documents.
//
// Examples of where the documents may absolutely change though are when we haven't even gotten a
// compilation yet. In that case, the project transitions to an empty state, which means we should remove
// all its documents from the filePathToDocumentIdsMap. Similarly, if we were at some in-progress-state we
// might reset the project back to a prior state from when the last compilation was requested, losing
// information about documents recently added or removed.

if (oldProjectState != newProjectState)
{
AddMissingOrChangedFilePathMappings(filePathToDocumentIdsMapBuilder, oldProjectState.DocumentStates, newProjectState.DocumentStates);
AddMissingOrChangedFilePathMappings(filePathToDocumentIdsMapBuilder, oldProjectState.AdditionalDocumentStates, newProjectState.AdditionalDocumentStates);
AddMissingOrChangedFilePathMappings(filePathToDocumentIdsMapBuilder, oldProjectState.AnalyzerConfigDocumentStates, newProjectState.AnalyzerConfigDocumentStates);
}
}

var newIdToProjectStateMap = newIdToProjectStateMapBuilder.ToImmutable();
var newIdToTrackerMap = newIdToTrackerMapBuilder.ToImmutable();

var filePathToDocumentIdsMap = filePathToDocumentIdsMapBuilder.ToImmutable();

var dependencyGraph = SolutionState.CreateDependencyGraph(this.SolutionState.ProjectIds, newIdToProjectStateMap);

var newState = this.SolutionState.Branch(
idToProjectStateMap: newIdToProjectStateMap,
filePathToDocumentIdsMap: filePathToDocumentIdsMap,
dependencyGraph: dependencyGraph);

var newCompilationState = this.Branch(
Expand All @@ -1219,41 +1182,6 @@ private SolutionCompilationState ComputeFrozenSnapshot(CancellationToken cancell
cachedFrozenSnapshot: _cachedFrozenSnapshot);

return newCompilationState;

static void AddMissingOrChangedFilePathMappings<TDocumentState>(
FilePathToDocumentIdsMap.Builder filePathToDocumentIdsMapBuilder,
TextDocumentStates<TDocumentState> oldStates,
TextDocumentStates<TDocumentState> newStates) where TDocumentState : TextDocumentState
{
if (oldStates.Equals(newStates))
return;

// We want to make sure that all the documents in the new-state are properly represented in the file map.
// It's ok if old-state documents are still in the map as GetDocumentIdsWithFilePath will filter them out
// later since we're producing a frozen-partial map.
//
// Iterating over the new-states has an additional benefit. For projects that haven't ever been looked at
// (so they haven't really parsed any documents), this will results in empty new-states. So this loop will
// be almost a no-op for most non-relevant projects.
foreach (var (documentId, newDocumentState) in newStates.States)
{
if (!oldStates.TryGetState(documentId, out var oldDocumentState))
{
// Keep track of files that are definitely added. Make sure the added doc is in the file path map.
filePathToDocumentIdsMapBuilder.Add(newDocumentState.FilePath, documentId);

}
else if (oldDocumentState != newDocumentState &&
oldDocumentState.FilePath != newDocumentState.FilePath)
{
// Otherwise, if the document is in both, but the file name changed, then remove the old mapping
// and add the new mapping. Importantly, we don't want other linked files with the *old* path
// to consider this document one of their linked brethren.
filePathToDocumentIdsMapBuilder.Remove(oldDocumentState.FilePath, oldDocumentState.Id);
filePathToDocumentIdsMapBuilder.Add(newDocumentState.FilePath, newDocumentState.Id);
}
}
}
}

/// <summary>
Expand Down Expand Up @@ -1469,9 +1397,7 @@ private SolutionCompilationState AddDocumentsToMultipleProjects<TDocumentState>(

var stateChange = newCompilationState.SolutionState.ForkProject(
oldProjectState,
newProjectState,
// intentionally accessing this.Solution here not newSolutionState
newFilePathToDocumentIdsMap: this.SolutionState.CreateFilePathToDocumentIdsMapWithAddedDocuments(newDocumentStates));
newProjectState);

newCompilationState = newCompilationState.ForkProject(
stateChange,
Expand Down Expand Up @@ -1523,9 +1449,7 @@ private SolutionCompilationState RemoveDocumentsFromMultipleProjects<T>(

var stateChange = newCompilationState.SolutionState.ForkProject(
oldProjectState,
newProjectState,
// Intentionally using this.Solution here and not newSolutionState
newFilePathToDocumentIdsMap: this.SolutionState.CreateFilePathToDocumentIdsMapWithRemovedDocuments(removedDocumentStatesForProject));
newProjectState);

newCompilationState = newCompilationState.ForkProject(
stateChange,
Expand Down
Loading

0 comments on commit 0288056

Please sign in to comment.