GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||
GeezerVison is showing github.com — deps: update V8 to 5.9 by targos · Pull Request #13263 · nodejs/node Conversation
|
|
There is one failing V8 test: https://ci.nodejs.org/job/node-test-commit-v8-linux/720/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
In order to make things easiest, please don't land this until the 8.0.0 release is out. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I cannot reproduce the V8 test failure on Fedora 25. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Added 22b3ad952d4c0e96610ae6604febf5be5d372790 to fix the Windows issue. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Another V8 CI run: https://ci.nodejs.org/job/node-test-commit-v8-linux/723/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM on the basis that node CI is green.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Looks like the issue is similar to #6678, where OOM killer kicks in and kills the process. in Following the same approach as #6678, following patch should fix this flaky behavior diff --git a/deps/v8/test/mjsunit/mjsunit.status b/deps/v8/test/mjsunit/mjsunit.status index 66ecfcf..c8c3c4e 100644 --- a/deps/v8/test/mjsunit/mjsunit.status +++ b/deps/v8/test/mjsunit/mjsunit.status @@ -148,7 +148,7 @@ # Slow tests. 'copy-on-write-assert': [PASS, SLOW], 'es6/tail-call-megatest*': [PASS, SLOW, FAST_VARIANTS, ['tsan', SKIP]], - 'es6/typedarray-construct-offset-not-smi': [PASS, SLOW], + 'es6/typedarray-construct-offset-not-smi': [PASS, SLOW, NO_VARIANTS], 'harmony/regexp-property-script-extensions': [PASS, SLOW], 'numops-fuzz-part*': [PASS, ['mode == debug', SLOW]], 'readonly': [PASS, SLOW], |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Disabled by default, not disabled altogether. Functions that don't pass |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The VS 2013 support patch can be dropped and wasn't the plan to keep 5.9 ABI and API compatible with 5.8? In that case bumping NODE_MODULE_VERSION isn't necessary.
(I realize the pull request in its current shape breaks API/ABI but I'm wondering what the plan is.)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@bnoordhuis I think the plan is to have clean V8 versions on |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
OK, I'll add it back.
Do you mean we don't support VS 2013 anymore? Then the CI jobs must change.
What @addaleax said. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
The vs2013 buildbot is only used for the v4.x branch at this point, I think, and possibly addon tests in v6.x. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@bnoordhuis see nodejs/node-v8#4 (comment) It looks like addon tests are compiled with vs2013 for all branches. /cc @joaocgreis |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
We still support building native addons with VS2013. The CI machines with Windows 2008 all have VS2013 to make sure the addons test suite is run with VS2013. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
That test allocating 2GB of memory is a bit extreme, we'll look into it on the v8 side for the future: (v8 bug) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Okay, but that means the V8 patch can be dropped. It's many things but an add-on it is not. LGTM apart from that. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
But without the patch, win2008r2 tests are red: https://ci.nodejs.org/job/node-test-binary-windows/8831/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ah, I somehow had the idea that the patch only touched files in deps/v8/src. Okay, objection withdrawn. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Raised #13372 to discuss dropping support for VS 2013 in node 9. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
We should only land this if it has the ABI compat patches for 6.0
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Agreed. Landing this without the ABI compat patches would be a regression |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@MylesBorins Even on master? We didn’t do that for V8 5.8 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
[email protected]
BUG=v8:6376
Review-Url: https://codereview.chromium.org/2874453002
Cr-Commit-Position: refs/heads/master@{#45216}
PR-URL: #13263
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
es6/typedarray-construct-offset-not-smi allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test. Refs: #6678 PR-URL: #13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
This is required for ICU 59.1. PR-URL: #13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
VS2013 does not support defaulting move constructor and assignment operator. This adds explicit definitions of those methods for two classes. This fix is required because we still support building addons with VS2013 and the incompatibility is in v8.h. Fixes: nodejs/node-v8#4 PR-URL: #13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
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.
[email protected]
BUG=v8:6376
Review-Url: https://codereview.chromium.org/2874453002
Cr-Commit-Position: refs/heads/master@{#45216}
PR-URL: #13263
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
es6/typedarray-construct-offset-not-smi allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test. Refs: #6678 PR-URL: #13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
es6/typedarray-construct-offset-not-smi allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test. Refs: nodejs/node#6678 PR-URL: nodejs/node#13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
es6/typedarray-construct-offset-not-smi allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test. Refs: nodejs/node#6678 PR-URL: nodejs/node#13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
es6/typedarray-construct-offset-not-smi allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test. Refs: #6678 PR-URL: #13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
This is required for ICU 59.1. PR-URL: #13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
VS2013 does not support defaulting move constructor and assignment operator. This adds explicit definitions of those methods for two classes. This fix is required because we still support building addons with VS2013 and the incompatibility is in v8.h. Fixes: nodejs/node-v8#4 PR-URL: #13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
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.
[email protected]
BUG=v8:6376
Review-Url: https://codereview.chromium.org/2874453002
Cr-Commit-Position: refs/heads/master@{#45216}
PR-URL: #13263
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
es6/typedarray-construct-offset-not-smi allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test. Refs: nodejs/node#6678 PR-URL: nodejs/node#13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
es6/typedarray-construct-offset-not-smi allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test. Refs: nodejs/node#6678 PR-URL: nodejs/node#13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
es6/typedarray-construct-offset-not-smi allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test. Refs: #6678 PR-URL: #13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
VS2013 does not support defaulting move constructor and assignment operator. This adds explicit definitions of those methods for two classes. This fix is required because we still support building addons with VS2013 and the incompatibility is in v8.h. Fixes: nodejs/node-v8#4 PR-URL: #13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
es6/typedarray-construct-offset-not-smi allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test. Refs: nodejs/node#6678 PR-URL: nodejs/node#13263 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Myles Borins <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.