-
-
Notifications
You must be signed in to change notification settings - Fork 413
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
Store active runnable and active function in CallFrame
#3197
Conversation
Test262 conformance changes
|
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #3197 +/- ##
==========================================
+ Coverage 50.30% 50.31% +0.01%
==========================================
Files 436 436
Lines 42656 42608 -48
==========================================
- Hits 21456 21440 -16
+ Misses 21200 21168 -32
☔ View full report in Codecov by Sentry. |
I still have to take a look at the PR in a bit more detail, but I thought I'd bring it up just a small discussion point early, especially since it may have been obvious that it was coming 😆 Using |
In my opinion, I don't see any advantages in doing this, We already have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of this looks pretty good to me. Just one small thing I noticed 😄
4eb05ea
to
29812b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 😄
29812b9
to
1c804fc
Compare
Depends on #3185Part of refactoring calling in VM. In this PR active runnable and active function are moved in the
CallFrame
, this allows us to untangle some of the function calling.It may seem like we are using more memory per ordinary function call, because of CallFrame size increase, but it actuality it's the same, before instead of storing it in the call frame we would spill it on the native rust stack which was filling the stack, contributing to smaller recursion limits.
@nekevss As I see it we already have the execution context stack, that being the
CallFrame
s (just a different name), we just have some work moving some fields that are supposed to be in an execution context like[[ScriptOrModule]]
which this PR does, realm needs to be moved too.