Skip to content

perf(workspace): don't re-run the analyser on every code action request - #1004

Open
psteinroe wants to merge 2 commits into
refactor/document-cache-modulefrom
perf/cache-document-analysis
Open

psteinroe wants to merge 2 commits into
refactor/document-cache-modulefrom
perf/cache-document-analysis

Conversation

@psteinroe

Copy link
Copy Markdown
Collaborator

Stacked on #1003. Part of the rebuild described in #997 (point 4). Replaces #929.

Editors request code actions on every cursor move, and each request ran the whole analysis of the file: every lint and typecheck rule, over every statement. Right after an edit, the diagnostics had already run the same analysis. On a file with 20,000 statements that took seconds per cursor move.

What changed

  • DocumentCache gets the caches that depend on more than the text of the document, under one CacheKey: the settings version, the project whose settings apply, and the catalog base (whose identity is the database snapshot's). When the key of a request differs from the cached one, all of them are dropped together. The settings version is bumped while the settings are write-locked (workspaces_mut), so nothing is cached for the old settings under the new version. The project is part of the key because a file can be opened by two projects with different settings (nested workspace folders).
  • The first of these caches is the analysis. The analysis that the editor asks for, with every rule category and no only/skip selection, is kept until the content changes (Document::update_content) or the key does. Requests with a rule selection, like check --only or fix_file with only, are not cached.
  • The analysis moves out of server.rs into server/analysis.rs: analyse_document returns an Arc<DocumentAnalysis>, whose fields are private, with what the diagnostics need (lint_diagnostics, syntax_diagnostics, disabled_rules, is_skipped, statements) instead of a four-element tuple. pull_file_diagnostics and the database checks read it by reference, and the lint code actions look at the diagnostics under the cursor without copying the others. LinterDiagnostic and its advices derive Clone, so that the diagnostics can be converted out of the shared analysis, and RuleCategories derives PartialEq to recognise the request for every category.
  • The LSP handler now honours context.only for all actions (it already did for the fix-all action). Kinds are matched hierarchically, so quickfix includes quickfix.pgls.banDropTable, and a request for quick fixes doesn't get the Execute Statement command.

EXPLAIN and plpgsql_check diagnostics are not part of the cached analysis and are not affected.

Tests

  • code_actions_reuse_the_analysis: the code actions use the analysis that the diagnostics cached (the same Arc), an edit drops it, and turning a rule off in the settings removes its actions.
  • test_lint_fix_code_actions (LSP): with only: [quickfix] only the fix and the suppression come back, and with only: [source.fixAll] nothing does.
  • requested_kinds_are_hierarchical: the kind matching, including "", a prefix that isn't a whole segment, and the empty kind of the commands.
  • pgls_workspace (including projects_have_their_own_nested_settings, which caught the project missing from the key) and pgls_lsp pass, and clippy on the whole workspace.

Cost

Memory: each open document keeps its last analysis (its diagnostics and per-statement facts) until the next edit, a settings change or a new snapshot. A budget across documents follows later in the stack. Timings for the whole stack are in its last PR.

The analysis that the editor asks for, with every rule category and no
rule selection, is kept in the document's cache under a CacheKey of the
settings version, the project and the catalog base. It is dropped when
the content changes. Code actions read it by reference instead of
analysing the whole file on every cursor move. The analysis moves to
server/analysis.rs as a DocumentAnalysis with private fields.

The LSP now honours the code action context's only kinds.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant