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

Further optimisations for compilation events triggered by the language server. #5445

Open
11 of 12 tasks
JoshuaBatty opened this issue Jan 10, 2024 · 0 comments
Open
11 of 12 tasks
Assignees
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
Copy link
Member

JoshuaBatty commented Jan 10, 2024

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 in sway-lsp tests.

@JoshuaBatty JoshuaBatty self-assigned this Jan 10, 2024
@JoshuaBatty 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
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
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.
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 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`
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>
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
Projects
None yet
Development

No branches or pull requests

2 participants