Avoid linking back to the original TreeSource in document state updates if the associated lazy is already completed #75355
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…es if the associated lazy is already completed Previously, I was seeing pull diagnostics requests for linked documents as computing the changes between the original and current version of the file. Instead, change the code where we determine the tree source to use the latest tree source if it's been calculated.
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -69,8 +69,11 @@ public DocumentState UpdateTextAndTreeContents( | |||
| // We only need to look one deep here as we'll pull that tree source forward to our level. If another link is | |||
| // later added to us, it will do the same thing. | |||
| var originalTreeSource = this.TreeSource; | |||
| if (originalTreeSource is LinkedFileReuseTreeAndVersionSource linkedFileTreeAndVersionSource) | |||
| if (originalTreeSource is LinkedFileReuseTreeAndVersionSource linkedFileTreeAndVersionSource | |||
| && !linkedFileTreeAndVersionSource.TryGetValue(out var _)) | |||
There was a problem hiding this comment.
alternatively, if you are able to get the value from the linkedSource, why not pass it along instead? And if you can't, you defer to the original tree source it points to.
Also, i think we should doc .OriginalTreeSource. Mention that it's there to produce a long chain if we haven't computed the intermediary steps. But if we have, we'll use that value instead.
There was a problem hiding this comment.
Moved code around a bit and added some comments.
CyrusNajmabadi
approved these changes
Oct 2, 2024
|
Removed my approval. I would like to see if this can be potentially different. |
Open
3 tasks
| @@ -24,7 +24,26 @@ private sealed class LinkedFileReuseTreeAndVersionSource( | |||
| ITreeAndVersionSource originalTreeSource, | |||
| AsyncLazy<TreeAndVersion> lazyComputation) : ITreeAndVersionSource | |||
| { | |||
| public readonly ITreeAndVersionSource OriginalTreeSource = originalTreeSource; | |||
| // Used as a fallback value in GetComputedTreeAndVersionSource to avoid long lazy chain evaluations. | |||
| private readonly ITreeAndVersionSource _originalTreeSource = originalTreeSource; | |||
| /// </remarks> | ||
| public ITreeAndVersionSource GetNonChainedTreeAndVersionSource() | ||
| { | ||
| if (TryGetValue(out _)) |
CyrusNajmabadi
approved these changes
Oct 3, 2024
This was referenced Oct 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, I was seeing pull diagnostics requests for linked documents as computing the changes between the original and current version of the file. Instead, change the code where we determine the tree source to use the latest tree source if it's been calculated.