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

Enable LSP Garbage Collection test for CI #5704

Merged
merged 7 commits into from
Mar 10, 2024
Merged

Enable LSP Garbage Collection test for CI #5704

merged 7 commits into from
Mar 10, 2024

Conversation

JoshuaBatty
Copy link
Member

@JoshuaBatty JoshuaBatty commented Mar 7, 2024

Description

Enables a did_change stress test with random wait times between each trigger to emulate random key typing. This then internally kicks off compilation and triggers garbage collection every 3 keystrokes.

I had intended to include this test when garbage collection was implemented in #5251. However, at that time, we were only performing GC every 10th keystroke and it was overloading the stack in CI. Now that we have reduced this to every 3rd keystroke it seems to be fine for CI.

Unfortunately, this test wasn't running to catch a bug that slipped through in #5306. As such, this PR currently allows a way for us to reproduce this error but won't be able to pass CI until the underlying issue is resolved.

P.S.: This also adds a temporary fix where we retain elements without a source id. This is tantamount to leaking them but seems preferable to disabling the GC altogether.

@JoshuaBatty JoshuaBatty marked this pull request as draft March 7, 2024 02:54
Copy link

github-actions bot commented Mar 7, 2024

Benchmark for 6248fd0

Click to view benchmark
Test Base PR %
code_action 5.3±0.14ms 5.3±0.17ms 0.00%
code_lens 287.0±8.63ns 287.5±6.86ns +0.17%
compile 5.7±0.10s 5.6±0.10s -1.75%
completion 4.8±0.12ms 4.9±0.11ms +2.08%
did_change_with_caching 5.1±0.02s 5.2±0.07s +1.96%
document_symbol 957.4±20.14µs 953.9±10.13µs -0.37%
format 69.2±1.44ms 68.2±1.59ms -1.45%
goto_definition 358.5±10.11µs 361.8±7.29µs +0.92%
highlight 9.1±0.14ms 9.1±0.13ms 0.00%
hover 582.7±5.73µs 577.8±6.72µs -0.84%
idents_at_position 122.7±1.02µs 121.0±1.13µs -1.39%
inlay_hints 663.7±11.43µs 671.9±18.78µs +1.24%
on_enter 486.9±13.90ns 484.4±15.39ns -0.51%
parent_decl_at_position 3.7±0.19ms 3.7±0.03ms 0.00%
prepare_rename 354.6±6.63µs 363.6±5.02µs +2.54%
rename 9.4±0.16ms 9.5±0.22ms +1.06%
semantic_tokens 995.1±20.10µs 1028.0±19.61µs +3.31%
token_at_position 355.7±2.64µs 362.3±4.36µs +1.86%
tokens_at_position 3.7±0.04ms 3.7±0.04ms 0.00%
tokens_for_file 410.1±1.88µs 414.9±3.82µs +1.17%
traverse 41.7±1.27ms 41.8±1.18ms +0.24%

@JoshuaBatty JoshuaBatty self-assigned this Mar 7, 2024
@JoshuaBatty JoshuaBatty added the ci label Mar 7, 2024
Copy link

github-actions bot commented Mar 7, 2024

Benchmark for c737ffc

Click to view benchmark
Test Base PR %
code_action 5.4±0.02ms 5.4±0.02ms 0.00%
code_lens 288.1±8.77ns 288.2±4.57ns +0.03%
compile 5.7±0.06s 5.7±0.07s 0.00%
completion 4.9±0.09ms 4.9±0.02ms 0.00%
did_change_with_caching 5.1±0.03s 5.1±0.03s 0.00%
document_symbol 1022.5±30.05µs 967.1±19.16µs -5.42%
format 70.7±0.55ms 70.5±1.30ms -0.28%
goto_definition 355.5±9.68µs 368.4±18.79µs +3.63%
highlight 9.1±0.15ms 9.1±0.16ms 0.00%
hover 585.6±5.47µs 586.1±5.96µs +0.09%
idents_at_position 122.5±0.44µs 122.9±1.59µs +0.33%
inlay_hints 658.6±22.79µs 664.9±12.41µs +0.96%
on_enter 507.6±21.74ns 496.1±39.67ns -2.27%
parent_decl_at_position 3.7±0.04ms 3.7±0.03ms 0.00%
prepare_rename 359.5±4.72µs 359.8±4.58µs +0.08%
rename 9.5±0.18ms 9.5±0.22ms 0.00%
semantic_tokens 1006.7±17.91µs 1061.1±53.61µs +5.40%
token_at_position 347.0±1.70µs 361.0±2.24µs +4.03%
tokens_at_position 3.7±0.02ms 3.7±0.03ms 0.00%
tokens_for_file 404.3±1.54µs 407.0±2.07µs +0.67%
traverse 42.0±1.74ms 42.1±1.57ms +0.24%

Copy link

github-actions bot commented Mar 7, 2024

Benchmark for 284a9c6

Click to view benchmark
Test Base PR %
code_action 5.3±0.18ms 5.2±0.15ms -1.89%
code_lens 281.2±12.47ns 279.6±12.39ns -0.57%
compile 5.5±0.10s 5.5±0.12s 0.00%
completion 4.7±0.08ms 4.7±0.09ms 0.00%
did_change_with_caching 5.0±0.07s 4.9±0.03s -2.00%
document_symbol 918.5±31.26µs 1048.0±49.13µs +14.10%
format 68.2±1.67ms 68.1±1.63ms -0.15%
goto_definition 351.0±8.02µs 351.3±10.31µs +0.09%
highlight 8.9±0.18ms 8.8±0.29ms -1.12%
hover 600.4±23.23µs 562.7±13.04µs -6.28%
idents_at_position 118.5±2.50µs 119.2±1.96µs +0.59%
inlay_hints 660.7±16.68µs 635.6±27.98µs -3.80%
on_enter 479.2±12.83ns 480.9±21.62ns +0.35%
parent_decl_at_position 3.6±0.09ms 3.6±0.05ms 0.00%
prepare_rename 345.5±7.47µs 348.7±9.00µs +0.93%
rename 9.3±0.14ms 9.1±0.18ms -2.15%
semantic_tokens 1016.6±22.14µs 1008.0±30.66µs -0.85%
token_at_position 360.2±7.52µs 344.4±5.78µs -4.39%
tokens_at_position 3.6±0.05ms 3.6±0.05ms 0.00%
tokens_for_file 390.2±7.12µs 404.8±8.74µs +3.74%
traverse 41.3±1.19ms 40.6±1.17ms -1.69%

@IGI-111 IGI-111 marked this pull request as ready for review March 7, 2024 09:25
IGI-111
IGI-111 previously approved these changes Mar 7, 2024
@IGI-111 IGI-111 requested a review from a team March 7, 2024 09:34
xunilrj
xunilrj previously approved these changes Mar 7, 2024
@JoshuaBatty JoshuaBatty dismissed stale reviews from xunilrj and IGI-111 via 53a641f March 7, 2024 22:46
@JoshuaBatty JoshuaBatty requested review from xunilrj, IGI-111 and a team March 7, 2024 22:48
kayagokalp
kayagokalp previously approved these changes Mar 7, 2024
@JoshuaBatty JoshuaBatty requested a review from a team March 7, 2024 23:07
Copy link

github-actions bot commented Mar 7, 2024

Benchmark for 8415a17

Click to view benchmark
Test Base PR %
code_action 5.4±0.14ms 5.4±0.05ms 0.00%
code_lens 289.3±12.06ns 287.3±6.74ns -0.69%
compile 5.7±0.06s 5.7±0.08s 0.00%
completion 4.9±0.02ms 4.9±0.05ms 0.00%
did_change_with_caching 5.2±0.03s 5.2±0.04s 0.00%
document_symbol 947.3±15.85µs 952.0±8.71µs +0.50%
format 69.7±0.66ms 70.2±2.15ms +0.72%
goto_definition 363.9±5.00µs 368.6±3.45µs +1.29%
highlight 9.2±0.18ms 9.1±0.10ms -1.09%
hover 582.1±10.15µs 587.0±5.52µs +0.84%
idents_at_position 123.1±0.29µs 123.5±0.44µs +0.32%
inlay_hints 671.7±12.45µs 674.3±14.37µs +0.39%
on_enter 488.5±18.67ns 489.1±14.22ns +0.12%
parent_decl_at_position 3.7±0.02ms 3.7±0.02ms 0.00%
prepare_rename 358.1±7.49µs 367.2±4.95µs +2.54%
rename 9.5±0.18ms 9.5±0.01ms 0.00%
semantic_tokens 1038.6±16.72µs 1059.8±10.76µs +2.04%
token_at_position 366.2±2.60µs 360.8±2.15µs -1.47%
tokens_at_position 3.7±0.03ms 3.7±0.02ms 0.00%
tokens_for_file 412.7±6.13µs 410.7±2.50µs -0.48%
traverse 43.3±2.90ms 42.8±1.97ms -1.15%

sdankel
sdankel previously approved these changes Mar 7, 2024
@kayagokalp kayagokalp changed the base branch from master to kayagokalp/0.49.3 March 8, 2024 06:05
@kayagokalp kayagokalp changed the base branch from kayagokalp/0.49.3 to master March 8, 2024 06:06
@kayagokalp kayagokalp dismissed stale reviews from sdankel and themself March 8, 2024 06:06

The base branch was changed.

Copy link

github-actions bot commented Mar 8, 2024

Benchmark for bc518e1

Click to view benchmark
Test Base PR %
code_action 6.0±0.21ms 5.4±0.15ms -10.00%
code_lens 294.7±27.21ns 286.6±8.34ns -2.75%
compile 6.0±0.13s 6.2±0.08s +3.33%
completion 5.0±0.06ms 5.0±0.15ms 0.00%
did_change_with_caching 5.4±0.13s 5.8±0.19s +7.41%
document_symbol 991.8±12.75µs 948.6±24.28µs -4.36%
format 70.2±1.48ms 71.9±2.55ms +2.42%
goto_definition 374.3±9.04µs 370.4±25.34µs -1.04%
highlight 9.3±0.12ms 9.1±0.20ms -2.15%
hover 602.7±30.26µs 586.8±6.62µs -2.64%
idents_at_position 124.3±4.48µs 123.7±6.05µs -0.48%
inlay_hints 674.2±8.27µs 684.8±8.94µs +1.57%
on_enter 494.7±30.73ns 499.9±33.19ns +1.05%
parent_decl_at_position 3.7±0.05ms 3.9±0.43ms +5.41%
prepare_rename 368.7±5.93µs 361.3±5.25µs -2.01%
rename 10.3±1.05ms 9.8±0.44ms -4.85%
semantic_tokens 1063.2±13.58µs 1088.6±43.55µs +2.39%
token_at_position 359.2±3.67µs 362.3±3.23µs +0.86%
tokens_at_position 3.7±0.11ms 3.8±0.15ms +2.70%
tokens_for_file 409.4±8.00µs 409.0±2.67µs -0.10%
traverse 44.6±1.79ms 47.2±1.32ms +5.83%

@IGI-111 IGI-111 merged commit 421caef into master Mar 10, 2024
35 checks passed
@IGI-111 IGI-111 deleted the josh/gc_test branch March 10, 2024 13:17
JoshuaBatty added a commit that referenced this pull request Mar 14, 2024
## Description
Removes comments added in #5704. The change setting None cases to true
is actually correct and what we want. @danielbate
JoshuaBatty added a commit that referenced this pull request Mar 14, 2024
## Description
This test was originally added in #5704. The initial ambition of
compiling 400 times, aimed at rigorously evaluating the garbage
collector's resilience, inadvertently led to stack overflows and adverse
interactions when executed alongside concurrent tests. To address these
issues while preserving the test's core objectives, the following
modifications have been made:

Compilation Frequency Reduction: The iteration count for compilations
has been adjusted from 400 to 60. This change significantly accelerates
test completion without compromising the test's ability to effectively
assess garbage collector performance and stability.

Runtime Isolation: Transitioning from #[tokio::test] to a standard
#[test] annotation, we now initiate a dedicated Tokio runtime within the
test. This approach prevents the test from sharing the runtime
environment with other tests, thereby eliminating concurrent execution
and its associated complexities.

---------

Co-authored-by: Sophie Dankel <[email protected]>
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.

6 participants