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

Refactor call frame access to avoid panic checks #3888

Merged
merged 1 commit into from
Jul 3, 2024
Merged

Conversation

raskad
Copy link
Member

@raskad raskad commented Jul 2, 2024

This PR slightly changes how we store and access call frames in the vm. In addition to the frames stack, there is now a frame field that always holds the current call frame. The main reason for this is, so that we can avoid panic checks in critical paths.

I also removed some duplicate calls to Vm::frame and Vm::fram_mut and grouped them where possible. This should not be that relevant anymore, since now it's just a field access.

Here two graphs to visualize the change. These are running in dev profile so functions are visible. Here the bigger impact seems to actually be the deref of the frames vec, but I'm not sure if the actual cost in release looks different.

Before:
image

After:
image

And here some benchmarks. I did not run them multiple times, the impact seems a bit high to me:

Before:

RESULT Richards 55.7
RESULT DeltaBlue 57.8
RESULT Crypto 67.7
RESULT RayTrace 187
RESULT EarleyBoyer 174
RESULT RegExp 60.4
RESULT Splay 192
RESULT NavierStokes 155
SCORE 103

After:

RESULT Richards 59.1
RESULT DeltaBlue 59.7
RESULT Crypto 72.5
RESULT RayTrace 197
RESULT EarleyBoyer 182
RESULT RegExp 60.0
RESULT Splay 189
RESULT NavierStokes 172
SCORE 108

@raskad raskad added performance Performance related changes and issues execution Issues or PRs related to code execution labels Jul 2, 2024
@raskad raskad added this to the v0.18.1 milestone Jul 2, 2024
Copy link

github-actions bot commented Jul 2, 2024

Test262 conformance changes

Test result main count PR count difference
Total 50,213 50,213 0
Passed 42,981 42,981 0
Ignored 1,411 1,411 0
Failed 5,821 5,821 0
Panics 0 0 0
Conformance 85.60% 85.60% 0.00%

@raskad raskad requested a review from a team July 2, 2024 23:24
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 optimization! :)

@jedel1043 jedel1043 added this pull request to the merge queue Jul 3, 2024
Merged via the queue into main with commit 961d7b4 Jul 3, 2024
13 checks passed
@jedel1043 jedel1043 deleted the refactor-vm-frame branch July 3, 2024 15:03
@raskad raskad modified the milestones: v0.18.1, v0.19.0 Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
execution Issues or PRs related to code execution performance Performance related changes and issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants