-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
deps: backport 6d38f89d from upstream V8 #13162
Conversation
Yes. This definitely needs to land for 8.x. |
@fhinkel Okay, It might be safe enough to bump the version number for V8 5.8 but we will run into the same issue with 5.9. Regardless, I will add a bump for 5.8 to this PR and we can deal with 5.9 when we get there. |
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 once the version number is bumped (hoping it won't conflict with last minute 5.8 changes).
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.
Rubber stamp LGTM
Original commit message: [turbofan] Boost performance of Array.prototype.shift by 4x. For small arrays, it's way faster to just move the elements instead of doing the fairly complex and heavy-weight left-trimming. Crankshaft has had this optimization for small arrays already; this CL more or less ports this functionality to TurboFan, which yields a 4x speed-up when using shift on small arrays (with up to 16 elements). This should recover some of the regressions reported in the Node.js issues nodejs#12657 and discovered for the syncthrough module using https://github.com/mcollina/syncthrough/blob/master/benchmarks/basic.js as benchmark. R=jarin@chromium.org BUG=v8:6376 Review-Url: https://codereview.chromium.org/2874453002 Cr-Commit-Position: refs/heads/master@{nodejs#45216} PR-URL: nodejs#13162 Reviewed-By: fhinkel - Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Landed in e51a201 |
Original commit message: [turbofan] Boost performance of Array.prototype.shift by 4x. For small arrays, it's way faster to just move the elements instead of doing the fairly complex and heavy-weight left-trimming. Crankshaft has had this optimization for small arrays already; this CL more or less ports this functionality to TurboFan, which yields a 4x speed-up when using shift on small arrays (with up to 16 elements). This should recover some of the regressions reported in the Node.js issues #12657 and discovered for the syncthrough module using https://github.com/mcollina/syncthrough/blob/master/benchmarks/basic.js as benchmark. R=jarin@chromium.org BUG=v8:6376 Review-Url: https://codereview.chromium.org/2874453002 Cr-Commit-Position: refs/heads/master@{#45216} PR-URL: #13162 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Original commit message: [turbofan] Boost performance of Array.prototype.shift by 4x. For small arrays, it's way faster to just move the elements instead of doing the fairly complex and heavy-weight left-trimming. Crankshaft has had this optimization for small arrays already; this CL more or less ports this functionality to TurboFan, which yields a 4x speed-up when using shift on small arrays (with up to 16 elements). This should recover some of the regressions reported in the Node.js issues #12657 and discovered for the syncthrough module using https://github.com/mcollina/syncthrough/blob/master/benchmarks/basic.js as benchmark. R=jarin@chromium.org BUG=v8:6376 Review-Url: https://codereview.chromium.org/2874453002 Cr-Commit-Position: refs/heads/master@{#45216} PR-URL: #13162 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Original commit message: [turbofan] Boost performance of Array.prototype.shift by 4x. For small arrays, it's way faster to just move the elements instead of doing the fairly complex and heavy-weight left-trimming. Crankshaft has had this optimization for small arrays already; this CL more or less ports this functionality to TurboFan, which yields a 4x speed-up when using shift on small arrays (with up to 16 elements). This should recover some of the regressions reported in the Node.js issues #12657 and discovered for the syncthrough module using https://github.com/mcollina/syncthrough/blob/master/benchmarks/basic.js as benchmark. R=jarin@chromium.org BUG=v8:6376 Review-Url: https://codereview.chromium.org/2874453002 Cr-Commit-Position: refs/heads/master@{#45216} PR-URL: #13162 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
@ofrobots safe to assume this is not to be landed on v6.x? |
Yep. This does not need to be back-ported to 6.x. |
This fixes some of the regressions in #12657. Since upstream doesn't backport things other than bug / security fixes, we will have to float this patch on Node.js 8.x until V8 6.0 lands on
master
.One problem here is that since 5.8 and 5.9 are still stable/beta, we cannot bump the V8 version number in our copy of V8. This might make reasoning about precise versions between us and upstream a bit tricky. (/cc @targos).
The fixup needed for the backport to V8 5.8 were fairly minimal.
Original commit message:
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
deps:v8
/cc @nodejs/v8 @bmeurer
EDIT: CI: https://ci.nodejs.org/job/node-test-pull-request/8242/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/710/