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

WIP: Ensure we always cache diag results. #73199

Closed
wants to merge 8 commits into from

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Apr 23, 2024

Followup to #73198

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

// Now, save all that data to the in-memory storage, so it can be retrieved later when making requests for
// other documents within the same project.
await SaveAllStatesToInMemoryStorageAsync().ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

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

@mavasani i was hoping this would help. By making it so that we always save for any caller of GetProjectAnalysisDataAsync (currently there are two of htem).

However, it looks like this doesn't help because the code that attempts to load from teh cache seems to bail out for any number of reasons i can't make heads or tails of.

// this is perf optimization. we cache these result since we know the result. (no diagnostics)
var activeAnalyzers = stateSets
.Select(s => s.Analyzer)
.Where(a => !a.IsOpenFileOnly(ideOptions.CleanupOptions?.SimplifierOptions));
Copy link
Member Author

Choose a reason for hiding this comment

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

@mavasani this part seems very weird to me. We would get the statesets... but then extract out the analyzers that we'd actually want to run. I tried to change this to just filter down teh statesets in the first place, makign it so that we have a 1:1 mapping from stateset to the analyzers we run, and so we wouldn't have a case where we were looking at statesets taht were actually running/computing/caching data because we filtered out their analyzer.

diagnostics.AddRange(analyzerResult.GetAllDiagnostics());
await state.SaveToInMemoryStorageAsync(project, analyzerResult).ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

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

this was removed because it happens within the call to GetProjectAnalysisDataAsync above now.

@CyrusNajmabadi
Copy link
Member Author

Closing out. we went with a project-level cache at the LSP pull diags level.

@CyrusNajmabadi CyrusNajmabadi deleted the pullDiagCache branch May 6, 2024 16:33
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.

None yet

1 participant