-
-
Notifications
You must be signed in to change notification settings - Fork 403
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
Remove arguments_binding
field from CodeBlock
#2969
Conversation
cfa3695
to
6eea363
Compare
Test262 conformance changes
|
Codecov Report
@@ Coverage Diff @@
## main #2969 +/- ##
==========================================
+ Coverage 50.01% 50.03% +0.01%
==========================================
Files 446 446
Lines 45906 45925 +19
==========================================
+ Hits 22961 22979 +18
- Misses 22945 22946 +1
|
Main:
PR:
There is an improvement in performance :) |
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.
Nice work!
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!
What benchmarking technique are you using here? Could we document it so that we can at some point run it in the CI or manually? |
I used the benchmark in this comment #1924 (comment) it is a subset of the quickjs benchmarks (for the full benchmarks see #1924 (comment)), I didn't use the full one because one has a very long execution time To run the benchmarks with the boa cli with the file. (
I don't know if the results would be that useful, at least while running with the github job runners... |
This PR reorders a spec step to ensure that the
"arguments"
binding is always in the0
position. This allows us to remove thearguments_binding
field completely since we know in which environment it is (last pushed) and what index it has (0
).Removes
24
bytes fromCodeBlock
(from184
bytes to160
bytes)