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

Rename module terminology to program #6056

Merged
merged 4 commits into from
May 27, 2024
Merged

Conversation

JoshuaBatty
Copy link
Member

@JoshuaBatty JoshuaBatty commented May 24, 2024

Description

Currently we have the ability to cache programs and also clear these programs from the engines using a garbage collector. The current terminology refers to modules rather than programs. I want to implement more granular caching and GC at the module level rather than just at the program level. This helps clear up what is actually being cached and cleared from the GC.

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.

@JoshuaBatty JoshuaBatty self-assigned this May 24, 2024
@JoshuaBatty JoshuaBatty requested review from a team May 24, 2024 01:04
@JoshuaBatty JoshuaBatty marked this pull request as draft May 24, 2024 01:14
@JoshuaBatty JoshuaBatty changed the title Rename module terminology to package Rename module terminology to program May 24, 2024
@JoshuaBatty JoshuaBatty marked this pull request as ready for review May 24, 2024 01:22
Copy link

Benchmark for 7d312a8

Click to view benchmark
Test Base PR %
code_action 5.4±0.13ms 5.3±0.06ms -1.85%
code_lens 335.9±4.55ns 287.6±8.10ns -14.38%
compile 3.1±0.05s 3.1±0.04s 0.00%
completion 4.6±0.11ms 4.5±0.11ms -2.17%
did_change_with_caching 2.9±0.03s 2.9±0.07s 0.00%
document_symbol 1043.4±61.80µs 1031.0±30.75µs -1.19%
format 74.6±1.53ms 73.0±1.25ms -2.14%
goto_definition 361.3±7.88µs 363.3±5.67µs +0.55%
highlight 8.8±0.22ms 8.8±0.54ms 0.00%
hover 484.9±10.05µs 490.1±9.33µs +1.07%
idents_at_position 124.0±1.47µs 122.7±0.94µs -1.05%
inlay_hints 645.0±35.59µs 652.2±18.43µs +1.12%
on_enter 459.2±6.01ns 455.3±18.08ns -0.85%
parent_decl_at_position 3.6±0.07ms 3.6±0.06ms 0.00%
prepare_rename 359.3±6.18µs 362.4±6.70µs +0.86%
rename 9.5±0.22ms 9.3±0.19ms -2.11%
semantic_tokens 987.1±18.77µs 970.5±26.29µs -1.68%
token_at_position 355.4±4.52µs 355.1±6.15µs -0.08%
tokens_at_position 3.6±0.03ms 3.6±0.16ms 0.00%
tokens_for_file 422.8±3.62µs 420.1±1.56µs -0.64%
traverse 42.6±0.86ms 41.6±1.61ms -2.35%

@JoshuaBatty JoshuaBatty marked this pull request as draft May 24, 2024 01:47
Copy link

Benchmark for e133ea9

Click to view benchmark
Test Base PR %
code_action 5.3±0.07ms 5.2±0.09ms -1.89%
code_lens 335.2±11.13ns 288.1±9.17ns -14.05%
compile 3.0±0.03s 3.0±0.05s 0.00%
completion 4.7±0.12ms 4.6±0.09ms -2.13%
did_change_with_caching 2.9±0.04s 2.9±0.04s 0.00%
document_symbol 990.8±28.78µs 953.8±35.22µs -3.73%
format 74.0±1.31ms 74.3±1.04ms +0.41%
goto_definition 414.0±4.94µs 360.0±5.55µs -13.04%
highlight 8.8±0.11ms 8.8±0.18ms 0.00%
hover 530.8±6.09µs 486.9±7.77µs -8.27%
idents_at_position 123.8±0.24µs 123.1±1.11µs -0.57%
inlay_hints 689.7±22.18µs 650.1±20.07µs -5.74%
on_enter 461.4±13.18ns 461.2±13.30ns -0.04%
parent_decl_at_position 3.6±0.02ms 3.6±0.04ms 0.00%
prepare_rename 405.9±11.49µs 360.0±3.73µs -11.31%
rename 9.3±0.12ms 9.3±0.16ms 0.00%
semantic_tokens 973.0±20.29µs 967.3±23.68µs -0.59%
token_at_position 401.2±2.51µs 355.9±5.71µs -11.29%
tokens_at_position 3.6±0.03ms 3.6±0.03ms 0.00%
tokens_for_file 419.8±1.94µs 416.9±1.92µs -0.69%
traverse 41.6±1.58ms 40.9±0.90ms -1.68%

Copy link

Benchmark for 8644c33

Click to view benchmark
Test Base PR %
code_action 5.3±0.10ms 5.6±0.28ms +5.66%
code_lens 336.1±9.16ns 287.9±8.00ns -14.34%
compile 3.1±0.08s 3.1±0.03s 0.00%
completion 4.6±0.06ms 4.5±0.07ms -2.17%
did_change_with_caching 3.0±0.11s 3.0±0.05s 0.00%
document_symbol 953.1±19.81µs 959.3±22.85µs +0.65%
format 73.8±0.58ms 73.8±1.05ms 0.00%
goto_definition 359.9±4.68µs 364.6±4.49µs +1.31%
highlight 8.9±0.02ms 8.8±0.10ms -1.12%
hover 487.4±6.21µs 488.2±5.52µs +0.16%
idents_at_position 123.3±0.41µs 123.8±0.39µs +0.41%
inlay_hints 658.1±35.65µs 658.2±13.16µs +0.02%
on_enter 466.4±11.80ns 454.6±13.13ns -2.53%
parent_decl_at_position 3.6±0.03ms 3.6±0.02ms 0.00%
prepare_rename 361.9±2.97µs 364.4±7.16µs +0.69%
rename 9.5±0.11ms 9.4±0.23ms -1.05%
semantic_tokens 951.9±14.21µs 963.9±16.79µs +1.26%
token_at_position 357.6±3.77µs 355.6±3.07µs -0.56%
tokens_at_position 3.6±0.05ms 3.6±0.03ms 0.00%
tokens_for_file 417.8±2.24µs 426.2±7.02µs +2.01%
traverse 41.5±1.20ms 42.7±1.38ms +2.89%

@JoshuaBatty JoshuaBatty marked this pull request as ready for review May 27, 2024 06:54
Copy link

Benchmark for ff7b626

Click to view benchmark
Test Base PR %
code_action 5.2±0.13ms 5.3±0.21ms +1.92%
code_lens 336.2±6.03ns 288.6±7.46ns -14.16%
compile 3.1±0.05s 3.1±0.07s 0.00%
completion 4.5±0.01ms 4.7±0.19ms +4.44%
did_change_with_caching 2.9±0.04s 3.0±0.04s +3.45%
document_symbol 1088.9±128.71µs 1023.5±38.82µs -6.01%
format 75.0±1.56ms 74.1±1.34ms -1.20%
goto_definition 364.8±5.56µs 369.0±7.05µs +1.15%
highlight 8.7±0.17ms 8.8±0.06ms +1.15%
hover 490.4±4.15µs 491.6±6.33µs +0.24%
idents_at_position 122.0±0.62µs 125.0±0.82µs +2.46%
inlay_hints 651.8±28.01µs 659.8±13.45µs +1.23%
on_enter 459.1±10.14ns 461.6±15.51ns +0.54%
parent_decl_at_position 3.6±0.03ms 3.6±0.28ms 0.00%
prepare_rename 368.3±5.24µs 368.5±7.04µs +0.05%
rename 9.3±0.17ms 9.4±0.54ms +1.08%
semantic_tokens 962.1±23.85µs 954.7±11.83µs -0.77%
token_at_position 358.2±3.80µs 363.9±7.43µs +1.59%
tokens_at_position 3.6±0.04ms 3.6±0.02ms 0.00%
tokens_for_file 427.5±3.88µs 419.2±3.30µs -1.94%
traverse 42.3±0.74ms 42.6±1.29ms +0.71%

Copy link
Member

@sdankel sdankel left a comment

Choose a reason for hiding this comment

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

Does program mean package, as in 1 program per Forc manifest file? And there can be multiple modules (sway files) per program?

@IGI-111 IGI-111 enabled auto-merge (squash) May 27, 2024 13:39
@IGI-111 IGI-111 merged commit 4fe6f1e into master May 27, 2024
36 checks passed
@IGI-111 IGI-111 deleted the josh/rename_terminology branch May 27, 2024 13:53
@tritao
Copy link
Contributor

tritao commented May 27, 2024

Does program mean package, as in 1 program per Forc manifest file? And there can be multiple modules (sway files) per program?

This looks good, but I was wondering the same here. My initial thought was that we would be using Package level caching for this, but Program also seems ok. Still think it would be helpful to address this somehow, as it can get confusing, maybe by documenting what each layer of caching means.

Copy link

Benchmark for 4804b0c

Click to view benchmark
Test Base PR %
code_action 6.0±0.26ms 5.4±0.07ms -10.00%
code_lens 286.1±4.99ns 294.6±9.63ns +2.97%
compile 3.1±0.04s 3.1±0.04s 0.00%
completion 5.3±0.31ms 4.8±0.07ms -9.43%
did_change_with_caching 3.0±0.05s 3.0±0.04s 0.00%
document_symbol 1027.0±38.72µs 985.8±24.10µs -4.01%
format 75.6±1.00ms 74.7±1.17ms -1.19%
goto_definition 368.6±5.92µs 375.5±9.27µs +1.87%
highlight 9.2±0.16ms 9.2±0.29ms 0.00%
hover 496.8±11.71µs 493.4±8.84µs -0.68%
idents_at_position 124.9±0.53µs 124.8±0.71µs -0.08%
inlay_hints 678.0±21.77µs 674.0±25.82µs -0.59%
on_enter 473.4±12.70ns 461.4±15.39ns -2.53%
parent_decl_at_position 4.0±0.15ms 3.8±0.05ms -5.00%
prepare_rename 370.5±7.72µs 371.1±5.81µs +0.16%
rename 10.2±0.14ms 9.8±0.17ms -3.92%
semantic_tokens 995.5±19.11µs 999.9±18.19µs +0.44%
token_at_position 357.8±4.37µs 365.9±2.77µs +2.26%
tokens_at_position 3.8±0.13ms 3.8±0.08ms 0.00%
tokens_for_file 433.1±3.18µs 421.7±3.75µs -2.63%
traverse 41.2±1.23ms 41.3±1.04ms +0.24%

@JoshuaBatty
Copy link
Member Author

@tritao and @sdankel yeah you're both correct, There is a second terminology refactor that needs to take place to sync the compiler and forc terminology here. I think package over program is the correct choice here personally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants