Skip to content
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.

Sign up for GitHub

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

Jump to bottom

v8.2.0 proposal #13744

Merged
merged 248 commits into from
Jul 19, 2017
Merged

v8.2.0 proposal #13744

merged 248 commits into from
Jul 19, 2017

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Jun 17, 2017 •
edited
Loading

The next Node 8 release will almost certainly be v8.1.3 (#13861) the July security release.

This is currently only blocked by waiting for the fix for https://bugs.chromium.org/p/v8/issues/detail?id=6490 to be merged upstream #14188.

2017-07-19, Version 8.2.0 (Current), @addaleax

Notable changes

  • Async Hooks

    • Multiple improvements to Promise support in async_hooks have been made.

  • Build

    • The compiler version requirement to build Node with GCC has been raised to
      GCC 4.9.4.
      [820b011ed6]
      #13466

  • Cluster

    • Users now have more fine-grained control over the inspector port used by
      individual cluster workers. Previously, cluster workers would simply
      increment from the master's debug port.
      [dfc46e262a]
      #14140

  • DNS

  • npm

Commits

@addaleax addaleax added the v8.x label Jun 17, 2017
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. v8 engine Issues and PRs related to the V8 dependency. v8.x labels Jun 17, 2017
@addaleax
Copy link
Member Author

@nodejs/release Do you think any of you could do an RC build next week? :)

@addaleax
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-commit/10625/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/879/

@mscdex
Copy link
Contributor

mscdex commented Jun 17, 2017

I just landed the nextTick() perf improvement PR if you want to slip that one in for this release.

@ronkorving
Copy link
Contributor

Does V8 being upgraded to 5.9 mean that Ignition and Turbofan are now on by default? If that's the case, what are the benchmark results, given that a lot of code has been optimized for Crankshaft? Is this release of Node.js going to be an upgrade or a downgrade in performance?

@Trott
Copy link
Member

Trott commented Jun 18, 2017

Does V8 being upgraded to 5.9 mean that Ignition and Turbofan are now on by default? If that's the case, what are the benchmark results, given that a lot of code has been optimized for Crankshaft? Is this release of Node.js going to be an upgrade or a downgrade in performance?

@ronkorving I think #11851 might answer your question.

@mgol
Copy link
Contributor

mgol commented Jun 19, 2017 •
edited
Loading

Has the V8 upgrade gone through the ABI-verification phase? @MylesBorins had some doubts in #13515 (comment).

@targos
Copy link
Member

targos commented Jun 19, 2017

It hasn't, and I think the ABI smoker needs to be updated.

@MylesBorins
Copy link
Contributor

MylesBorins commented Jun 19, 2017 via email •
edited by gibfahn
Loading

@gibfahn
Copy link
Member

gibfahn commented Jun 19, 2017 •
edited
Loading

Is changing GCC support not Semver Major?

No, see #13466 (comment). Node should work on all the machines it used to work on.

@rvagg
Copy link
Member

rvagg commented Jun 19, 2017

Strong -1 on this release with the 5.9 commits included from me.

I have serious concerns about the perf impact of 5.9 on our codebase and queries from myself (and others?) so far have gone unanswered. See #13631 for my last comments on the matter.

The most recent benchmarks we have is from an older 5.9 and are not encouraging for our .js against TurboFan, #11851.

Can we please back this out of v8.x and have a separate discussion about performance after someone runs more recent benchmarks. If you are keen for 5.9 to go ahead in 8.x then the onus is on you to prove that it's going to have minimal impact before we release it. Without answering these questions we put ourselves into a very tricky position prior to 8.x LTS, will we get to 6.0 (ABI stable) soon enough for LTS? Will 6.0 have the perf improvements we need?

Just because we deferred 8.0.0 to fit in 5.8 so we get close to the 6.0 ABI doesn't automatically mean we should automatically just roll forward.

@XadillaX
Copy link
Contributor

How about this PR? #13723

@mcollina
Copy link
Member

Can we include #13850?
It's kind of important!

@addaleax
Copy link
Member Author

@mcollina As long as it lands on master before the release happens, I see no reason not to (independently of whether the patch makes it into a possible RC or not).

@XadillaX I’ve included it and some other commits that have landed by now in this proposal.

@addaleax addaleax added the blocked PRs that are blocked by other issues or PRs. label Jun 21, 2017
addaleax added a commit that referenced this pull request Jun 21, 2017
@addaleax
2017-06-??, Version 8.2.0 (Current)
9dba18e 
Notable changes:

* **Async Hooks**
  * Multiple improvements to Promise support in `async_hooks` have been made.

* **Build**
  * The compiler version requirement to build Node with GCC has been raised to
    GCC 4.9.4.
    [[`23d41f3118`](2abaa86ba8)]
    [#13466](#13466)

* **DNS**
  * The server used for DNS queries can now use a custom port.
    [[`2bb6614904`](8506acc1b5)]
    [#13723](#13723)

* **V8**
  * The V8 engine has been upgraded to version 5.9, which has a significantly
    changed performance profile.
    [#13515](#13515)

PR-URL: #13744
@addaleax addaleax mentioned this pull request Jun 21, 2017
Merged
@addaleax
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-commit/10719/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/887/

(Note: This is rebased on top of #13861 now)

@rvagg
Copy link
Member

rvagg commented Jun 21, 2017

I've kicked off an 8.2.0-rc.1 build from this branch as per @addaleax' suggestion, will update with a link when it starts spitting them out. Hopefully someone will have the time to do some benchmarking on it to compare to 8.1.x.

See #13861 for next likely 8.x release.

@rvagg rvagg mentioned this pull request Jun 21, 2017
Closed
bnoordhuis and others added 13 commits July 19, 2017 15:02
@bnoordhuis @Fishrock123
src: make array arg length compile-time checkable
36e1351 
PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis @Fishrock123
src: avoid heap allocation in crypto.pbkdf2()
2d2d759 
PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis @Fishrock123
src: remove PBKDF2Request::release()
7d44590 
PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis @Fishrock123
src: simplify PBKDF2Request
5caaf83 
This also renames a misnamed variable `error_` to `success_`.

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis @Fishrock123
src: guard against double free in randomBytes()
62be8ef 
PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis @Fishrock123
src: reduce allocations in exportPublicKey()
83234db 
PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis @Fishrock123
src: fix memory leak in DH key setters
e3ed425 
Fix a memory leak in dh.setPublicKey() and dh.setPrivateKey() where the
old keys weren't freed.

Fixes: #8377
PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson @Fishrock123
doc: add guidance on testing new errors
48c6dbd 
PR-URL: #14207
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott @Fishrock123
lib: update indentation of ternaries
8010b21 
In preparation for stricter indentation linting and to increase code
clarity, update indentation for ternaries in lib.

PR-URL: #14247
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@shaman2009 @Fishrock123
test: check error with regex in test-signal-safety
e650fe0 
Change TypeError to a regular expression in assert.throws() in
test/parallel/test-signal-safety.js.

PR-URL: #14285
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@f48vj @Fishrock123
test: use regex error check in test-crypto-random
7936f91 
Replace constructor in assert.throws() to a regular expression to check
the error thrown in a test in test-crypto-random.

PR-URL: #14273
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@gergelyke @Fishrock123
test: add test for http outgoing internal headers
740ab3d 
PR-URL: #13980
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@addaleax @Fishrock123
doc: fix inspectPort documentation in cluster.md
64867a9 
- Add missing `changes:` entry
- Use full sentences
- Use proper indentation

Ref: #14140
PR-URL: #14349
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Fishrock123
Copy link
Contributor

Fishrock123 commented Jul 19, 2017 •
edited
Loading

Tests: https://ci.nodejs.org/job/node-test-pull-request/9256/
Builds: https://ci-release.nodejs.org/job/iojs+release/1859/

@Fishrock123
2017-07-19, Version 8.2.0 (Current)
eb66efd 
Big thanks to @addaleax who prepared the vast majority of this release.

Notable changes:

* **Async Hooks**
  * Multiple improvements to Promise support in `async_hooks` have been made.

* **Build**
  * The compiler version requirement to build Node with GCC has been raised to
    GCC 4.9.4.
    [[`820b011ed6`](820b011ed6)]
    [#13466](#13466)

* **Cluster**
  * Users now have more fine-grained control over the inspector port used by
    individual cluster workers. Previously, cluster workers would simply
    increment from the master's debug port.
    [[`dfc46e262a`](dfc46e262a)]
    [#14140](#14140)

* **DNS**
  * The server used for DNS queries can now use a custom port.
    [[`ebe7bb29aa`](ebe7bb29aa)]
    [#13723](#13723)
  * Support for `dns.resolveAny()` has been added.
    [[`6e30e2558e`](6e30e2558e)]
    [#13137](#13137)

* **npm**
  * The `npm` CLI has been updated to version 5.3.0. In particular, it now comes
    with the `npx` binary, which is also shipped with Node.
    [[`dc3f6b9ac1`](dc3f6b9ac1)]
    [#14235](#14235)
  * `npm` Changelogs:
      - [v5.0.4](https://github.com/npm/npm/releases/tag/v5.0.4)
      - [v5.1.0](https://github.com/npm/npm/releases/tag/v5.1.0)
      - [v5.2.0](https://github.com/npm/npm/releases/tag/v5.2.0)
      - [v5.3.0](https://github.com/npm/npm/releases/tag/v5.3.0)

PR-URL: #13744
@Fishrock123
Copy link
Contributor

Builds, again: https://ci-release.nodejs.org/job/iojs+release/1861/

@Fishrock123 Fishrock123 merged commit eb66efd into v8.x Jul 19, 2017
Fishrock123 added a commit that referenced this pull request Jul 19, 2017
@Fishrock123
Working on v8.2.1
7c3fed5 
PR-URL: #13744
Fishrock123 added a commit that referenced this pull request Jul 19, 2017
@Fishrock123
2017-07-19, Version 8.2.0 (Current)
65eefa0 
Big thanks to @addaleax who prepared the vast majority of this release.

Notable changes:

* **Async Hooks**
  * Multiple improvements to Promise support in `async_hooks` have been made.

* **Build**
  * The compiler version requirement to build Node with GCC has been raised to
    GCC 4.9.4.
    [[`820b011ed6`](820b011ed6)]
    [#13466](#13466)

* **Cluster**
  * Users now have more fine-grained control over the inspector port used by
    individual cluster workers. Previously, cluster workers would simply
    increment from the master's debug port.
    [[`dfc46e262a`](dfc46e262a)]
    [#14140](#14140)

* **DNS**
  * The server used for DNS queries can now use a custom port.
    [[`ebe7bb29aa`](ebe7bb29aa)]
    [#13723](#13723)
  * Support for `dns.resolveAny()` has been added.
    [[`6e30e2558e`](6e30e2558e)]
    [#13137](#13137)

* **npm**
  * The `npm` CLI has been updated to version 5.3.0. In particular, it now comes
    with the `npx` binary, which is also shipped with Node.
    [[`dc3f6b9ac1`](dc3f6b9ac1)]
    [#14235](#14235)
  * `npm` Changelogs:
      - [v5.0.4](https://github.com/npm/npm/releases/tag/v5.0.4)
      - [v5.1.0](https://github.com/npm/npm/releases/tag/v5.1.0)
      - [v5.2.0](https://github.com/npm/npm/releases/tag/v5.2.0)
      - [v5.3.0](https://github.com/npm/npm/releases/tag/v5.3.0)

PR-URL: #13744
Fishrock123 added a commit to nodejs/nodejs.org that referenced this pull request Jul 19, 2017
@Fishrock123
Blog: v8.2.0 release post
bb3542c 
Refs: nodejs/node#13744
@Fishrock123
Copy link
Contributor

Blog post at nodejs/nodejs.org@bb3542c

Fishrock123 added a commit to nodejs/nodejs.org that referenced this pull request Jul 19, 2017
@Fishrock123
Blog: edit 8.2.0 with releaser thanks note
f6c36a3 
Refs: nodejs/node#13744
@Fishrock123 Fishrock123 deleted the v8.2.0-proposal branch July 19, 2017 22:59
@kachkaev kachkaev mentioned this pull request Jul 20, 2017
Closed
Fishrock123 added a commit to nodejs/nodejs.org that referenced this pull request Jul 20, 2017
@Fishrock123
Blog: update 8.2.0 with armv6l link
a5c369e 
PR-URL: nodejs/node#13744
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fishrock123 Fishrock123 Fishrock123 approved these changes

@refack refack refack approved these changes

Assignees

@Fishrock123 Fishrock123

Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.