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

Continue even if the lsp cannot acquire an exclusive lock #5740

Merged
merged 2 commits into from
Mar 18, 2024

Conversation

crodas
Copy link
Contributor

@crodas crodas commented Mar 17, 2024

Description

Before this PR the lsp will throw an error if it can't acquire a lock, instead, log the error and continue gracefully.

Checklist

  • 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.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

Before this PR the lsp will throw an error if it can't acquire a lock,
instead, log the error and continue gracefully.
@crodas crodas requested review from JoshuaBatty and a team March 17, 2024 13:12
@crodas crodas self-assigned this Mar 17, 2024
@crodas crodas enabled auto-merge (squash) March 17, 2024 13:13
Copy link

Benchmark for 18c2daa

Click to view benchmark
Test Base PR %
code_action 5.5±0.14ms 5.7±0.17ms +3.64%
code_lens 286.7±8.32ns 290.1±5.22ns +1.19%
compile 6.4±0.10s 6.5±0.09s +1.56%
completion 4.9±0.03ms 4.9±0.02ms 0.00%
did_change_with_caching 5.6±0.05s 5.7±0.08s +1.79%
document_symbol 960.8±19.12µs 1025.3±43.91µs +6.71%
format 70.9±1.31ms 71.4±1.45ms +0.71%
goto_definition 366.6±10.77µs 371.5±11.03µs +1.34%
highlight 9.2±0.03ms 9.1±0.02ms -1.09%
hover 594.3±10.16µs 654.9±15.17µs +10.20%
idents_at_position 122.1±2.82µs 122.0±2.78µs -0.08%
inlay_hints 681.3±19.88µs 667.3±23.60µs -2.05%
on_enter 483.8±12.88ns 484.2±5.96ns +0.08%
parent_decl_at_position 3.7±0.06ms 3.7±0.07ms 0.00%
prepare_rename 364.3±9.02µs 366.1±9.17µs +0.49%
rename 9.8±0.14ms 9.8±0.09ms 0.00%
semantic_tokens 1051.4±16.47µs 1028.1±40.69µs -2.22%
token_at_position 357.1±2.74µs 357.3±2.25µs +0.06%
tokens_at_position 3.7±0.02ms 3.7±0.05ms 0.00%
tokens_for_file 418.5±2.69µs 410.0±8.52µs -2.03%
traverse 45.4±1.60ms 46.0±2.02ms +1.32%

@JoshuaBatty JoshuaBatty requested a review from a team March 17, 2024 22:18
@JoshuaBatty JoshuaBatty added the language server LSP server label Mar 17, 2024
Copy link

Benchmark for 0cd0737

Click to view benchmark
Test Base PR %
code_action 5.3±0.15ms 5.3±0.08ms 0.00%
code_lens 287.1±12.13ns 287.8±11.41ns +0.24%
compile 6.0±0.07s 6.0±0.13s 0.00%
completion 4.8±0.06ms 5.0±0.17ms +4.17%
did_change_with_caching 5.5±0.07s 5.5±0.06s 0.00%
document_symbol 982.4±24.06µs 951.7±28.67µs -3.12%
format 70.8±1.46ms 70.3±1.44ms -0.71%
goto_definition 366.0±6.70µs 364.9±10.91µs -0.30%
highlight 8.8±0.21ms 8.8±0.12ms 0.00%
hover 595.4±18.86µs 603.4±24.63µs +1.34%
idents_at_position 121.5±0.31µs 121.7±1.16µs +0.16%
inlay_hints 647.4±24.86µs 677.9±25.49µs +4.71%
on_enter 490.7±16.50ns 483.2±13.33ns -1.53%
parent_decl_at_position 3.6±0.06ms 3.6±0.06ms 0.00%
prepare_rename 352.3±8.92µs 369.4±8.89µs +4.85%
rename 9.2±0.08ms 9.3±0.15ms +1.09%
semantic_tokens 1033.8±21.57µs 1049.9±17.92µs +1.56%
token_at_position 351.4±3.26µs 358.7±19.48µs +2.08%
tokens_at_position 3.6±0.03ms 3.6±0.02ms 0.00%
tokens_for_file 418.2±1.87µs 412.3±4.21µs -1.41%
traverse 44.7±1.30ms 45.4±1.66ms +1.57%

@crodas crodas merged commit bf8d8a4 into master Mar 18, 2024
32 of 35 checks passed
@crodas crodas deleted the improve-lsp-locking-failure branch March 18, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants