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

Reduce environment allocations #4002

Merged
merged 4 commits into from
Dec 4, 2024
Merged

Conversation

raskad
Copy link
Member

@raskad raskad commented Sep 22, 2024

This PR builds on #3988 and removes runtime environment allocations when possible.
All three commits can be reviewd independently. I just put them all together here, because they depend on each other anyways.

Running the benchmarks did not show a big reduction in execution time, but this should reduce memory usage since we can skip and reduce size of allocations.

  • Skip environment creation when all bindings in the scope are local
  • Skip environment creation when possible for arrow functions
  • Do not allocate space for local bindings in runtime environments

@raskad raskad added execution Issues or PRs related to code execution ast Issue surrounding the abstract syntax tree labels Sep 22, 2024
@raskad raskad added this to the next-release milestone Sep 22, 2024
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 48,494 48,494 0
Passed 43,609 43,609 0
Ignored 1,500 1,500 0
Failed 3,385 3,385 0
Panics 0 0 0
Conformance 89.93% 89.93% 0.00%

@raskad raskad requested a review from a team September 23, 2024 00:01
Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

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

Nice work! Looks great to me! :)

@HalidOdat HalidOdat requested a review from a team September 28, 2024 13:05
@jasonwilliams
Copy link
Member

LGTM
Decent bump in performance on the overall score.

Main:

PROGRESS Richards
RESULT Richards 116
PROGRESS DeltaBlue
RESULT DeltaBlue 104
PROGRESS Encrypt
PROGRESS Decrypt
RESULT Crypto 170
PROGRESS RayTrace
RESULT RayTrace 316
PROGRESS Earley
PROGRESS Boyer
RESULT EarleyBoyer 352
PROGRESS RegExp
RESULT RegExp 87.9
PROGRESS Splay
RESULT Splay 334
PROGRESS NavierStokes
RESULT NavierStokes 358
SCORE 198

PR:

PROGRESS Richards
RESULT Richards 119
PROGRESS DeltaBlue
RESULT DeltaBlue 108
PROGRESS Encrypt
PROGRESS Decrypt
RESULT Crypto 221
PROGRESS RayTrace
RESULT RayTrace 373
PROGRESS Earley
PROGRESS Boyer
RESULT EarleyBoyer 386
PROGRESS RegExp
RESULT RegExp 86.3
PROGRESS Splay
RESULT Splay 324
PROGRESS NavierStokes
RESULT NavierStokes 368
SCORE 213

DHAT:
Running Richards and DeltaBlue

Main:

dhat: Total:     3,813,150,097 bytes in 40,575,592 blocks
dhat: At t-gmax: 14,327,077 bytes in 69,262 blocks
dhat: At t-end:  9,426,645 bytes in 68,271 blocks

PR:

dhat: Total:     3,698,882,937 bytes in 36,460,318 blocks
dhat: At t-gmax: 14,284,501 bytes in 69,115 blocks
dhat: At t-end:  9,401,553 bytes in 67,046 blocks

We can see less allocations in DHAT.
Feel free to take a look at the files in the inspector:

dhat-heap.json
dhat-heap-main.json

core/ast/src/scope.rs Outdated Show resolved Hide resolved
@raskad raskad force-pushed the reduce-environment-allocations branch from 8df3d00 to 5827d98 Compare December 4, 2024 00:14
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks great!

@jasonwilliams jasonwilliams added this pull request to the merge queue Dec 4, 2024
Merged via the queue into main with commit e1da912 Dec 4, 2024
13 checks passed
@jasonwilliams jasonwilliams deleted the reduce-environment-allocations branch December 4, 2024 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ast Issue surrounding the abstract syntax tree execution Issues or PRs related to code execution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants