perf(lsp): Cache semantic tokens for open documents #23799
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dsherret
reviewed
May 14, 2024
| @@ -252,6 +262,7 @@ fn get_maybe_test_module_fut( | |||
| pub struct DocumentOpenData { | |||
| lsp_version: i32, | |||
| maybe_parsed_source: Option<ParsedSourceResult>, | |||
| maybe_semantic_tokens: Arc<Mutex<Option<lsp::SemanticTokens>>>, | |||
There was a problem hiding this comment.
Might be nice to add something like CloneCell<T: Send + Clone> to cli::util::sync that has methods like set(value: T) and get_cloned(&self): T in order to bury the mutex there.
There was a problem hiding this comment.
That said, should this use something like OnceCell instead? I guess no because async.
nayeemrmn
reviewed
May 14, 2024
| }), | ||
| ); | ||
|
|
||
| assert_eq!(res, res_cached); |
There was a problem hiding this comment.
How are we integration testing whether or not a cache is being used? Shouldn't the result be the same?
There was a problem hiding this comment.
Added some code to the test client to collect performance markers, so we can assert on how many times tsc.request.getEncodedSemanticClassifications has been called
bartlomieju
pushed a commit
that referenced
this pull request
May 16, 2024
VScode will typically send a `textDocument/semanticTokens/full` request
followed by `textDocument/semanticTokens/range`, and occassionally
request semantic tokens even when we know nothing has changed. Semantic
tokens also get refreshed on each change. Computing semantic tokens is
relatively heavy in TSC, so we should avoid it as much as possible.
Caches the semantic tokens for open documents, to avoid making TSC do
unnecessary work. Results in a noticeable improvement in local
benchmarking
before:
```
Starting Deno benchmark
-> Start benchmarking lsp
- Simple Startup/Shutdown
(10 runs, mean: 383ms)
- Big Document/Several Edits
(5 runs, mean: 1079ms)
- Find/Replace
(10 runs, mean: 59ms)
- Code Lens
(10 runs, mean: 440ms)
- deco-cx/apps Multiple Edits + Navigation
(5 runs, mean: 9921ms)
<- End benchmarking lsp
```
after:
```
Starting Deno benchmark
-> Start benchmarking lsp
- Simple Startup/Shutdown
(10 runs, mean: 395ms)
- Big Document/Several Edits
(5 runs, mean: 1024ms)
- Find/Replace
(10 runs, mean: 56ms)
- Code Lens
(10 runs, mean: 438ms)
- deco-cx/apps Multiple Edits + Navigation
(5 runs, mean: 8927ms)
<- End benchmarking lsp
```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
VScode will typically send a
textDocument/semanticTokens/fullrequest followed bytextDocument/semanticTokens/range, and occassionally request semantic tokens even when we know nothing has changed. Semantic tokens also get refreshed on each change. Computing semantic tokens is relatively heavy in TSC, so we should avoid it as much as possible.Caches the semantic tokens for open documents, to avoid making TSC do unnecessary work. Results in a noticeable improvement in local benchmarking
before:
after: