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

fix size of configurable buffer for enums #6366

Merged
merged 7 commits into from
Aug 7, 2024

Conversation

xunilrj
Copy link
Contributor

@xunilrj xunilrj commented Aug 6, 2024

Description

This PR fixes an issue where the encoded bytes for enums were not big enough for the biggest variant.

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.

@xunilrj xunilrj requested a review from a team as a code owner August 6, 2024 12:53
@xunilrj xunilrj self-assigned this Aug 6, 2024
Copy link

github-actions bot commented Aug 6, 2024

Benchmark for d3b9777

Click to view benchmark
Test Base PR %
code_action 5.2±0.04ms 5.2±0.22ms 0.00%
code_lens 297.9±8.19ns 296.6±8.13ns -0.44%
compile 2.7±0.05s 2.7±0.07s 0.00%
completion 4.8±0.12ms 4.8±0.33ms 0.00%
did_change_with_caching 2.7±0.06s 2.7±0.03s 0.00%
document_symbol 869.1±25.34µs 913.3±43.29µs +5.09%
format 72.1±1.04ms 73.1±0.78ms +1.39%
goto_definition 337.3±10.16µs 342.9±10.22µs +1.66%
highlight 9.1±0.14ms 8.8±0.24ms -3.30%
hover 494.6±7.04µs 501.2±10.52µs +1.33%
idents_at_position 120.2±0.56µs 120.0±0.51µs -0.17%
inlay_hints 638.3±25.43µs 629.6±27.24µs -1.36%
on_enter 2.1±0.07µs 2.1±0.06µs 0.00%
parent_decl_at_position 3.8±0.09ms 3.6±0.06ms -5.26%
prepare_rename 343.1±10.36µs 346.3±12.74µs +0.93%
rename 9.5±0.20ms 9.3±0.33ms -2.11%
semantic_tokens 1240.2±19.65µs 1271.6±70.59µs +2.53%
token_at_position 347.6±2.81µs 342.1±4.33µs -1.58%
tokens_at_position 3.8±0.08ms 3.6±0.06ms -5.26%
tokens_for_file 410.2±3.97µs 407.5±5.95µs -0.66%
traverse 38.2±0.77ms 38.9±0.98ms +1.83%

Copy link

github-actions bot commented Aug 6, 2024

Benchmark for 2f9b1ee

Click to view benchmark
Test Base PR %
code_action 5.1±0.13ms 5.2±0.08ms +1.96%
code_lens 343.4±10.45ns 331.7±11.80ns -3.41%
compile 2.6±0.04s 2.7±0.06s +3.85%
completion 4.5±0.01ms 4.7±0.02ms +4.44%
did_change_with_caching 2.6±0.04s 2.6±0.02s 0.00%
document_symbol 868.9±12.49µs 866.3±29.18µs -0.30%
format 72.5±2.18ms 72.6±2.64ms +0.14%
goto_definition 327.1±5.72µs 345.9±4.10µs +5.75%
highlight 8.6±0.01ms 8.9±0.02ms +3.49%
hover 492.1±11.63µs 499.3±7.67µs +1.46%
idents_at_position 119.8±0.39µs 120.0±1.77µs +0.17%
inlay_hints 614.7±27.37µs 646.1±41.97µs +5.11%
on_enter 2.1±0.04µs 2.0±0.06µs -4.76%
parent_decl_at_position 3.6±0.11ms 3.7±0.10ms +2.78%
prepare_rename 329.1±4.05µs 343.7±10.27µs +4.44%
rename 8.9±0.15ms 9.3±0.11ms +4.49%
semantic_tokens 1251.4±16.56µs 1229.5±56.83µs -1.75%
token_at_position 333.4±2.14µs 337.6±2.56µs +1.26%
tokens_at_position 3.6±0.25ms 3.7±0.02ms +2.78%
tokens_for_file 403.4±20.10µs 404.8±2.39µs +0.35%
traverse 37.1±0.82ms 38.2±0.49ms +2.96%

sdankel
sdankel previously approved these changes Aug 6, 2024
Copy link
Contributor

@IGI-111 IGI-111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test for this?

@xunilrj
Copy link
Contributor Author

xunilrj commented Aug 7, 2024

Can we add a test for this?

Done.

Copy link

github-actions bot commented Aug 7, 2024

Benchmark for 161d055

Click to view benchmark
Test Base PR %
code_action 5.2±0.13ms 5.1±0.32ms -1.92%
code_lens 286.7±8.00ns 288.1±7.34ns +0.49%
compile 2.6±0.05s 2.7±0.08s +3.85%
completion 4.7±0.02ms 4.5±0.01ms -4.26%
did_change_with_caching 2.6±0.03s 2.6±0.02s 0.00%
document_symbol 926.7±36.69µs 924.2±30.04µs -0.27%
format 72.4±0.94ms 73.3±1.29ms +1.24%
goto_definition 341.0±6.47µs 344.9±7.10µs +1.14%
highlight 8.9±0.13ms 8.7±0.02ms -2.25%
hover 499.0±5.12µs 495.8±9.06µs -0.64%
idents_at_position 117.3±0.52µs 118.5±0.52µs +1.02%
inlay_hints 638.3±15.67µs 636.2±25.20µs -0.33%
on_enter 2.0±0.03µs 2.1±0.23µs +5.00%
parent_decl_at_position 3.7±0.04ms 3.6±0.04ms -2.70%
prepare_rename 339.6±6.07µs 342.3±4.59µs +0.80%
rename 9.3±0.05ms 8.9±0.20ms -4.30%
semantic_tokens 1243.9±16.91µs 1269.0±15.32µs +2.02%
token_at_position 337.7±1.96µs 342.0±1.61µs +1.27%
tokens_at_position 3.7±0.03ms 3.6±0.01ms -2.70%
tokens_for_file 400.7±3.36µs 405.1±5.89µs +1.10%
traverse 37.9±1.47ms 37.0±0.89ms -2.37%

Copy link

github-actions bot commented Aug 7, 2024

Benchmark for 7941ec7

Click to view benchmark
Test Base PR %
code_action 5.2±0.07ms 5.0±0.02ms -3.85%
code_lens 283.8±5.26ns 285.9±14.94ns +0.74%
compile 2.7±0.03s 2.7±0.05s 0.00%
completion 4.7±0.10ms 4.6±0.15ms -2.13%
did_change_with_caching 2.7±0.04s 2.6±0.03s -3.70%
document_symbol 858.5±23.07µs 876.8±51.08µs +2.13%
format 72.9±1.18ms 73.0±1.21ms +0.14%
goto_definition 339.9±8.34µs 347.4±6.29µs +2.21%
highlight 9.0±0.12ms 8.7±0.26ms -3.33%
hover 493.5±6.67µs 500.5±6.92µs +1.42%
idents_at_position 118.8±0.49µs 118.9±0.64µs +0.08%
inlay_hints 631.0±22.93µs 627.3±17.96µs -0.59%
on_enter 2.1±0.06µs 2.0±0.04µs -4.76%
parent_decl_at_position 3.7±0.03ms 3.6±0.06ms -2.70%
prepare_rename 342.6±7.78µs 347.8±6.69µs +1.52%
rename 9.2±0.03ms 9.0±0.16ms -2.17%
semantic_tokens 1197.1±10.60µs 1243.3±16.31µs +3.86%
token_at_position 335.9±10.41µs 336.2±2.13µs +0.09%
tokens_at_position 3.7±0.02ms 3.6±0.05ms -2.70%
tokens_for_file 397.1±3.80µs 399.8±2.68µs +0.68%
traverse 37.4±1.12ms 37.7±1.03ms +0.80%

@xunilrj xunilrj requested review from IGI-111, sdankel and a team August 7, 2024 14:24
@IGI-111 IGI-111 requested a review from a team August 7, 2024 15:13
Copy link

github-actions bot commented Aug 7, 2024

Benchmark for c4d448f

Click to view benchmark
Test Base PR %
code_action 5.0±0.18ms 4.7±0.10ms -6.00%
code_lens 274.9±10.21ns 274.8±9.69ns -0.04%
compile 2.5±0.04s 2.4±0.05s -4.00%
completion 4.4±0.12ms 4.2±0.03ms -4.55%
did_change_with_caching 2.5±0.03s 2.4±0.03s -4.00%
document_symbol 875.4±32.60µs 798.0±8.40µs -8.84%
format 70.7±1.55ms 69.3±2.38ms -1.98%
goto_definition 327.0±8.51µs 327.2±8.98µs +0.06%
highlight 8.6±0.26ms 8.1±0.06ms -5.81%
hover 474.1±11.28µs 497.1±11.41µs +4.85%
idents_at_position 112.1±2.27µs 110.1±1.14µs -1.78%
inlay_hints 610.2±21.46µs 692.2±15.81µs +13.44%
on_enter 1950.9±50.41ns 2.0±0.07µs +2.52%
parent_decl_at_position 3.5±0.06ms 3.4±0.17ms -2.86%
prepare_rename 326.3±7.82µs 326.5±5.56µs +0.06%
rename 8.8±0.17ms 8.4±0.13ms -4.55%
semantic_tokens 1177.4±30.26µs 1164.7±29.13µs -1.08%
token_at_position 319.2±7.38µs 325.2±3.12µs +1.88%
tokens_at_position 3.6±0.10ms 3.3±0.05ms -8.33%
tokens_for_file 389.0±9.48µs 376.2±5.31µs -3.29%
traverse 35.4±0.91ms 34.9±0.66ms -1.41%

@ironcev ironcev merged commit d5aa2d6 into master Aug 7, 2024
39 checks passed
@ironcev ironcev deleted the xunilrj/fix-configurables-enum branch August 7, 2024 18:53
esdrubal pushed a commit that referenced this pull request Aug 13, 2024
## Description

This PR fixes an issue where the encoded bytes for enums were not big
enough for the biggest variant.

## Checklist

- [ ] 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).
- [ ] 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.
- [ ] 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants