Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
deps: update V8 to 7.0#22754
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: update V8 to 7.0 #22754
Uh oh!
There was an error while loading. Please reload this page.
Conversation
targos commented Sep 7, 2018 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
nodejs-github-bot commented Sep 7, 2018
targos commented Sep 7, 2018 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
vsemozhetbyt commented Sep 7, 2018
Hello, stable Array.prototype.sort(). |
targos commented Sep 8, 2018
This version is also affected by nodejs/node-v8#77 /cc @refack who's been working on it. |
targos commented Sep 9, 2018
Pushed temporary fixes for nodejs/node-v8#77 and nodejs/node-v8#78. CI: https://ci.nodejs.org/job/node-test-pull-request/17087/ |
AyushG3112 commented Sep 9, 2018
Just confirming, this will release with nodejs 11 right? |
targos commented Sep 9, 2018
@AyushG3112 That's the plan |
targos commented Sep 9, 2018
@nodejs/platform-smartos There is a small error with |
targos commented Sep 9, 2018
There are missing postmortem metadata constants: |
targos commented Sep 9, 2018
@nodejs/platform-ppc something related to WASM seems broken: |
targos commented Sep 12, 2018
Ping @nodejs/platform-smartos @nodejs/platform-ppc |
mhdawson commented Sep 13, 2018
@john-yan Please look at the ppc issue and comment here. |
john-yan commented Sep 13, 2018
Hello Michael, I am aware of the errors and trying to fix asap |
john-yan commented Sep 13, 2018
Hello @targos , the ppc Atomic errors shown in https://ci.nodejs.org/job/node-test-commit-v8-linux/1675/#showFailuresLink are skipped by https://chromium-review.googlesource.com/c/v8/v8/+/1217003 as I64Atomic operations are not yet supported on the platform. |
761f10e to fe5cc29Comparetargos commented Sep 17, 2018
Thanks @john-yan, I just included the changes. CI: https://ci.nodejs.org/job/node-test-pull-request/17231/ |
targos commented Sep 17, 2018
@cjihrig maybe you can have a look at the postmortem issues? There is a small error with There are missing postmortem metadata constants: |
cjihrig commented Sep 17, 2018
I'll take a look. |
cjihrig commented Sep 17, 2018
@targos please cherry pick cjihrig@3ea5acc for the test fix and cjihrig@38a738d for SmartOS compilation. |
targos commented Sep 17, 2018
@cjihrig Done. Thanks a lot! |
targos commented Sep 17, 2018
Only nodejs/node-v8#78 remains. /cc @ofrobots |
Notable changes: * Build * FreeBSD 10 is no longer supported.[#22617](#22617) * `child_process` * The default value of the `windowsHide` option has been changed to `true`. [#21316](#21316) * `console` * `console.countReset()` will emit a warning if the timer being reset does not exist. [#21649](#21649) * `console.time()` will no longer reset a timer if it already exists. [#20442](#20442) * Dependencies * V8 has been updated to 7.0. [#22754](#22754) * `fs` * The `fs.read()` method now requires a callback. [#22146](#22146) * The previously deprecated `fs.SyncWriteStream` utility has been removed.[#20735](#20735) * `http` * The `http`, `https`, and `tls` modules now use the WHATWG URL parser by default. [#20270](#20270) * General * Use of `process.binding()` has been deprecated. Userland code using `process.binding()` should re-evaluate that use and begin migrating. If there are no supported API alternatives, please open an issue in the Node.js GitHub repository so that a suitable alternative may be discussed. * An experimental implementation of `queueMicrotask()` has been added. [#22951](#22951) * Internal * Windows performance-counter support has been removed. [#22485](#22485) * The `--expose-http2` command-line option has been removed. [#20887](#20887) * Timers * Interval timers will be rescheduled even if previous interval threw an error. [#20002](#20002) * `util` * The WHATWG `TextEncoder` and `TextDecoder` are now globals. [#22281](#22281) * `util.inspect()` output size is limited to 128 MB by default. [#22756](#22756) * A runtime warning will be emitted when `NODE_DEBUG` is set for either `http` or `http2`. [#21914](#21914)
Notable changes: * Build * FreeBSD 10 is no longer supported.[#22617](#22617) * `child_process` * The default value of the `windowsHide` option has been changed to `true`. [#21316](#21316) * `console` * `console.countReset()` will emit a warning if the timer being reset does not exist. [#21649](#21649) * `console.time()` will no longer reset a timer if it already exists. [#20442](#20442) * Dependencies * V8 has been updated to 7.0. [#22754](#22754) * `fs` * The `fs.read()` method now requires a callback. [#22146](#22146) * The previously deprecated `fs.SyncWriteStream` utility has been removed.[#20735](#20735) * `http` * The `http`, `https`, and `tls` modules now use the WHATWG URL parser by default. [#20270](#20270) * General * Use of `process.binding()` has been deprecated. Userland code using `process.binding()` should re-evaluate that use and begin migrating. If there are no supported API alternatives, please open an issue in the Node.js GitHub repository so that a suitable alternative may be discussed. * An experimental implementation of `queueMicrotask()` has been added. [#22951](#22951) * Internal * Windows performance-counter support has been removed. [#22485](#22485) * The `--expose-http2` command-line option has been removed. [#20887](#20887) * Timers * Interval timers will be rescheduled even if previous interval threw an error. [#20002](#20002) * `util` * The WHATWG `TextEncoder` and `TextDecoder` are now globals. [#22281](#22281) * `util.inspect()` output size is limited to 128 MB by default. [#22756](#22756) * A runtime warning will be emitted when `NODE_DEBUG` is set for either `http` or `http2`. [#21914](#21914)
Notable changes: * Build * FreeBSD 10 is no longer supported.[nodejs#22617](nodejs#22617) * `child_process` * The default value of the `windowsHide` option has been changed to `true`. [nodejs#21316](nodejs#21316) * `console` * `console.countReset()` will emit a warning if the timer being reset does not exist. [nodejs#21649](nodejs#21649) * `console.time()` will no longer reset a timer if it already exists. [nodejs#20442](nodejs#20442) * Dependencies * V8 has been updated to 7.0. [nodejs#22754](nodejs#22754) * `fs` * The `fs.read()` method now requires a callback. [nodejs#22146](nodejs#22146) * The previously deprecated `fs.SyncWriteStream` utility has been removed.[nodejs#20735](nodejs#20735) * `http` * The `http`, `https`, and `tls` modules now use the WHATWG URL parser by default. [nodejs#20270](nodejs#20270) * General * Use of `process.binding()` has been deprecated. Userland code using `process.binding()` should re-evaluate that use and begin migrating. If there are no supported API alternatives, please open an issue in the Node.js GitHub repository so that a suitable alternative may be discussed. * An experimental implementation of `queueMicrotask()` has been added. [nodejs#22951](nodejs#22951) * Internal * Windows performance-counter support has been removed. [nodejs#22485](nodejs#22485) * The `--expose-http2` command-line option has been removed. [nodejs#20887](nodejs#20887) * Timers * Interval timers will be rescheduled even if previous interval threw an error. [nodejs#20002](nodejs#20002) * `util` * The WHATWG `TextEncoder` and `TextDecoder` are now globals. [nodejs#22281](nodejs#22281) * `util.inspect()` output size is limited to 128 MB by default. [nodejs#22756](nodejs#22756) * A runtime warning will be emitted when `NODE_DEBUG` is set for either `http` or `http2`. [nodejs#21914](nodejs#21914)
refack commented Nov 11, 2018
Adding an extra |
targos commented Nov 11, 2018
It's Semver-major. There's no risk for it to land on v10.x |
Notable changes: * Build * FreeBSD 10 is no longer supported.[#22617](nodejs/node#22617) * `child_process` * The default value of the `windowsHide` option has been changed to `true`. [#21316](nodejs/node#21316) * `console` * `console.countReset()` will emit a warning if the timer being reset does not exist. [#21649](nodejs/node#21649) * `console.time()` will no longer reset a timer if it already exists. [#20442](nodejs/node#20442) * Dependencies * V8 has been updated to 7.0. [#22754](nodejs/node#22754) * `fs` * The `fs.read()` method now requires a callback. [#22146](nodejs/node#22146) * The previously deprecated `fs.SyncWriteStream` utility has been removed.[#20735](nodejs/node#20735) * `http` * The `http`, `https`, and `tls` modules now use the WHATWG URL parser by default. [#20270](nodejs/node#20270) * General * Use of `process.binding()` has been deprecated. Userland code using `process.binding()` should re-evaluate that use and begin migrating. If there are no supported API alternatives, please open an issue in the Node.js GitHub repository so that a suitable alternative may be discussed. * An experimental implementation of `queueMicrotask()` has been added. [#22951](nodejs/node#22951) * Internal * Windows performance-counter support has been removed. [#22485](nodejs/node#22485) * The `--expose-http2` command-line option has been removed. [#20887](nodejs/node#20887) * Timers * Interval timers will be rescheduled even if previous interval threw an error. [#20002](nodejs/node#20002) * `util` * The WHATWG `TextEncoder` and `TextDecoder` are now globals. [#22281](nodejs/node#22281) * `util.inspect()` output size is limited to 128 MB by default. [#22756](nodejs/node#22756) * A runtime warning will be emitted when `NODE_DEBUG` is set for either `http` or `http2`. [#21914](nodejs/node#21914)
Notable changes: * Build * FreeBSD 10 is no longer supported.[#22617](nodejs/node#22617) * `child_process` * The default value of the `windowsHide` option has been changed to `true`. [#21316](nodejs/node#21316) * `console` * `console.countReset()` will emit a warning if the timer being reset does not exist. [#21649](nodejs/node#21649) * `console.time()` will no longer reset a timer if it already exists. [#20442](nodejs/node#20442) * Dependencies * V8 has been updated to 7.0. [#22754](nodejs/node#22754) * `fs` * The `fs.read()` method now requires a callback. [#22146](nodejs/node#22146) * The previously deprecated `fs.SyncWriteStream` utility has been removed.[#20735](nodejs/node#20735) * `http` * The `http`, `https`, and `tls` modules now use the WHATWG URL parser by default. [#20270](nodejs/node#20270) * General * Use of `process.binding()` has been deprecated. Userland code using `process.binding()` should re-evaluate that use and begin migrating. If there are no supported API alternatives, please open an issue in the Node.js GitHub repository so that a suitable alternative may be discussed. * An experimental implementation of `queueMicrotask()` has been added. [#22951](nodejs/node#22951) * Internal * Windows performance-counter support has been removed. [#22485](nodejs/node#22485) * The `--expose-http2` command-line option has been removed. [#20887](nodejs/node#20887) * Timers * Interval timers will be rescheduled even if previous interval threw an error. [#20002](nodejs/node#20002) * `util` * The WHATWG `TextEncoder` and `TextDecoder` are now globals. [#22281](nodejs/node#22281) * `util.inspect()` output size is limited to 128 MB by default. [#22756](nodejs/node#22756) * A runtime warning will be emitted when `NODE_DEBUG` is set for either `http` or `http2`. [#21914](nodejs/node#21914)
Due to an update in the v8 runtime, Node.js `Array.prototype.sort()` is now stable (See [here](nodejs/node#22754 (comment))). These changes allow for tests to pass on both Node.js 10 and 11. Fixes#3445
For the signature `Array.prototype.sort((a, b) => rt)` `rt` encodes the sorting positions as follows [1]: - `rt < 0`: a should go first - `rt = 0`: same position - `rt > 0`: b should go first Node version 11 bundles an updated v8 engine [2]. One of the changes is a different sorting algorithm for Array.prototype.sort. Quicksort was replaced by Timsort[3], a stable sorting algorithm. Node v12 (new LTS) has the same updated behaviour. For an arbitrary sorted array every comparision would yield 0 or 1. Our comparision function using `rt = a > b`, is not sufficient anymore, as it would yield the signature of a sorted array and no changes are made to the sequence accordingly. The fix is simple: adjust the return value of 0 to -1. There should be only one entity per unique path, so we can omit the rt=0 case. Here is a minimal demo of the unit test test/unit/src/Project/ProjectControllerTests.js "ProjectController" -> "projectEntitiesJson" -> "should produce a list of entities" For future reference I kept the nvm output referencing the used node version (Running node ...) in place. Using the current sorting function ``` $ nvm run v10 -e 'console.log( [{path: "/things/b.txt", type: "doc" },{path: "/main.tex", type: "doc" },{path: "/things/a.txt", type: "file" } ].sort((a, b) =>{const rt = a.path > b.path console.log("comparing", a.path, b.path, rt) return rt}))' Running node v10.17.0 (npm v6.12.1) comparing /things/b.txt /main.tex true comparing /things/b.txt /things/a.txt true comparing /main.tex /things/a.txt false [{path: '/main.tex', type: 'doc' },{path: '/things/a.txt', type: 'file' },{path: '/things/b.txt', type: 'doc' } ] $ nvm run v11 -e 'console.log( [{path: "/things/b.txt", type: "doc" },{path: "/main.tex", type: "doc" },{path: "/things/a.txt", type: "file" } ].sort((a, b) =>{const rt = a.path > b.path console.log("comparing", a.path, b.path, rt) return rt}))' Running node v11.15.0 (npm v6.12.1) comparing /main.tex /things/b.txt false comparing /things/a.txt /main.tex true [{path: '/things/b.txt', type: 'doc' },{path: '/main.tex', type: 'doc' },{path: '/things/a.txt', type: 'file' } ] ``` Using the adjusted return value ``` $ nvm run v10 -e 'console.log( [{path: "/things/b.txt", type: "doc" },{path: "/main.tex", type: "doc" },{path: "/things/a.txt", type: "file" } ].sort((a, b) =>{const rt = a.path > b.path ? 1 : -1 console.log("comparing", a.path, b.path, rt) return rt}))' Running node v10.17.0 (npm v6.12.1) comparing /things/b.txt /main.tex 1 comparing /things/b.txt /things/a.txt 1 comparing /main.tex /things/a.txt -1 [{path: '/main.tex', type: 'doc' },{path: '/things/a.txt', type: 'file' },{path: '/things/b.txt', type: 'doc' } ] $ nvm run v11 -e 'console.log( [{path: "/things/b.txt", type: "doc" },{path: "/main.tex", type: "doc" },{path: "/things/a.txt", type: "file" } ].sort((a, b) =>{const rt = a.path > b.path ? 1 : -1 console.log("comparing", a.path, b.path, rt) return rt}))' Running node v11.15.0 (npm v6.12.1) comparing /main.tex /things/b.txt -1 comparing /things/a.txt /main.tex 1 comparing /things/a.txt /things/b.txt -1 comparing /things/a.txt /main.tex 1 [{path: '/main.tex', type: 'doc' },{path: '/things/a.txt', type: 'file' },{path: '/things/b.txt', type: 'doc' } ] ``` --- [1] https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description [2] nodejs/node#22754 [3] https://chromium-review.googlesource.com/c/v8/v8/+/1186801 Signed-off-by: Jakob Ackermann <[email protected]>For the signature `Array.prototype.sort((a, b) => rt)` `rt` encodes the sorting positions as follows [1]: - `rt < 0`: a should go first - `rt = 0`: same position - `rt > 0`: b should go first Node version 11 bundles an updated v8 engine [2]. One of the changes is a different sorting algorithm for Array.prototype.sort. Quicksort was replaced by Timsort[3], a stable sorting algorithm. Node v12 (new LTS) has the same updated behaviour. For an arbitrary sorted array every comparision would yield 0 or 1. Our comparision function using `rt = a > b`, is not sufficient anymore, as it would yield the signature of a sorted array and no changes are made to the sequence accordingly. The fix is simple: adjust the return value of 0 to -1. There should be only one entity per unique path, so we can omit the rt=0 case. Here is a minimal demo of the unit test test/unit/src/Project/ProjectControllerTests.js "ProjectController" -> "projectEntitiesJson" -> "should produce a list of entities" For future reference I kept the nvm output referencing the used node version (Running node ...) in place. Using the current sorting function ``` $ nvm run v10 -e 'console.log( [{path: "/things/b.txt", type: "doc" },{path: "/main.tex", type: "doc" },{path: "/things/a.txt", type: "file" } ].sort((a, b) =>{const rt = a.path > b.path console.log("comparing", a.path, b.path, rt) return rt}))' Running node v10.17.0 (npm v6.12.1) comparing /things/b.txt /main.tex true comparing /things/b.txt /things/a.txt true comparing /main.tex /things/a.txt false [{path: '/main.tex', type: 'doc' },{path: '/things/a.txt', type: 'file' },{path: '/things/b.txt', type: 'doc' } ] $ nvm run v11 -e 'console.log( [{path: "/things/b.txt", type: "doc" },{path: "/main.tex", type: "doc" },{path: "/things/a.txt", type: "file" } ].sort((a, b) =>{const rt = a.path > b.path console.log("comparing", a.path, b.path, rt) return rt}))' Running node v11.15.0 (npm v6.12.1) comparing /main.tex /things/b.txt false comparing /things/a.txt /main.tex true [{path: '/things/b.txt', type: 'doc' },{path: '/main.tex', type: 'doc' },{path: '/things/a.txt', type: 'file' } ] ``` Using the adjusted return value ``` $ nvm run v10 -e 'console.log( [{path: "/things/b.txt", type: "doc" },{path: "/main.tex", type: "doc" },{path: "/things/a.txt", type: "file" } ].sort((a, b) =>{const rt = a.path > b.path ? 1 : -1 console.log("comparing", a.path, b.path, rt) return rt}))' Running node v10.17.0 (npm v6.12.1) comparing /things/b.txt /main.tex 1 comparing /things/b.txt /things/a.txt 1 comparing /main.tex /things/a.txt -1 [{path: '/main.tex', type: 'doc' },{path: '/things/a.txt', type: 'file' },{path: '/things/b.txt', type: 'doc' } ] $ nvm run v11 -e 'console.log( [{path: "/things/b.txt", type: "doc" },{path: "/main.tex", type: "doc" },{path: "/things/a.txt", type: "file" } ].sort((a, b) =>{const rt = a.path > b.path ? 1 : -1 console.log("comparing", a.path, b.path, rt) return rt}))' Running node v11.15.0 (npm v6.12.1) comparing /main.tex /things/b.txt -1 comparing /things/a.txt /main.tex 1 comparing /things/a.txt /things/b.txt -1 comparing /things/a.txt /main.tex 1 [{path: '/main.tex', type: 'doc' },{path: '/things/a.txt', type: 'file' },{path: '/things/b.txt', type: 'doc' } ] ``` --- [1] https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description [2] nodejs/node#22754 [3] https://chromium-review.googlesource.com/c/v8/v8/+/1186801 Signed-off-by: Jakob Ackermann <[email protected]>Align with stable `Array.prototype.sort` in Node.js 12.x References: - https://v8.dev/features/stable-sort - nodejs/node#22754 (comment)
Align with stable `Array.prototype.sort` in Node.js 12.x References: - https://v8.dev/features/stable-sort - nodejs/node#22754 (comment)
Align with stable `Array.prototype.sort` in Node.js 12.x References: - https://v8.dev/features/stable-sort - nodejs/node#22754 (comment)
ETA: Oct 16th, 2018
Issues to fix:
failing test-trace-events-dynamic-enable node-v8#78https://chromium-review.googlesource.com/c/v8/v8/+/1235927v8ustack.d(SmartOS)