Ensure taggers that compute frozen partial data eventually move to a 'correct' final state. #72878

CyrusNajmabadi · 2024-04-04T18:34:33Z

Fixes AB#2016199

Our previous approach for trying to make this work was buggy and was removed in #72384.

The issue there was that we would first compute tags, using frozen-partial-semantics. Then kick off work to get the compilation for the current project (in the host and oop), then kick off an event to compute tags again, again using frozen-partial-semantics. The issue with that was the presumption that if a compilation was successfully retrieved that the next frozen-partial snapshot would contain information from that. That's not how frozen-partial works though. As it is just a snapshot system, once we build that snapshot, it's fixed (and we want that, so that all the features talking to the same frozen snapshot get all the normal caching we want for solution snapshots). So the next frozen-partial-request could still just get the same results as the prior time, effectively making the call pointless.

The new approach changes things fundamentally at the tagger level in the following fashion:

Taggers can state that they support computing results with frozen-partial-semantics.
For taggers that do (classification, reference highlighting, and inheritance margin tagger), the tagger system itself will first compute tags on a frozen-partial solution, calling into the tag provider, also telling it to compute with frozen-partial semantics. It will then report these tags as normal.
Then, after computing that set of possibly-incorrect tags, the tagger will re-enqueue work to compute tags again, this time with non-frozen-partial snapshots. This work is only then done at the next tagging step if no more work has come in to do normal frozen-partial-tagging.
the tagger will then run again, but this time with a normal snapshot, with the normal costs associated with it.

This change now means that the last piece of work one of these taggers will always do is a request to tag on a real full snapshot, with full semantics. So if there are any inaccurate/incomplete tags from a prior frozen-tagging pass, they will be updated to now be correct.

Perf should also be fine as we always prefer to do work for the frozen-case first, continuously computing in a fast, but inaccurate, mode as long as events keep coming in. Only once there is a lull in processing will we be able to get to the full mode.

CyrusNajmabadi · 2024-04-04T19:13:13Z

...s/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs

+        // amount of time waiting for all the compilation information to be built.  For example, we can classify types
+        // that we've parsed in other files, or partially loaded from metadata, even if we're still parsing/loading.
+        // For cross language projects, this also produces semantic classifications more quickly as we do not have to
+        // wait on skeletons to be built.


this is a move of this comment.

Pilchie · 2024-04-04T19:13:24Z

✅ Successfully linked to Azure Boards work item(s):

FeedbackTicket 2016199

CyrusNajmabadi · 2024-04-04T20:08:11Z

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs

+            EnqueueWork(highPriority, _dataSource.SupportsFrozenPartialSemantics);
+        }
+
+        private void EnqueueWork(bool highPriority, bool frozenPartialSemantics)


@sharwell for eyes on thsi threading logic.

CyrusNajmabadi · 2024-04-04T20:10:11Z

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs

+                }
+                else
+                {
+                    // We're asking for expensive tags, and this tagger supports frozen partial tags.  Kick off the work


@sharwell for extra attention here.

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs

…ider.TagSource_ProduceTags.cs

…adi/roslyn into frozenPartialTagging

CyrusNajmabadi · 2024-04-04T20:52:55Z

...s/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs

    }

-    protected override bool TagEquals(IClassificationTag tag1, IClassificationTag tag2)
-        => tag1.ClassificationType.Classification == tag2.ClassificationType.Classification;
+    public async Task ProduceTagsAsync(


these are just method moves from a utilities class far awqay. they were only called here (and referred to tagger types). so i just moved them all back over. they are otherwise unchanged.

CyrusNajmabadi · 2024-04-04T20:53:30Z

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs

+        /// cancel the expensive non-frozen tagging pass (which might be computing skeletons, SG docs, etc.), do the 
+        /// next cheap frozen-tagging-pass, and then push the expensive-nonfrozen-tagging-pass to the end again.
+        /// </summary>
+        private readonly CancellationSeries _nonFrozenComputationCancellationSeries = new();


@sharwell for eyes.

CyrusNajmabadi · 2024-04-04T20:54:07Z

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs

+                    {
+                    }
+                }
+            }


i felt like there could be a cleaner way of writing all this. but i couldn't figure it out :)

src/EditorFeatures/Core/ReferenceHighlighting/ReferenceHighlightingViewTaggerProvider.cs

…htingViewTaggerProvider.cs

…adi/roslyn into frozenPartialTagging

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.cs

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.cs

sharwell · 2024-04-05T18:32:10Z

src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensHelpers.cs

@@ -60,7 +60,7 @@ public static async Task<int[]> HandleRequestHelperAsync(Document document, Immu
            // If the full compilation is not yet available, we'll try getting a partial one. It may contain inaccurate
            // results but will speed up how quickly we can respond to the client's request.
            document = document.WithFrozenPartialSemantics(cancellationToken);
-            options = options with { ForceFrozenPartialSemanticsForCrossProcessOperations = true };
+            options = options with { FrozenPartialSemantics = true };


📝 This seems like something we'll need to follow-up on.

…ider.cs Co-authored-by: Sam Harwell <[email protected]>

genlu · 2024-04-05T19:58:19Z

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs

-            var highPriority = changes.Contains(true);
-            return new ValueTask<VoidResult>(RecomputeTagsAsync(highPriority, cancellationToken));
+            var highPriority = changes.Contains(x => x.highPriority);
+            var frozenPartialSemantics = changes.Contains(t => t.frozenPartialSemantics);


);

if we have one request of (highPriority: true, frozenPartialSemantics: false) and another (highPriority: false, frozenPartialSemantics: true), then we'd recompute with a mix setting of (true, true), is that intended? If so, pls add comment to clearify that. Thanks!

will add comment.

CyrusNajmabadi added 2 commits April 4, 2024 10:33

Initial work to support frozen partial tagging

db6b65e

Update tests

0dab34b

CyrusNajmabadi requested a review from a team as a code owner April 4, 2024 18:34

dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 4, 2024

CyrusNajmabadi marked this pull request as draft April 4, 2024 18:34

CyrusNajmabadi added 2 commits April 4, 2024 12:00

Add tests

209a95c

Update tests

1411434

CyrusNajmabadi marked this pull request as ready for review April 4, 2024 19:12

CyrusNajmabadi requested review from sharwell and dibarbet April 4, 2024 19:12

CyrusNajmabadi commented Apr 4, 2024

View reviewed changes

CyrusNajmabadi added 5 commits April 4, 2024 13:00

in progress

be9305d

Cancellation series

303982a

comments

06116b2

move docs

b976155

Simplify

4e3d729

CyrusNajmabadi commented Apr 4, 2024

View reviewed changes

ASserts

d869168

CyrusNajmabadi commented Apr 4, 2024

View reviewed changes

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs Outdated Show resolved Hide resolved

CyrusNajmabadi added 4 commits April 4, 2024 13:10

Update src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProv…

7c8bf35

…ider.TagSource_ProduceTags.cs

Simplify

3ec3530

Merge branch 'frozenPartialTagging' of https://github.com/CyrusNajmab…

f554737

…adi/roslyn into frozenPartialTagging

move helpers

24c34e5

CyrusNajmabadi commented Apr 4, 2024

View reviewed changes

indentatin

3de1764

CyrusNajmabadi commented Apr 4, 2024

View reviewed changes

voidresult

5ae41b5

CyrusNajmabadi commented Apr 5, 2024

View reviewed changes

src/EditorFeatures/Core/ReferenceHighlighting/ReferenceHighlightingViewTaggerProvider.cs Outdated Show resolved Hide resolved

CyrusNajmabadi added 6 commits April 5, 2024 09:48

Update src/EditorFeatures/Core/ReferenceHighlighting/ReferenceHighlig…

05d0c6f

…htingViewTaggerProvider.cs

voidresult

df8d68a

Merge branch 'frozenPartialTagging' of https://github.com/CyrusNajmab…

492c3fc

…adi/roslyn into frozenPartialTagging

Simplify

f31e40f

REvert

6e8da4c

revert

b78d0a8

sharwell reviewed Apr 5, 2024

View reviewed changes

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs Outdated Show resolved Hide resolved

remove redundant check

c58a604

sharwell reviewed Apr 5, 2024

View reviewed changes

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.cs Outdated Show resolved Hide resolved

sharwell reviewed Apr 5, 2024

View reviewed changes

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs Show resolved Hide resolved

Docs

a9b19d3

CyrusNajmabadi requested a review from sharwell April 5, 2024 17:44

sharwell reviewed Apr 5, 2024

View reviewed changes

src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.cs Outdated Show resolved Hide resolved

sharwell reviewed Apr 5, 2024

View reviewed changes

sharwell approved these changes Apr 5, 2024

View reviewed changes

Update src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProv…

0e8d413

…ider.cs Co-authored-by: Sam Harwell <[email protected]>

CyrusNajmabadi enabled auto-merge April 5, 2024 18:44

genlu reviewed Apr 5, 2024

View reviewed changes

CyrusNajmabadi merged commit 85c6efb into dotnet:main Apr 5, 2024
25 of 27 checks passed

dotnet-policy-service bot added this to the Next milestone Apr 5, 2024

CyrusNajmabadi deleted the frozenPartialTagging branch April 5, 2024 20:05

This was referenced Apr 13, 2024

[Automated] PRs inserted in VS build main-34812.241 #73011

Closed

[Automated] PRs inserted in VS build feature.debugger.main-34815.163 #73037

Closed

CyrusNajmabadi mentioned this pull request Apr 20, 2024

No semantic syntax highlighting until a change is made #73119

Closed

dibarbet modified the milestones: Next, 17.11 P1 Apr 29, 2024

CyrusNajmabadi mentioned this pull request May 30, 2024

Perform nav bar computation in two passes #73786

Merged

CyrusNajmabadi mentioned this pull request Jun 10, 2024

Syntax coloring broken after updating to 17.10 #73862

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure taggers that compute frozen partial data eventually move to a 'correct' final state. #72878

Ensure taggers that compute frozen partial data eventually move to a 'correct' final state. #72878

CyrusNajmabadi commented Apr 4, 2024 •

edited by Pilchie

Loading

CyrusNajmabadi Apr 4, 2024

Pilchie commented Apr 4, 2024

CyrusNajmabadi Apr 4, 2024

CyrusNajmabadi Apr 4, 2024

CyrusNajmabadi Apr 4, 2024

CyrusNajmabadi Apr 4, 2024

CyrusNajmabadi Apr 4, 2024

sharwell Apr 5, 2024

genlu Apr 5, 2024

CyrusNajmabadi Apr 5, 2024

Ensure taggers that compute frozen partial data eventually move to a 'correct' final state. #72878

Ensure taggers that compute frozen partial data eventually move to a 'correct' final state. #72878

Conversation

CyrusNajmabadi commented Apr 4, 2024 • edited by Pilchie Loading

Choose a reason for hiding this comment

Pilchie commented Apr 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusNajmabadi commented Apr 4, 2024 •

edited by Pilchie

Loading