-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Further optimisations for compilation events triggered by the language server. #5445
Open
11 of 12 tasks
Labels
compiler: frontend
Everything to do with type checking, control flow analysis, and everything between parsing and IRgen
epic
An epic is a high-level master issue for large pieces of work.
language server
LSP server
Comments
JoshuaBatty
added
language server
LSP server
compiler: frontend
Everything to do with type checking, control flow analysis, and everything between parsing and IRgen
labels
Jan 10, 2024
This was referenced Jan 15, 2024
JoshuaBatty
added a commit
that referenced
this issue
Jan 17, 2024
## Description We clone the `Engines` before each keystroke in the language server. We shouldn't need to clone anything other than a pointer for the `programs_cache`. Measured against the `benchmark` example in `sway-lsp` tests, this saves us `23.015ms`. related to #5445 ## Checklist - [x] I have linked to any relevant issues. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
JoshuaBatty
added a commit
that referenced
this issue
Jan 17, 2024
…5473) ## Description Traversing the typed tokens from `sway-lib-std` now goes from `3.957833ms` → `1.372ms`. Collecting typed tokens on the benchmark project goes from `139.6ms` → `127.7ms`. I noticed that there are transient contentions when reading items from the `TokenMap` with this change. As such, I've implemented a new function on the `TokenMap` called `try_get_mut_with_retry`. This method tries to access the value up to 8 times if the lock is still held using the backoff and retry pattern. related to #5445
JoshuaBatty
added a commit
that referenced
this issue
Jan 17, 2024
…5487) ## Description Use `rayon` par_iters in the `traverse::typed_tree` module. We are already doing this in `traverse::parsed_tree`, this just brings this over to the typed_tree module as well. Branched off #5473. <s>Will want to merge that first then will take this out of draft.</s> Collecting typed tokens on the benchmark project goes from `120.508917ms` → `14.853125ms` related to #5445
This was referenced Jan 23, 2024
JoshuaBatty
added a commit
that referenced
this issue
Jan 25, 2024
…ectly (#5516) ## Description We no longer write TokenMap or Metrics into an intermediate copy and then back to the original in `write_parse_result`. Instead, we write to them directly during traversal if the compilation was successful. related to #5445 | Metric | Before | After | Change (%) | |----------|-----------|-----------|------------------| | traverse & write | 122.018ms | 74.353ms | -39.07% |
JoshuaBatty
added a commit
that referenced
this issue
Jan 25, 2024
…refs instead. (#5509) ## Description This PR refactors the functions implemented on `TokenMap` to remove clones where possible, and instead return the `RefMulti` type from `DashMap` directly. As can be seen below, we are getting significant performance wins and most LSP capabilities are now measured in µs. 🚀 related to #5445 | Metric | Before | After | Change (%) | |---------------------------|-----------------|---------------|------------| | tokens_for_file | 9.8117 ms | 253.845 µs | -97.411% | | idents_at_position | 3.6331 ms | 47.870 µs | -98.692% | | tokens_at_position | 6.9532 ms | 1.1478 ms | -83.474% | | token_at_position | 5.9395 ms | 208.23 µs | -96.483% | | parent_decl_at_position | 6.9780 ms | 1.1633 ms | -83.428% | | semantic_tokens | 18.2205 ms | 587.64 µs | -96.767% | | document_symbol | 6.6865 ms | 584.33 µs | -91.186% | | completion | 14.0025 ms | 1.9808 ms | -85.855% | | hover | 6.2409 ms | 317.90 µs | -94.902% | | highlight | 14.8390 ms | 2.7572 ms | -81.417% | | goto_definition | 5.9403 ms | 225.65 µs | -96.204% | | inlay_hints | 5.9934 ms | 318.41 µs | -94.677% | | prepare_rename | 5.8056 ms | 207.62 µs | -96.411% | | rename | 12.5200 ms | 2.8592 ms | -77.168% | | code_action | 11.0798 ms | 1.9683 ms | -82.233% | ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
7 tasks
JoshuaBatty
added a commit
that referenced
this issue
Feb 6, 2024
## Description This PR does 3 optimizations. The timings below are measured against the LSP benchmarking project. 1. Disable running DCA & control flow analysis, for did_change events `39.544ms` 2. Disable running collect_types_metadata for did_change events `3.522ms` 3. Only write the diagnostics res to self.diagnostics.write() on did_open & did_save `21.135ms` I also had to increase the frequency that we are calling GC to every 3rd keystroke as during stress tests I was occasionally getting stack overflows otherwise. related to #5445 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers.
sdankel
pushed a commit
that referenced
this issue
Feb 8, 2024
## Description This PR does 3 optimizations. The timings below are measured against the LSP benchmarking project. 1. Disable running DCA & control flow analysis, for did_change events `39.544ms` 2. Disable running collect_types_metadata for did_change events `3.522ms` 3. Only write the diagnostics res to self.diagnostics.write() on did_open & did_save `21.135ms` I also had to increase the frequency that we are calling GC to every 3rd keystroke as during stress tests I was occasionally getting stack overflows otherwise. related to #5445 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers.
sdankel
added
the
epic
An epic is a high-level master issue for large pieces of work.
label
Jun 4, 2024
IGI-111
pushed a commit
that referenced
this issue
Jul 22, 2024
## Description closes #5462 related #5445 before we were recreating the build plan on every keystroke. We now cache the result and reuse on subsequent did change events. We invalidate the cache if there has been any modifications to the `Forc.toml` file since the cache was created. Here are the timings when triggering this with a did_change event on the FUSD repo. previous: `80.146ms` with this change: `56µs`
esdrubal
pushed a commit
that referenced
this issue
Aug 13, 2024
## Description closes #5462 related #5445 before we were recreating the build plan on every keystroke. We now cache the result and reuse on subsequent did change events. We invalidate the cache if there has been any modifications to the `Forc.toml` file since the cache was created. Here are the timings when triggering this with a did_change event on the FUSD repo. previous: `80.146ms` with this change: `56µs`
8 tasks
JoshuaBatty
added a commit
that referenced
this issue
Aug 22, 2024
## Description Implemented a caching mechanism for manifest directory lookups using a DashMap. This optimization reduces average lookup time from approximately 421µs to 0.47µs, resulting in an 895x speed improvement for subsequent calls. part of #5445 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. Co-authored-by: Sophie Dankel <47993817+sdankel@users.noreply.github.com> Co-authored-by: IGI-111 <igi-111@protonmail.com>
8 tasks
JoshuaBatty
added a commit
that referenced
this issue
Aug 29, 2024
…6464) ## Description This PR introduces significant optimisations in how we handle `TokenMap` updates and file traversals, resulting in substantial performance improvements. #### Key Changes: 1. Selective TokenMap Clearing: Instead of clearing the entire `TokenMap` on each keystroke, we now only clear tokens belonging to the modified file. 2. Targeted Traversal: We've limited the traversal to only the modified file, eliminating unnecessary processing of unchanged dependencies and workspace files. #### Performance Impact: These optimisations have led to substantial improvements in traverse times, particularly for subsequent traversals: | Build Type | Before (ms) | After (ms) | Absolute Improvement (ms) | Relative Improvement (%) | |------------|-------------|------------|---------------------------|--------------------------| | Release | 16.218875 | 4.758958 | 11.459917 | 70.66% | | Debug | 30.196084 | 7.144166 | 23.051918 | 76.34% | closes #6292 related to #5445 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
compiler: frontend
Everything to do with type checking, control flow analysis, and everything between parsing and IRgen
epic
An epic is a high-level master issue for large pieces of work.
language server
LSP server
Here is a list of optimisations that can be made to improve the responsiveness of the language server in response to did change events. Times are measured against the
benchmark
example insway-lsp
tests.programs_cache
from the QE around an Arc23.015ms
| Reduce cloning in the programs cache for LSP #555039.544ms
Disable DCA and writing diagnostics on did_change events #5555running collect_types_metadata
for didChange events3.522ms
Disable DCA and writing diagnostics on did_change events #5555self.diagnostics.write()
on didSave21.135ms
Disable DCA and writing diagnostics on did_change events #5555write_parse_result
| see if we can write to them directly.36.635ms
LSP Optimization: RemoveParseResult
and write to session types directly #5516document::mark_file_as_dirty
at the beginning of didChange708.625µs
cache pid lock files to reduce IO operations #6297uri_and_session_from_workspace
function by caching results575.667µs
Optimize manifest directory lookup with efficient caching #6444write_changes_to_file
func3.387ms
BuildPlan
instead of creating on before compilation2.425ms
CacheBuildPlan
in LSP #5462TokenMap
and traverse and repopulate the modules alongside the users workspace. We should only need to do this once. Optimise clearing and traversing code to only re-parse the modified file #62928.674ms
parse_ast_to_typed_tokens
using a rayon par iter? (139.6ms
→127.7ms
) | LSP Optimization: Use rayon forparse_ast_to_typed_tokens
function #5473traverse::typed_tree
module. We are doing this intraverse::parsed_tree
which takes11.3ms
while typed traverse takes140ms
| LSP Optimization: Add par_iters to thetraverse::typed_tree
module #5487The text was updated successfully, but these errors were encountered: