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.

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

Perform nav bar computation in two passes #73786

Merged
merged 11 commits into from
May 31, 2024

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented May 30, 2024

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2065682

This follows the same change made in taggers back in #72878.

The idea is fairly simple. When computing nav bar items, we always first try a simple and quick pass using frozen-partial semantics. This allows the nav bars to be reasonably up to date with whatever the user jsut types, as well as with the last contents of SG documents and skeleton references.

As long as new changes are coming in, we keep performing those cheap computation passes.

When a cheap computation pass finishes, we then enqueue a request to do one final expensive pass that will actually update hte navbars with non-frozen data. Thus, ensuring that we always finally get into a final, correct, state. That expensive pass will be pre-empted by more cheap requests that come in. But we'll always eventually run that expensive/correct pass as the final thing we do.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner May 30, 2024 19:34
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels May 30, 2024
_ = _nonFrozenComputationCancellationSeries.CreateNext();
_computeModelQueue.AddWork(
new NavigationBarQueueItem(FrozenPartialSemantics: true, NonFrozenComputationToken: null),
cancelExistingWork: true);
Copy link
Member Author

Choose a reason for hiding this comment

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

mirrors what we do in tagging.

// to do the slow-but-accurate pass.
var frozenPartialSemantics = queueItems.Any(t => t.FrozenPartialSemantics);

if (!frozenPartialSemantics)
Copy link
Member Author

Choose a reason for hiding this comment

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

mirrors the same logic in tagging.

/// computing skeletons, SG docs, etc.), do the next cheap frozen-nav-bar-pass, and then push the
/// expensive-nonfrozen-nav-bar-pass to the end again.
/// </summary>
private readonly CancellationSeries _nonFrozenComputationCancellationSeries;
Copy link
Member Author

Choose a reason for hiding this comment

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

same pattern as tagging (including nameing).

@CyrusNajmabadi
Copy link
Member Author

@jasonmalinowski @sharwell @ToddGrun ptal. This impacts a large partner.

Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

It feels like there should be an easier way to do this. If a feature was added to the queue to just say "and run this once there are no batches left" it might be easier? I think? Either way since this pattern is established, no reason to try to delay this here.

@CyrusNajmabadi
Copy link
Member Author

It feels like there should be an easier way to do this. If a feature was added to the queue to just say "and run this once there are no batches left" it might be easier?

I'm waiting for the 3rd feature to need this. At that point, i'll wrap up in a helper (or make part of the queue itself :)).

@CyrusNajmabadi CyrusNajmabadi merged commit f1642d7 into dotnet:main May 31, 2024
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone May 31, 2024
@CyrusNajmabadi CyrusNajmabadi deleted the frozenNavBar branch May 31, 2024 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants