deps: fix Array.prototype.forEach on v8 6.8 #22899
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nodejs-github-bot
added
build
|
@nodejs/v8-update |
|
The title and description seems different than what we normally do. An example of such a commit is ab15016 Right now I am not sure what commit this will include from V8. |
|
The pull request should target the master branch or, if the slowdown does not affect V8 6.9, |
|
@ripsawridge is currently on vacation. |
|
We are about to merge V8 7.0 into |
addaleax
force-pushed
the
patchy
branch
2 times, most recently
from
2caf1c0 to
d885262
Compare
This applies a variant of v8/v8@e1163c14f7e4fef2c549 to V8 6.8. Original commit message: [Builtins] Array.prototype.forEach perf regression on dictionaries An unnecessary call to ToString() on the array index caused trips to the runtime. The fix also includes performance micro-benchmarks so we'll have a harder time regressing this case in future. TBR=tebbi@chromium.org Bug: v8:8112 Change-Id: I781e8b1bbe2eb56db961cf33b0dca8523868b83d Reviewed-on: https://chromium-review.googlesource.com/1213207 Commit-Queue: Michael Stanton <mvstanton@chromium.org> Reviewed-by: Michael Stanton <mvstanton@chromium.org> Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#55733} Refs: v8/v8@e1163c1 Fixes: nodejs#22859
|
I’ve rebased against v10.x-staging, and update the commit message to point to v8/v8@e1163c1 (this is not a clean cherry-pick, though). CI: https://ci.nodejs.org/job/node-test-pull-request/17368/ |
addaleax
added
author ready
addaleax
removed
the
author ready
addaleax
approved these changes
Sep 25, 2018
|
The embedder string can be fixed while landing |
|
Needs a quick rebase |
jasnell
approved these changes
Sep 25, 2018
targos
pushed a commit
that referenced
this pull request
Sep 25, 2018
This applies a variant of v8/v8@e1163c14f7e4fef2c549 to V8 6.8. Original commit message: [Builtins] Array.prototype.forEach perf regression on dictionaries An unnecessary call to ToString() on the array index caused trips to the runtime. The fix also includes performance micro-benchmarks so we'll have a harder time regressing this case in future. TBR=tebbi@chromium.org Bug: v8:8112 Change-Id: I781e8b1bbe2eb56db961cf33b0dca8523868b83d Reviewed-on: https://chromium-review.googlesource.com/1213207 Commit-Queue: Michael Stanton <mvstanton@chromium.org> Reviewed-by: Michael Stanton <mvstanton@chromium.org> Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/master@{#55733} Refs: v8/v8@e1163c1 Fixes: #22859 PR-URL: #22899 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 5d70652 |
This was referenced Oct 10, 2018
This was referenced Oct 11, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refs: #22859
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes