Skip to content

Escape analyze function scopes on non-arrow functions#4266

Merged
HalidOdat merged 8 commits intomainfrom
optimization/omit-function-scopes
Jun 4, 2025
Merged

Escape analyze function scopes on non-arrow functions#4266
HalidOdat merged 8 commits intomainfrom
optimization/omit-function-scopes

Conversation

@HalidOdat
Copy link
Member

@HalidOdat HalidOdat commented Jun 3, 2025

This PR omits functions scopes that that are not needed for non-arrow functions. We can see a nice performance increase in the QuickJS benchmarks :).

@HalidOdat HalidOdat added A-Performance Performance related changes and issues A-Execution Issues or PRs related to code execution labels Jun 3, 2025
@HalidOdat HalidOdat changed the title Escape function scopes on regular functions Escape function scopes on non-arrow functions Jun 3, 2025
@github-actions
Copy link

github-actions bot commented Jun 3, 2025

Test262 conformance changes

Test result main count PR count difference
Total 50,254 50,254 0
Passed 47,093 47,093 0
Ignored 1,444 1,444 0
Failed 1,717 1,717 0
Panics 0 0 0
Conformance 93.71% 93.71% 0.00%

@HalidOdat HalidOdat force-pushed the optimization/omit-function-scopes branch from 70c6caf to 6545364 Compare June 4, 2025 02:01
@HalidOdat HalidOdat force-pushed the optimization/omit-function-scopes branch from 6545364 to 4b7c5cf Compare June 4, 2025 04:34
@HalidOdat HalidOdat force-pushed the optimization/omit-function-scopes branch from 4b7c5cf to eaf0b1d Compare June 4, 2025 04:39
@HalidOdat
Copy link
Member Author

This is ready for review/merge 🎉


Latest Benchmarks

main

PROGRESS Richards
RESULT Richards 107
PROGRESS DeltaBlue
RESULT DeltaBlue 101
PROGRESS Encrypt
PROGRESS Decrypt
RESULT Crypto 140
PROGRESS RayTrace
RESULT RayTrace 276
PROGRESS Earley
PROGRESS Boyer
RESULT EarleyBoyer 336
PROGRESS RegExp
RESULT RegExp 68.8
PROGRESS Splay
RESULT Splay 519
PROGRESS NavierStokes
RESULT NavierStokes 339
SCORE 190

PR

PROGRESS Richards
RESULT Richards 142
PROGRESS DeltaBlue
RESULT DeltaBlue 140
PROGRESS Encrypt
PROGRESS Decrypt
RESULT Crypto 137
PROGRESS RayTrace
RESULT RayTrace 323
PROGRESS Earley
PROGRESS Boyer
RESULT EarleyBoyer 373
PROGRESS RegExp
RESULT RegExp 71.5
PROGRESS Splay
RESULT Splay 576
PROGRESS NavierStokes
RESULT NavierStokes 337
SCORE 215

@HalidOdat HalidOdat marked this pull request as ready for review June 4, 2025 05:24
@HalidOdat HalidOdat requested a review from a team June 4, 2025 05:24
Copy link
Member

@jasonwilliams jasonwilliams left a comment

Choose a reason for hiding this comment

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

This is even better than my suggestion which was to not GC these function environments. Instead they don't need to be created at all because if the bindings don't escape they are just poppedToLocal anyway and don't pollute the environment above.

@HalidOdat HalidOdat changed the title Escape function scopes on non-arrow functions Escape analyze function scopes on non-arrow functions Jun 4, 2025
Copy link
Member

@raskad raskad left a comment

Choose a reason for hiding this comment

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

Really nice :)

@HalidOdat HalidOdat added this pull request to the merge queue Jun 4, 2025
Merged via the queue into main with commit 27973f6 Jun 4, 2025
14 checks passed
@HalidOdat HalidOdat deleted the optimization/omit-function-scopes branch June 4, 2025 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Execution Issues or PRs related to code execution A-Performance Performance related changes and issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Perf] - Consider not GCing function environments which don’t need it

3 participants