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.

Sign up for GitHub

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

Jump to bottom

Avoid generating Document instances in cases where we only need a documentId in the work coordinator. #53817

Merged
merged 6 commits into from Jun 8, 2021
Merged

Avoid generating Document instances in cases where we only need a documentId in the work coordinator. #53817

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a77c37f
Avoid generating Document instances in cases where we only need a doc…
CyrusNajmabadi Jun 1, 2021
08ef9ae
Allow doc to be passed along if we ahve it
CyrusNajmabadi Jun 1, 2021
8a9065f
Do not realize doc
CyrusNajmabadi Jun 2, 2021
6f4a04f
Defer more work till later
CyrusNajmabadi Jun 2, 2021
0b7aa20
Make private
CyrusNajmabadi Jun 2, 2021
a6fd47f
Fix null ref
CyrusNajmabadi Jun 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 7 additions & 13 deletions src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.SemanticChangeProcessor.cs
View file
Open in desktop
Expand Up @@ -185,11 +185,9 @@ private async Task EnqueueWorkItemAsync(Document thisDocument, ImmutableArray<Lo

var document = solution.GetDocument(location.SourceTree, projectId);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
if (document == null || thisDocument == document)
{
continue;
}

await _processor.EnqueueWorkItemAsync(document).ConfigureAwait(false);
await _processor.EnqueueWorkItemAsync(document.Project, document.Id, document).ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we were in a scenario where we actually had the document, then we pass it along to avoid extra work later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could consdier not passing hte doc with the hope that project.GetDocument(id) is cheap if the red doc was already created.

}
}

Expand Down Expand Up @@ -394,17 +392,17 @@ public void Enqueue(ProjectId projectId, bool needDependencyTracking = false)
Logger.Log(FunctionId.WorkCoordinator_Project_Enqueue, s_enqueueLogger, Environment.TickCount, projectId);
}

public async Task EnqueueWorkItemAsync(Document document)
public async Task EnqueueWorkItemAsync(Project project, DocumentId documentId, Document? document)
{
// we are shutting down
CancellationToken.ThrowIfCancellationRequested();

// call to this method is serialized. and only this method does the writing.
var priorityService = document.GetLanguageService<IWorkCoordinatorPriorityService>();
var isLowPriority = priorityService != null && await priorityService.IsLowPriorityAsync(document, CancellationToken).ConfigureAwait(false);
var priorityService = project.GetLanguageService<IWorkCoordinatorPriorityService>();
var isLowPriority = priorityService != null && await priorityService.IsLowPriorityAsync(GetRequiredDocument(project, documentId, document), CancellationToken).ConfigureAwait(false);

_processor.Enqueue(
new WorkItem(document.Id, document.Project.Language, InvocationReasons.SemanticChanged,
new WorkItem(documentId, project.Language, InvocationReasons.SemanticChanged,
isLowPriority, activeMember: null, Listener.BeginAsyncOperation(nameof(EnqueueWorkItemAsync), tag: EnqueueItem)));
}

Expand Down Expand Up @@ -445,14 +443,10 @@ private Data Dequeue()
private async Task EnqueueWorkItemAsync(Project? project)
{
if (project == null)
{
return;
}

foreach (var document in project.Documents)
{
await EnqueueWorkItemAsync(document).ConfigureAwait(false);
}
foreach (var documentId in project.DocumentIds)
await EnqueueWorkItemAsync(project, documentId, document: null).ConfigureAwait(false);
}

private readonly struct Data
Expand Down
81 changes: 35 additions & 46 deletions src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs
View file
Open in desktop
Expand Up @@ -301,13 +301,13 @@ private void ProcessEvent(WorkspaceChangeEventArgs args, string eventName)
private void OnDocumentOpened(object? sender, DocumentEventArgs e)
{
_eventProcessingQueue.ScheduleTask("OnDocumentOpened",
() => EnqueueWorkItemAsync(e.Document, InvocationReasons.DocumentOpened), _shutdownToken);
() => EnqueueWorkItemAsync(e.Document.Project, e.Document.Id, e.Document, InvocationReasons.DocumentOpened), _shutdownToken);
}

private void OnDocumentClosed(object? sender, DocumentEventArgs e)
{
_eventProcessingQueue.ScheduleTask("OnDocumentClosed",
() => EnqueueWorkItemAsync(e.Document, InvocationReasons.DocumentClosed), _shutdownToken);
() => EnqueueWorkItemAsync(e.Document.Project, e.Document.Id, e.Document, InvocationReasons.DocumentClosed), _shutdownToken);
}

private void ProcessDocumentEvent(WorkspaceChangeEventArgs e, string eventName)
Expand Down Expand Up @@ -436,29 +436,32 @@ private void EnqueueEvent(Solution oldSolution, Solution newSolution, DocumentId
() => EnqueueWorkItemAfterDiffAsync(oldSolution, newSolution, documentId), _shutdownToken);
}

private async Task EnqueueWorkItemAsync(Document document, InvocationReasons invocationReasons, SyntaxNode? changedMember = null)
private async Task EnqueueWorkItemAsync(Project project, DocumentId documentId, Document? document, InvocationReasons invocationReasons, SyntaxNode? changedMember = null)
{
// we are shutting down
_shutdownToken.ThrowIfCancellationRequested();

var priorityService = document.GetLanguageService<IWorkCoordinatorPriorityService>();
var isLowPriority = priorityService != null && await priorityService.IsLowPriorityAsync(document, _shutdownToken).ConfigureAwait(false);
var priorityService = project.GetLanguageService<IWorkCoordinatorPriorityService>();
var isLowPriority = priorityService != null && await priorityService.IsLowPriorityAsync(GetRequiredDocument(project, documentId, document), _shutdownToken).ConfigureAwait(false);

var currentMember = GetSyntaxPath(changedMember);

// call to this method is serialized. and only this method does the writing.
_documentAndProjectWorkerProcessor.Enqueue(
new WorkItem(document.Id, document.Project.Language, invocationReasons, isLowPriority, currentMember, _listener.BeginAsyncOperation("WorkItem")));
new WorkItem(documentId, project.Language, invocationReasons, isLowPriority, currentMember, _listener.BeginAsyncOperation("WorkItem")));

// enqueue semantic work planner
if (invocationReasons.Contains(PredefinedInvocationReasons.SemanticChanged))
{
// must use "Document" here so that the snapshot doesn't go away. we need the snapshot to calculate p2p dependency graph later.
// due to this, we might hold onto solution (and things kept alive by it) little bit longer than usual.
_semanticChangeProcessor.Enqueue(document, currentMember);
_semanticChangeProcessor.Enqueue(GetRequiredDocument(project, documentId, document), currentMember);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}
}

private static Document GetRequiredDocument(Project project, DocumentId documentId, Document? document)
=> document ?? project.GetRequiredDocument(documentId);

private static SyntaxPath? GetSyntaxPath(SyntaxNode? changedMember)
{
// using syntax path might be too expansive since it will be created on every keystroke.
Expand All @@ -474,31 +477,28 @@ private async Task EnqueueWorkItemAsync(Document document, InvocationReasons inv
private async Task EnqueueWorkItemAsync(Project project, InvocationReasons invocationReasons)
{
foreach (var documentId in project.DocumentIds)
{
var document = project.GetRequiredDocument(documentId);
await EnqueueWorkItemAsync(document, invocationReasons).ConfigureAwait(false);
}
await EnqueueWorkItemAsync(project, documentId, document: null, invocationReasons).ConfigureAwait(false);
}

private async Task EnqueueWorkItemAsync(IIncrementalAnalyzer analyzer, ReanalyzeScope scope, bool highPriority)
{
var solution = _registration.GetSolutionToAnalyze();
var invocationReasons = highPriority ? InvocationReasons.ReanalyzeHighPriority : InvocationReasons.Reanalyze;

foreach (var document in scope.GetDocuments(solution))
{
await EnqueueWorkItemAsync(analyzer, document, invocationReasons).ConfigureAwait(false);
}
foreach (var (project, documentId) in scope.GetDocumentIds(solution))
await EnqueueWorkItemAsync(analyzer, project, documentId, document: null, invocationReasons).ConfigureAwait(false);
}

private async Task EnqueueWorkItemAsync(IIncrementalAnalyzer analyzer, Document document, InvocationReasons invocationReasons)
private async Task EnqueueWorkItemAsync(
IIncrementalAnalyzer analyzer, Project project, DocumentId documentId, Document? document, InvocationReasons invocationReasons)
{
var priorityService = document.GetLanguageService<IWorkCoordinatorPriorityService>();
var isLowPriority = priorityService != null && await priorityService.IsLowPriorityAsync(document, _shutdownToken).ConfigureAwait(false);
var priorityService = project.GetLanguageService<IWorkCoordinatorPriorityService>();
var isLowPriority = priorityService != null && await priorityService.IsLowPriorityAsync(
GetRequiredDocument(project, documentId, document), _shutdownToken).ConfigureAwait(false);

_documentAndProjectWorkerProcessor.Enqueue(
new WorkItem(document.Id, document.Project.Language, invocationReasons,
isLowPriority, analyzer, _listener.BeginAsyncOperation("WorkItem")));
new WorkItem(documentId, project.Language, invocationReasons,
isLowPriority, analyzer, _listener.BeginAsyncOperation("WorkItem")));
}

private async Task EnqueueWorkItemAsync(Solution oldSolution, Solution newSolution)
Expand Down Expand Up @@ -527,9 +527,7 @@ private async Task EnqueueWorkItemAsync(ProjectChanges projectChanges)
await EnqueueProjectConfigurationChangeWorkItemAsync(projectChanges).ConfigureAwait(false);

foreach (var addedDocumentId in projectChanges.GetAddedDocuments())
{
await EnqueueWorkItemAsync(projectChanges.NewProject.GetRequiredDocument(addedDocumentId), InvocationReasons.DocumentAdded).ConfigureAwait(false);
}
await EnqueueWorkItemAsync(projectChanges.NewProject, addedDocumentId, document: null, InvocationReasons.DocumentAdded).ConfigureAwait(false);

foreach (var changedDocumentId in projectChanges.GetChangedDocuments())
{
Expand All @@ -538,9 +536,7 @@ await EnqueueWorkItemAsync(projectChanges.OldProject.GetRequiredDocument(changed
}

foreach (var removedDocumentId in projectChanges.GetRemovedDocuments())
{
await EnqueueWorkItemAsync(projectChanges.OldProject.GetRequiredDocument(removedDocumentId), InvocationReasons.DocumentRemoved).ConfigureAwait(false);
}
await EnqueueWorkItemAsync(projectChanges.OldProject, removedDocumentId, document: null, InvocationReasons.DocumentRemoved).ConfigureAwait(false);
}

private async Task EnqueueProjectConfigurationChangeWorkItemAsync(ProjectChanges projectChanges)
Expand Down Expand Up @@ -589,24 +585,21 @@ private async Task EnqueueWorkItemAsync(Document oldDocument, Document newDocume
{
// For languages that don't use a Roslyn syntax tree, they don't export a document difference service.
// The whole document should be considered as changed in that case.
await EnqueueWorkItemAsync(newDocument, InvocationReasons.DocumentChanged).ConfigureAwait(false);
await EnqueueWorkItemAsync(newDocument.Project, newDocument.Id, newDocument, InvocationReasons.DocumentChanged).ConfigureAwait(false);
}
else
{
var differenceResult = await differenceService.GetDifferenceAsync(oldDocument, newDocument, _shutdownToken).ConfigureAwait(false);

if (differenceResult != null)
{
await EnqueueWorkItemAsync(newDocument, differenceResult.ChangeType, differenceResult.ChangedMember).ConfigureAwait(false);
}
await EnqueueWorkItemAsync(newDocument.Project, newDocument.Id, newDocument, differenceResult.ChangeType, differenceResult.ChangedMember).ConfigureAwait(false);
}
}

private Task EnqueueWorkItemForDocumentAsync(Solution solution, DocumentId documentId, InvocationReasons invocationReasons)
{
var document = solution.GetRequiredDocument(documentId);

return EnqueueWorkItemAsync(document, invocationReasons);
var project = solution.GetRequiredProject(documentId.ProjectId);
return EnqueueWorkItemAsync(project, documentId, document: null, invocationReasons);
}

private Task EnqueueWorkItemForProjectAsync(Solution solution, ProjectId projectId, InvocationReasons invocationReasons)
Expand Down Expand Up @@ -796,7 +789,7 @@ public int GetDocumentCount(Solution solution)
return count;
}

public IEnumerable<Document> GetDocuments(Solution solution)
public IEnumerable<(Project project, DocumentId documentId)> GetDocumentIds(Solution solution)
{
if (_solutionId != null && solution.Id != _solutionId)
{
Expand All @@ -805,9 +798,10 @@ public IEnumerable<Document> GetDocuments(Solution solution)

if (_solutionId != null)
{
foreach (var document in solution.Projects.SelectMany(p => p.Documents))
foreach (var project in solution.Projects)
{
yield return document;
foreach (var documentId in project.DocumentIds)
yield return (project, documentId);
}

yield break;
Expand All @@ -824,22 +818,17 @@ public IEnumerable<Document> GetDocuments(Solution solution)
var project = solution.GetProject(projectId);
if (project != null)
{
foreach (var document in project.Documents)
{
yield return document;
}
foreach (var documentId in project.DocumentIds)
yield return (project, documentId);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}

break;
}
case DocumentId documentId:
{
var document = solution.GetDocument(documentId);
if (document != null)
{
yield return document;
}

var project = solution.GetProject(documentId.ProjectId);
if (project != null)
yield return (project, documentId);
break;
}
}
Expand Down