-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
buffer: removed unneeded FastBuffer constructor #19684
Conversation
For context, this was originally added because the default constructor for subclasses forwards arguments using array spread, which required a |
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.
LGTM if the benchmarks are happy
Re-ran benchmarks since the benchmark machine went down during the first run: |
Hrmmm.... there is this result which is interesting:
|
@nodejs/v8, could anyone take a look at this, specifically the performance regression in #19684 (comment)? |
I ran the benchmarks again and this time the result is different again. It seems like the former result is just a statistical slip-up. The same with the results that show up as changed in this benchmark run. |
Seems like the benchmark is flaky enough for it not to matter then. I'll go ahead and land this soon. |
Landed in d1af1e4. |
PR-URL: #19684 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #19684 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes