Reduce project forks on solution open #73839
Conversation
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.
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.cs
Outdated
Show resolved
Hide resolved
| { | ||
| // Changing a document in a linked-doc-chain will end up producing N changed documents. We only want to | ||
| // process that chain once. | ||
| if (processedDocuments.Add(changedDocumentId)) | ||
| using var _ = PooledDictionary<DocumentId, DocumentState>.GetInstance(out var documentIdsToUpdate); |
There was a problem hiding this comment.
can this happen? do we have any case where we'd see a doc-id twice for changed docs.
There was a problem hiding this comment.
I was being careful, but it seems like two docs could change that have the same related document, no?
| if (documentIdsToUpdate.Count == 0) | ||
| return solution; | ||
|
|
||
| var toProcessRelatedDocumentIds = documentIdsToUpdate.SelectAsArray(kvp => (kvp.Key, kvp.Value)); |
There was a problem hiding this comment.
i can't quite reason about everything here. do we really ever need a map? couldn't an array work for this?
also, toProcessRelatedDocumentIds seems like the wrong name, since it's more than just the id.
There was a problem hiding this comment.
As I mentioned in the other comment, it seems like it could need a map. I'll change the name.
|
I'm a little more inclined to move forward since you didn't see anything blatantly wrong. Feels nice to get rid of the last big forking issue on solution open. In reply to: 2097221328 |
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.
*** CPU from forks (before) ***
*** CPU from forks (after) ***
