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.

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

Ready to upgrade to V8 4.2? #1393

Closed
domenic opened this issue Apr 10, 2015 · 48 comments
Closed

Ready to upgrade to V8 4.2? #1393

domenic opened this issue Apr 10, 2015 · 48 comments
Labels
meta Issues and PRs related to the general management of the project. v8 engine Issues and PRs related to the V8 dependency.
Milestone

Comments

@domenic
Copy link
Contributor

domenic commented Apr 10, 2015

Counting down the weeks, Chrome 42 should be coming out any day now... That means it's time for V8 4.2!

#1026 was the last discussion. Looks like this will require a 2.x since everyone will need to recompile their addons?

Selflishly, I want this ASAP... Native classes will be huuuge.

@cjihrig
Copy link
Contributor

cjihrig commented Apr 10, 2015

+1. Are there any semver major PRs remaining that can go into the next branch?

Are there nightlies or anything for the next branch? If not, some sort of preview release would be nice.

@Fishrock123 Fishrock123 added the meta Issues and PRs related to the general management of the project. label Apr 10, 2015
@mscdex mscdex added the v8 engine Issues and PRs related to the V8 dependency. label Apr 11, 2015
@rvagg
Copy link
Member

rvagg commented Apr 11, 2015

nightlies for next were on my TODO list sorry, I'm obviously overstretched at the moment but I'll try and get to this ASAP cause this is pretty important (and fun)

@cjihrig
Copy link
Contributor

cjihrig commented Apr 11, 2015

@rvagg I'd be glad to offer help if you can use it.

@rvagg
Copy link
Member

rvagg commented Apr 11, 2015

@iojs/build / @Fishrock123 I've adjusted iojs+release+nightly on Jenkins to take an additional parameter: buildtype which can be either next or next-nightly, it's primarily used to put the nightlies into the right place and set their version name appropriately, it won't do any checks that you are giving it a commit on the next branch though so you still need to use your brain a little when making a nightly.

First nightly built using this has landed here: https://iojs.org/download/next-nightly/ and the rest will go in the same place.

$ cat foo.js
"use strict"

class Dude {
  constructor() {
    this.rnd = Math.random()
  }

  toString() {
    return 'Whoooooooa dude!'
  }
}

class Whoa extends Dude {
  constructor() {
    super()
    console.log(`
      Java me up scotty!
      ${ JSON.stringify(process.versions) }
      ${ JSON.stringify(this.derp('foo', 'bar')) }
      ${ this }
    `)
  }

  derp(...args) {
    return {
        [this.rnd]: 'eh?'
      , [args[0]]: args[1]
    }
  }
}

new Whoa()
$ node --harmony-computed-property-names --harmony-rest-parameters foo.js
      Java me up scotty!
      {"http_parser":"2.3.0","node":"2.0.0-next-nightly2015041138225a7203","v8":"4.2.77.13","uv":"1.4.2","zlib":"1.2.8","ares":"1.10.0-DEV","modules":"44","openssl":"1.0.1m"}
      {"0.6709401244297624":"eh?","foo":"bar"}
      Whoooooooa dude!

Wait, what language is this?

/cc @iojs/evangelism

@rvagg
Copy link
Member

rvagg commented Apr 11, 2015

@othiym23 does npm not like my extended prerelease here?

$ npm i xtend@latest
npm http request GET https://registry.npmjs.org/xtend
npm http 304 https://registry.npmjs.org/xtend
npm WARN engine [email protected]: wanted: {"node":">=0.4"} (current: {"node":"2.0.0-next-nightly2015041138225a7203","npm":"2.7.1"})
[email protected] node_modules/xtend

@yosuke-furukawa
Copy link
Member

yay! v8 4.2 shipped class syntax by default.

@YurySolovyov
Copy link

I thought M43 already branched

@Fishrock123
Copy link
Contributor

@rvagg I think npm sometimes warns for pre-releases.

derp(...args) {

oooo, 4.2 supports spread by default? :D

Edit: is that just --harmony-rest-parameters?

@bricss
Copy link

bricss commented Apr 11, 2015

+1

@leecade
Copy link

leecade commented Apr 12, 2015

Looking forward 👍

@shekhei
Copy link

shekhei commented Apr 12, 2015

+1 been using classes with the harmony flag for quite a while, code is much cleaner, and it still retains all the prototypal stuff internally, best of both worlds.

What else is delivered along with 4.2? anybody's got a link to the list?

@bnoordhuis
Copy link
Member

What else is delivered along with 4.2? anybody's got a link to the list?

Classes and object literals are no longer behind a flag.

Rest parameters, computed property names and Unicode escapes ('\u{xxxx}') are new and behind flags. There is a separate flag for Unicode escapes in regular expressions.

Preliminary support for strong mode (tracking bug).

@hrishikeshs
Copy link

👍 Really looking forward to this!! Can promises be subclassed with the new V8?

@rlidwka
Copy link
Contributor

rlidwka commented Apr 12, 2015

Can promises be subclassed with the new V8?

Promises can already be subclassed even in current V8.

@hrishikeshs
Copy link

@rlidwka Thanks! my bad, didn't know I could do that in the current V8.

@shekhei
Copy link

shekhei commented Apr 12, 2015

@bnoordhuis thanks mate!

@bricss
Copy link

bricss commented Apr 14, 2015

Time has come!

Chrome 42 finally released.

@rvagg
Copy link
Member

rvagg commented Apr 14, 2015

From our roadmap/policy on versioning:

  • When V8 ships a breaking change to their C++ API that can be handled by nan the minor version of io.js will be increased.
  • When V8 ships a breaking change to their C++ API that can NOT be handled by nan the major version of io.js will be increased.

and my understanding is that there are no breaking changes in this version of V8 so this there isn't really a justification in a 1.8 unless we're pulling in other breaking changes. But I'd like to hear opinions from @kkoopa and @bnoordhuis on this.

4.3 is looking more like the breaking-change nightmare that we're used to from V8 however.

@domenic
Copy link
Contributor Author

domenic commented Apr 14, 2015

We are hopefully going to finally revert the floating patch we have on top of V8 that is in place to avoid breaking changes.

@Fishrock123
Copy link
Contributor

@rvagg we'd have to major anyways since it's an ABI change...

(Which, for the record, is really silly.)

@rvagg
Copy link
Member

rvagg commented Apr 14, 2015

but that's the point of that clause in ROADMAP.md, NAN should be able to deal with this ABI change as far as I understand so it'd be a minor bump in NAN and a minor bump here

@domenic
Copy link
Contributor Author

domenic commented Apr 14, 2015

I thought NAN only dealt with API, not ABI.

@Fishrock123
Copy link
Contributor

NAN should be able to deal with this ABI change as far as I understand so it'd be a minor bump in NAN and a minor bump here

Argument made is that people should 100% never have to recompile addons outside of majors.

@rvagg
Copy link
Member

rvagg commented Apr 14, 2015

Argument made is that people should 100% never have to recompile addons outside of majors.

I seem to have missed that discussion, where is that specified? That sounds like a major every 6 weeks thanks to the V8 team's preference for shuffling the deckchairs with every release.

@Fishrock123
Copy link
Contributor

I seem to have missed that discussion, where is that specified?

it's not specified in any one place, it's just been the pervading argument in discussions.

That sounds like a major every 6 weeks thanks to the V8 team's preference for shuffling the deckchairs with every release.

Yeah.

@rvagg
Copy link
Member

rvagg commented Apr 14, 2015

Well, since this isn't in any of our official documentation then it's not going to drive versioning decisions as far as I'm concerned. So if anyone (@iojs/collaborators) feels strongly enough about the argument that "people should 100% never have to recompile addons outside of majors" then you'd better PR that to the Stability Policy section of ROADMAP.md otherwise this next release with V8 4.2 will be 1.8 rather than 2.0 (unless a semver-major gets in too).

@rvagg
Copy link
Member

rvagg commented Apr 14, 2015

also /cc @mikeal since he wrote most of the base Stability Policy section of ROADMAP.md.

@trevnorris
Copy link
Contributor

I realize that native modules require V8's API, but requiring complete ABI compatibility for all dependencies OR require major release seems excessive. As long as they can recompile without an issue I don't see the need for a major release.

@kkoopa
Copy link

kkoopa commented Apr 15, 2015

I have not tested v8 4.2 yet, but according to the changelog there should not be any issues. Only important change mentioned is that support for signatures with arguments was removed. I don't believe anyone was ever using this, apart from our tests, so its removal should be fairly uncontroversial. nodejs/nan#309 It's not worth bumping NAN major for this either. That might come around when v8 4.3 becomes actual, as everything will break then, which is a good opportunity to make breaking changes.

@indutny
Copy link
Member

indutny commented Apr 15, 2015

+1

@joliss
Copy link
Contributor

joliss commented Apr 17, 2015

Classes and object literals are no longer behind a flag.

Is classes support stable enough yet?

The following two snippets work with io.js 1.7.1 (with --harmony_classes), but fail with Babel - I'm not sure what the spec wants, but it might be worth investigating the discrepancy.

"use strict";

class A { }

class B extends A {
  constructor () {
    this; // Babel throws error: 'this' is not allowed before super()
  }
}
"use strict";

class A { }

class B extends A {
  constructor () {
    // Babel throws error: Derived constructor must call super()
  }
}

@jkrems
Copy link
Contributor

jkrems commented Apr 17, 2015

The ES6 spec lists that behavior under "runtime behavior". Not sure who is right here (not an expert in reading the spec). But when instantiating B the 2.0.0 nightly does throw:

> ./node -v && ./node -e '"use strict"; class A {} class B extends A { constructor() {} } new B()'
v2.0.0-next-nightly2015041138225a7203
[eval]:1
"use strict"; class A {} class B extends A { constructor() {} } new B()
                                                            ^
ReferenceError: this is not defined
    at Function.B ([eval]:1:61)
    at [eval]:1:65
    at Object.exports.runInThisContext (vm.js:54:17)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:410:26)
    at evalScript (node.js:497:25)
    at startup (node.js:84:9)
    at node.js:868:3

This seems to match the behavior described in the spec (e.g. https://people.mozilla.org/~jorendorff/es6-draft.html#sec-super-keyword-runtime-semantics-evaluation).

@joliss
Copy link
Contributor

joliss commented Apr 17, 2015

Ah, I didn't test with the nightly - you are right, V8's behavior seems fine then.

@domenic
Copy link
Contributor Author

domenic commented Apr 17, 2015

Yes, best not to test the previous release when wondering whether the upcoming release has the correct semantics :)

@mgol
Copy link
Contributor

mgol commented Apr 17, 2015

@trevnorris

I realize that native modules require V8's API, but requiring complete ABI compatibility for all dependencies OR require major release seems excessive. As long as they can recompile without an issue I don't see the need for a major release.

Yeah, if we're going to have a major release every 6 weeks, no one will seriously consider io.js for stable apps that should require minimal maintenance after finishing. No one wants to fix issues related to various breaking changes every 6 weeks...

@kkoopa

It's not worth bumping NAN major for this either. That might come around when v8 4.3 becomes actual, as everything will break then, which is a good opportunity to make breaking changes.

Is there sth I could read about what's going to change there so drastically?

@YurySolovyov
Copy link

@mzgol does chrome that releases every 6 weeks feels unstable to you?
Also, breaking ABI does not nessesairly mean apps breakage, npm rebuild is not so hard to do.
At last, if you can't update every 6 weeks, it is fine, just stay at release that works for you, thought you might miss security patches, but they may also break ABI, so choice is yours.

@mgol
Copy link
Contributor

mgol commented Apr 17, 2015

@YuriSolovyov Chrome doesn't follow semver, its versions aren't even in the semver format (it has 4 numbers). Also, major Chrome releases generally don't break JS APIs because ECMAScript is trying to be as backwards compatible as possible.

@mgol
Copy link
Contributor

mgol commented Apr 17, 2015

Well, if all that's needed is npm rebuild then it may not be worth bumping the major, saving it for changes that will require changes in the code.

@kkoopa
Copy link

kkoopa commented Apr 17, 2015

The v8 changelog usually lists what has changed. The major thing this time around will be introduction of the maybe monad and making every function return them (but nothing takes them as arguments, making it all rather useless) nodejs/nan#310

@jkrems
Copy link
Contributor

jkrems commented Apr 20, 2015

With dedicated build servers ABI breaking is kind-of-a-big-deal because the current assumption (coming from node 0.8 and 0.10) is that building on a different node version than running the app is fine. npm rebuild only works if the whole compiler chain is installed on the host - which isn't necessarily true on application hosts. If there's not major bump, it would be great to have some other visible sign for "you can just copy the artifact between hosts". Maybe a wiki page with a list of ABI compatible versions would be enough?

@trevnorris
Copy link
Contributor

@jkrems You can check NODE_MODULE_VERSION within your modules, though that might not be what you're looking for?

@jkrems
Copy link
Contributor

jkrems commented Apr 20, 2015

@trevnorris The setup we have is dedicated build servers creating artifacts that are then just copied to target app hosts. The build host have a compiler chain installed, the app hosts do not. Right now (with node 0.10) it doesn't matter that the patch version of the app host matches the build host. E.g. we can build on 0.10.33 and copy the artifact to a host running 0.10.36.

Arguably it would be better not to have global node versions on the build hosts and always building against an exact match and/or including node in the artifact (e.g. via a container). Just describing our current setup. Not sure how common this kind of flow is.

@trevnorris
Copy link
Contributor

You should never experience ABI brake in a patch version. The discussion here is whether to increment major or minor.

@jkrems
Copy link
Contributor

jkrems commented Apr 20, 2015

Sorry, maybe calling out node "0.8 vs. 0.10" was misleading in this regard - for me "0.8 vs. 0.10" was a major bump (according to semver rules). And node patch versions were "minor bumps" from time to time.

My concern here is less about "should ABI bump minor or major" and more about "having a reliable list of what io.js versions are ABI compatible". Given the frequency of minor bumps, assuming that minor bumps break ABI in general seems to be pretty close to assuming that every version breaks ABI. In terms of how helpful it is for reducing maintenance overhead.

@Fishrock123
Copy link
Contributor

Given the frequency of minor bumps, assuming that minor bumps break ABI in general seems to be pretty close to assuming that every version breaks ABI.

I suppose, but it'd basically only be for v8.

@jkrems
Copy link
Contributor

jkrems commented Apr 20, 2015

I suppose, but it'd basically only be for v8.

In terms of ABI compatibility I don't see how that makes a difference - either the ABI is compatible or it's not. It doesn't matter how many errors there are, it's all or nothing.

If the assumption is that ABI will break every few weeks and this kind of process (having fixed build server versions that might differ from app server versions) won't be supported, then that's fine. Just wanted to bring it up. :) I'm all for getting v8 upgrades out as quickly as possible.

@trevnorris
Copy link
Contributor

@jkrems good idea. That sounds like a documentation thing. Though probably easier if it lived on the wiki then in official docs?

@Fishrock123
Copy link
Contributor

4.2 has landed in master as per #1506

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests