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

fix: semantic tokens performance #3932

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

mhuisi
Copy link
Contributor

@mhuisi mhuisi commented Apr 17, 2024

While implementing #3925, I noticed that the performance of the textDocument/semanticTokens/full request is extremely bad due to a quadratic implementation. Specifically, on my machine, computing the full semantic tokens for Lean/Elab/Do.lean took a full 5s. In practice, this means that while elaborating the file, one core is entirely busy with computing the semantic tokens for the file.

This PR fixes this performance bug by re-implementing the semantic token handling, reducing the latency for Lean/Elab/Do.lean from 5s to 60ms. As a result, the overly cautious refresh latency of 5s in #3925 can easily be reduced to 2s again.

Since the previous semantic tokens implementation used a very brittle hack to identify projections, this PR also changes the projection notation elaboration to augment the InfoTree syntax for the field of a projection with a special syntax node of kind Lean.Parser.Term.identProjKind. With this syntax kind, projection fields can now easily be identified in the InfoTree.

@mhuisi mhuisi requested a review from Kha as a code owner April 17, 2024 11:43
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Apr 17, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Apr 17, 2024

Mathlib CI status (docs):

  • ❗ Std/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase c51e4f57bd4debbe332e6b55039703531fdcc6fc --onto d3e004932c1f5ac3946850692940512d381c7634. (2024-04-17 12:01:18)
  • ✅ Mathlib branch lean-pr-testing-3932 has successfully built against this PR. (2024-04-17 16:09:22) View Log
  • ✅ Mathlib branch lean-pr-testing-3932 has successfully built against this PR. (2024-04-17 16:53:22) View Log

@mhuisi mhuisi force-pushed the mhuisi/semantic-highlighting-perf branch from c2fc7d4 to 869325a Compare April 17, 2024 12:07
src/Lean/Parser/Term.lean Outdated Show resolved Hide resolved
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 17, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Apr 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 17, 2024
@mhuisi mhuisi added this pull request to the merge queue Apr 18, 2024
Merged via the queue into leanprover:master with commit faa4d16 Apr 18, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants