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.

Sign up for GitHub

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

Jump to bottom

change submodules to use im::HashMap instead of im::OrdMap #5976

Merged
merged 7 commits into from
May 9, 2024
Merged

change submodules to use im::HashMap instead of im::OrdMap #5976

merged 7 commits into from
May 9, 2024

Conversation

JoshuaBatty
Copy link
Member

@JoshuaBatty JoshuaBatty commented May 9, 2024 •
edited
Loading

Description

The im docs state that the performance of HashMap is similar to that found in std but the ImOrd is between 2-3x slower than BTreeMap. Changing submodules from using im::OrdMap to using im::HashMap is giving us a nice performance win.

I tried compiling the libraries project from fluid protocol as a test, it has 19 seperate libraries. This change gives us a 21.25% speed increase.

Before: Elapsed time: 27.968193458s
After: Elapsed time: 22.02603425s

Running the compile benchmark in LSP is showing a 29% improvement.

Before: Elapsed time: 3.8328s
After: Elapsed time: 2.7086s

@JoshuaBatty JoshuaBatty changed the title change submodules to use im::HashMap instead of im::OrdMap change submodules to use im::HashMap instead of im::OrdMap May 9, 2024
@JoshuaBatty JoshuaBatty self-assigned this May 9, 2024
@JoshuaBatty JoshuaBatty added the compiler General compiler. Should eventually become more specific as the issue is triaged label May 9, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024

Benchmark for 3a67701

Click to view benchmark
Test Base PR %
code_action 5.5±0.03ms 5.3±0.16ms -3.64%
code_lens 292.8±9.62ns 290.6±11.80ns -0.75%
compile 6.8±0.10s 5.0±0.25s -26.47%
completion 5.0±0.04ms 4.9±0.09ms -2.00%
did_change_with_caching 6.2±0.05s 4.4±0.14s -29.03%
document_symbol 944.3±21.37µs 1016.2±38.48µs +7.61%
format 90.4±1.45ms 89.0±1.53ms -1.55%
goto_definition 352.3±9.35µs 353.2±5.38µs +0.26%
highlight 9.1±0.12ms 8.7±0.25ms -4.40%
hover 590.8±8.34µs 593.1±8.03µs +0.39%
idents_at_position 119.1±0.36µs 119.7±1.46µs +0.50%
inlay_hints 712.9±43.37µs 641.9±27.26µs -9.96%
on_enter 498.7±19.58ns 485.1±12.96ns -2.73%
parent_decl_at_position 3.7±0.03ms 3.6±0.06ms -2.70%
prepare_rename 347.5±5.24µs 344.5±11.89µs -0.86%
rename 9.6±0.03ms 9.3±0.20ms -3.12%
semantic_tokens 1029.7±12.66µs 1039.8±34.67µs +0.98%
token_at_position 351.5±1.36µs 340.5±1.63µs -3.13%
tokens_at_position 3.7±0.01ms 3.6±0.04ms -2.70%
tokens_for_file 427.4±3.15µs 414.3±3.55µs -3.07%
traverse 51.9±2.45ms 52.3±2.00ms +0.77%

@JoshuaBatty JoshuaBatty requested review from a team May 9, 2024 01:03
@JoshuaBatty JoshuaBatty marked this pull request as draft May 9, 2024 02:00
@JoshuaBatty JoshuaBatty force-pushed the josh/im_hash branch 2 times, most recently from fda9b01 to 0d71bf2 Compare May 9, 2024 03:55
@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024

Benchmark for b2e4bc9

Click to view benchmark
Test Base PR %
code_action 5.5±0.07ms 5.5±0.07ms 0.00%
code_lens 287.5±7.08ns 292.0±26.65ns +1.57%
compile 6.9±0.18s 5.0±0.08s -27.54%
completion 5.0±0.05ms 4.9±0.08ms -2.00%
did_change_with_caching 6.2±0.09s 4.6±0.07s -25.81%
document_symbol 1019.6±23.67µs 1030.0±15.26µs +1.02%
format 90.9±1.61ms 88.5±1.47ms -2.64%
goto_definition 352.3±6.99µs 345.9±9.03µs -1.82%
highlight 9.1±0.04ms 9.0±0.12ms -1.10%
hover 587.3±10.34µs 588.9±8.02µs +0.27%
idents_at_position 119.9±0.49µs 119.9±1.72µs 0.00%
inlay_hints 651.7±6.38µs 650.4±25.75µs -0.20%
on_enter 497.6±15.51ns 475.2±17.72ns -4.50%
parent_decl_at_position 3.7±0.03ms 3.7±0.19ms 0.00%
prepare_rename 352.2±6.93µs 348.5±6.35µs -1.05%
rename 9.7±0.14ms 9.6±0.18ms -1.03%
semantic_tokens 1042.5±11.49µs 1027.3±14.80µs -1.46%
token_at_position 343.0±2.31µs 344.2±1.98µs +0.35%
tokens_at_position 3.7±0.03ms 3.7±0.03ms 0.00%
tokens_for_file 419.1±2.86µs 419.6±5.05µs +0.12%
traverse 51.3±2.54ms 50.6±1.67ms -1.36%

@JoshuaBatty JoshuaBatty marked this pull request as ready for review May 9, 2024 05:18
@JoshuaBatty JoshuaBatty requested review from a team and IGI-111 May 9, 2024 05:18
@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024

Benchmark for dd91662

Click to view benchmark
Test Base PR %
code_action 5.5±0.01ms 5.5±0.03ms 0.00%
code_lens 288.8±6.06ns 288.3±19.54ns -0.17%
compile 6.7±0.10s 5.0±0.07s -25.37%
completion 5.0±0.11ms 5.0±0.09ms 0.00%
did_change_with_caching 6.2±0.06s 4.6±0.07s -25.81%
document_symbol 1069.7±21.43µs 1003.7±68.68µs -6.17%
format 88.4±1.76ms 88.2±1.24ms -0.23%
goto_definition 352.6±13.20µs 351.0±5.85µs -0.45%
highlight 9.1±0.11ms 9.1±0.20ms 0.00%
hover 587.5±6.30µs 600.5±11.35µs +2.21%
idents_at_position 121.4±0.92µs 120.1±0.87µs -1.07%
inlay_hints 656.3±31.03µs 658.7±23.58µs +0.37%
on_enter 500.6±10.90ns 464.8±17.43ns -7.15%
parent_decl_at_position 3.7±0.06ms 3.7±0.04ms 0.00%
prepare_rename 346.7±5.15µs 347.4±7.90µs +0.20%
rename 9.7±0.10ms 9.7±0.04ms 0.00%
semantic_tokens 1025.1±9.09µs 1055.2±10.68µs +2.94%
token_at_position 355.0±3.89µs 340.8±2.44µs -4.00%
tokens_at_position 3.8±0.09ms 3.7±0.06ms -2.63%
tokens_for_file 420.1±3.06µs 423.9±1.82µs +0.90%
traverse 50.7±1.49ms 51.5±2.11ms +1.58%

@JoshuaBatty JoshuaBatty mentioned this pull request May 9, 2024
Merged
@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024

Benchmark for 83566b1

Click to view benchmark
Test Base PR %
code_action 5.3±0.13ms 5.5±0.06ms +3.77%
code_lens 295.8±19.04ns 288.4±4.76ns -2.50%
compile 3.9±0.04s 3.5±0.04s -10.26%
completion 4.7±0.06ms 4.7±0.07ms 0.00%
did_change_with_caching 3.4±0.07s 2.8±0.05s -17.65%
document_symbol 993.3±37.62µs 988.6±19.49µs -0.47%
format 90.5±1.49ms 91.1±3.92ms +0.66%
goto_definition 357.7±9.39µs 364.9±8.09µs +2.01%
highlight 9.0±0.15ms 9.1±0.22ms +1.11%
hover 473.6±7.83µs 488.5±10.11µs +3.15%
idents_at_position 124.2±1.62µs 122.1±1.22µs -1.69%
inlay_hints 646.1±17.72µs 661.6±10.65µs +2.40%
on_enter 480.9±12.25ns 503.9±12.81ns +4.78%
parent_decl_at_position 3.7±0.07ms 3.7±0.05ms 0.00%
prepare_rename 349.8±8.08µs 368.0±7.50µs +5.20%
rename 9.6±0.13ms 9.8±0.10ms +2.08%
semantic_tokens 1020.5±20.94µs 1071.3±51.23µs +4.98%
token_at_position 361.3±4.96µs 363.7±3.35µs +0.66%
tokens_at_position 3.7±0.03ms 3.8±0.02ms +2.70%
tokens_for_file 412.7±6.72µs 428.8±4.14µs +3.90%
traverse 41.3±1.12ms 40.4±0.84ms -2.18%

@JoshuaBatty JoshuaBatty requested a review from a team May 9, 2024 22:14
@JoshuaBatty JoshuaBatty enabled auto-merge (squash) May 9, 2024 22:28
@JoshuaBatty JoshuaBatty merged commit 9e51a28 into master May 9, 2024
38 checks passed
@JoshuaBatty JoshuaBatty deleted the josh/im_hash branch May 9, 2024 22:43
@JoshuaBatty JoshuaBatty mentioned this pull request May 10, 2024
Merged
JoshuaBatty added a commit that referenced this pull request May 10, 2024
@JoshuaBatty
Make ControlFlowGraph store a reference to Engines to remove clones (#…
e2f55b8 
…5983)

## Description
Using the same performance test as in #5976 and #5978, this change gives
a further 21.33% performance improvement.

Before: Elapsed time: `14.653166459s`
After: Elapsed time: `11.527667375s`
@IGI-111 IGI-111 added the breaking May cause existing user code to break. Requires a minor or major release. label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IGI-111 IGI-111 IGI-111 approved these changes

@tritao tritao tritao approved these changes

Assignees

@JoshuaBatty JoshuaBatty

Labels
breaking May cause existing user code to break. Requires a minor or major release. compiler General compiler. Should eventually become more specific as the issue is triaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JoshuaBatty @tritao @IGI-111