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
Avoid generating Document instances in cases where we only need a documentId in the work coordinator. #53817
Avoid generating Document instances in cases where we only need a documentId in the work coordinator. #53817
Conversation
|
||
await _processor.EnqueueWorkItemAsync(document).ConfigureAwait(false); | ||
await _processor.EnqueueWorkItemAsync(document.Project, document.Id, document).ConfigureAwait(false); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.SemanticChangeProcessor.cs
Outdated
Show resolved
Hide resolved
|
||
await _processor.EnqueueWorkItemAsync(document).ConfigureAwait(false); | ||
await _processor.EnqueueWorkItemAsync(solution.GetRequiredProject(documentId.ProjectId), documentId, document: null).ConfigureAwait(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to have some comment here that we're trying to avoid grabbing the Document? I can imagine somebody else coming back to this and wondering why it wasn't written the original way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look bad from my perspective, but I admit I don't understand how much of this code works.
Fixes #53816