Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Turbofan + Ignition release plan && comms plan #155

Closed
MylesBorins opened this issue Jul 17, 2017 · 35 comments
Closed

Turbofan + Ignition release plan && comms plan #155

MylesBorins opened this issue Jul 17, 2017 · 35 comments

Comments

@MylesBorins
Copy link

Hey All,

So now that 5.9 is stabilized and 6.0 is soon to be released we need to make a decision about which we would like to release. The options include:

  1. Releasing with 5.9 in a semver-minor ASAP 🎉
  2. Holding off until 6.0 ❤️

Once we decide which we are going to move forward with we will also need to co-ordinate with @ZibbyKeaton and the rest of the foundation comms team to make sure that we are properly getting the word out to our community.

This could include:

  • A blog post
  • Social Network Coverage
  • External media coverage

We should make sure that which ever path we choose above we leave enough time for the comms team to plan a release.

If people from @nodejs/ctc could chime in on preference for the version with the emoji above that would be greatly appreciated. If you have a potential path forward that was not mentioned please add it to the comments and we can adjust above.

If you think we have missed anything regarding the outbound communication related to the release please chime in

/cc @hackygolucky

@mcollina
Copy link
Member

Both approaches are good for me, as long as we ship one before mid-august. The plan should be to ship 6.0 asap, or 5.9 if that is not possible.

@ZibbyKeaton
Copy link

@MylesBorins I think similar to last time, it would be nice to write a detailed blog on the subject and we can post it on our Medium blog and share on social. I can also share with a few media folks that are generally interested in this. Happy to review the blog and write an outline too, if that would be helpful to you. In terms of timing, do you happen to know when this will be?

@MylesBorins
Copy link
Author

We are still trying to work out the timing. How much lead time do you need for a media push?

@ZibbyKeaton
Copy link

@MylesBorins I will likely push this out to the media day of. I wanted to get a sense of timing, so I can put it into our editorial calendar and if it's around NodeSummit, I would reach out to the media attending the conference to give them a heads up.

@MylesBorins
Copy link
Author

@nodejs/ctc based around the above. How do people feel about doing a push to land V8 5.9 next week so we can do it around Node Summit and get a media push? I've tagged this for ctc-agenda so we can discuss tomorrow.

@addaleax
Copy link
Member

Sounds good to me – I’ll update the 8.2.0 proposal in a bit and remove V8 5.9 from it, the plan here sounds good and I can see that having better communications around the introduction of TF+I will be a good idea.

@mhdawson
Copy link
Member

I agree using the release where TF+I is enabled to generate awareness and to re-enforce the calls @addaleax has already been making asking for people to performance test and give us feedback is a good idea.

@winksaville
Copy link

I was asked by @vsemozhetbyt to provide a link to a performance issue I found between V8 5.8 and V8 6.1. Basically Typescript code targeting ES6 is 6x slower in my test than when targeting ES5. See my comment to nodejs issue 14220.

@mcollina
Copy link
Member

@winksaville from how I read that conversation, it seems the problem is resolved in V8 6.1. Can you please check if the same problem exists in 6.0 and 5.9?

@winksaville
Copy link

winksaville commented Jul 19, 2017 via email

@vsemozhetbyt
Copy link

@winksaville
V8 5.9: last nightly

Unfortunately, our first distributed V8-canary build already has V8 6.1. I hope somebody can link you to V8 6.0 Node.js build (I have an old canary one for Windows x64, let me know if it can be of any help).

@winksaville
Copy link

winksaville commented Jul 20, 2017 via email

@vsemozhetbyt
Copy link

Actually, this is still uncertain what V8 version will be shipped with Node.js 8 LTS (see, for example, nodejs/node#14384 (comment)).

@jasnell
Copy link
Member

jasnell commented Jul 20, 2017

If we're only a week out from having a stable 6.0, let's go with 6.0 but let's definitely work on getting it out by mid august. I'm cutting the 9.x working branch in early september and would like to get things resolved before then.

@winksaville
Copy link

Here are the performance numbers of test-nn-ts nodev9 branch running on my laptop using the various versions of Node. The takeaway is V8 6.1 is fastest followed by V8 5.9 and V8 5.8 is slowest:

**** Summary ES5 code:
nodev8-V8-5.8 20.07s
nodev9-V8-5.9 12.30s
nodev9-V8-6.1 12.13s

**** Summary ES6 code:
nodev8-V8-5.8 131.06s
nodev9-V8-5.9 19.36s
nodev9-V8-6.1 12.30s

**** Here are the node versions
$ node --version
v8.1.3
$ ./nodev9-V8-5.9/bin/node --version
v9.0.0-nightly20170718f406a7ebae
$ ./nodev9-V8-6.1.0/bin/node --version
v9.0.0-v8-canary201706199cf43ea3fd

**** Here are the V8 versions:
$ node -e "console.log(process.versions.v8)"
5.8.283.41
$ nodev9-V8-5.9/bin/node -e "console.log(process.versions.v8)"
5.9.211.38
$ ./nodev9-V8-6.1.0/bin/node -e "console.log(process.versions.v8)"
6.1.0 (candidate)

**** Details ES5 code:
$ node build-es5/test-nn.js 10000000
Epoch=10,000,000 Error=4.58e-8 time=20.07s eps=498,206

Pat Input0 Input1 Target0 Output0
0 0 0 0 0.00014153806736740904
1 1 0 1 0.9998568235021613
2 0 1 1 0.9998568475569924
3 1 1 0 0.0001748320545000856

$ ./nodev9-V8-5.9/bin/node build-es5/test-nn.js 10000000
Epoch=10,000,000 Error=4.58e-8 time=12.30s eps=813,074

Pat Input0 Input1 Target0 Output0
0 0 0 0 0.00014153806736740904
1 1 0 1 0.9998568235021613
2 0 1 1 0.9998568475569924
3 1 1 0 0.0001748320545000856

$ ./nodev9-V8-6.1.0/bin/node build-es5/test-nn.js 10000000
Epoch=10,000,000 Error=4.58e-8 time=12.13s eps=824,402

Pat Input0 Input1 Target0 Output0
0 0 0 0 0.00014153806736740904
1 1 0 1 0.9998568235021613
2 0 1 1 0.9998568475569924
3 1 1 0 0.0001748320545000856

**** Details ES6 code:
$ node build-es6/test-nn.js 10000000
Epoch=10,000,000 Error=4.58e-8 time=131.06s eps=76,300

Pat Input0 Input1 Target0 Output0
0 0 0 0 0.00014153806736740904
1 1 0 1 0.9998568235021613
2 0 1 1 0.9998568475569924
3 1 1 0 0.0001748320545000856

$ ./nodev9-V8-5.9/bin/node build-es6/test-nn.js 10000000
Epoch=10,000,000 Error=4.58e-8 time=19.36s eps=516,476

Pat Input0 Input1 Target0 Output0
0 0 0 0 0.00014153806736740904
1 1 0 1 0.9998568235021613
2 0 1 1 0.9998568475569924
3 1 1 0 0.0001748320545000856

$ ./nodev9-V8-6.1.0/bin/node build-es6/test-nn.js 10000000
Epoch=10,000,000 Error=4.58e-8 time=12.30s eps=812,744

Pat Input0 Input1 Target0 Output0
0 0 0 0 0.00014153806736740904
1 1 0 1 0.9998568235021613
2 0 1 1 0.9998568475569924
3 1 1 0 0.0001748320545000856

@Trott
Copy link
Member

Trott commented Jul 24, 2017

@MylesBorins Do we have a more-or-less decided path forward on this at this point? Is the plan to do a release with V5.9 this week to coincide with NodeSummit?

@rvagg
Copy link
Member

rvagg commented Jul 24, 2017

thanks for contributing the numbers @winksaville, they're actually really interesting!

@winksaville
Copy link

winksaville commented Jul 24, 2017 via email

@mcollina
Copy link
Member

Here is our writeup on Turbofan: https://www.nearform.com/blog/node-js-is-getting-a-new-v8-with-turbofan/

@ZibbyKeaton
Copy link

@MylesBorins @mcollina is this what we were originally planning to announce?

@mcollina
Copy link
Member

mcollina commented Jul 27, 2017

@ZibbyKeaton no, I don't think so. But we can lift as much we want from there, or even republish as-is on the Foundation blog.

@ZibbyKeaton
Copy link

@mcollina sorry for original comment. Didn't read everything. Yes, I would like to republish this on Node.js Collection. Do you have a Medium page or do you want me to post on the Node.js Foundation Medium page and then add it to Node.js Collection that way?

@mcollina
Copy link
Member

You can post it to the Foundation medium page, just add a link to the original piece at the beginning.

@TimothyGu
Copy link
Member

We should probably fix the errata (by @jkummerow and @hashseed) before republishing though.

@ZibbyKeaton
Copy link

We already published, but I can easily go in and make the edits, please just let me know what they are asap.

@vsemozhetbyt
Copy link

Refs: nodejs/node#14528

@TimothyGu
Copy link
Member

@ZibbyKeaton

  • When V8 was first built the optimizing JIT Compiler was dubbed: Crankshaft.

  • How "small integers" are tested. Specifically:

    function sum (base, max) {
      var total = 0
    
      for (var i = base; i < max; i++) {
        total += i
      }
    }
    // all big
    sum(65536, 131071)

    This function counts from 0, to 65536, to 65536 + 65537 = 131073, eventually going over the range of small int (2 ** 31 - 1 = 2147483647). However, unlike "all big" as the test claims to be, it is still going from small (0) to big.

    After my changes, the benchmark would then need to be re-run across the different V8 versions, but locally (Node.js v8.2.1 with V8 5.8):

    sum small x 21,061 ops/sec ±1.68% (88 runs sampled)
    from small to big x 14,684 ops/sec ±0.50% (91 runs sampled)
    all big x 9,515 ops/sec ±0.46% (90 runs sampled)
    

    On Node.js v9.0.0-v8-canary20170724c045f1f8dc with V8 6.2.4:

    sum small x 16,438 ops/sec ±1.40% (86 runs sampled)
    from small to big x 14,751 ops/sec ±0.67% (90 runs sampled)
    all big x 11,993 ops/sec ±5.12% (84 runs sampled)
    

    Note how there is a large drop-off from "from small to big" to "all big" on both versions, unlike the overlapping lines in the graph currently.

    The background info in the blog post will also need to be updated:

    In the case of integers (that is, when we specify a number in JS without a decimal), V8 assumes that all numbers are 32bit - until they aren’t. This seems like a fair choice, since in many cases a number is within the 0-65535-2147483648-2147483647 range. If a JavaScript (whole) number exceeds 655352147483647 the JIT Compiler has to dynamically alter the underlying type for the number to 64bita double (or: a double-precision floating point number) - this may also have potential knock on effects with regards to other optimizations.

  • Object creation benchmarks don't actually create the objects because TF optimized it away. Again, with my changes, the benchmark results are more useful:

    Node.js v8.2.1 with V8 5.8:

    literal x 28,455,810 ops/sec ±1.19% (83 runs sampled)
    class x 21,993,317 ops/sec ±1.13% (86 runs sampled)
    constructor x 28,093,899 ops/sec ±0.73% (82 runs sampled)
    

    Node.js v9.0.0-v8-canary20170724c045f1f8dc with V8 6.2.4:

    literal x 31,861,977 ops/sec ±0.88% (86 runs sampled)
    class x 32,265,246 ops/sec ±0.65% (90 runs sampled)
    constructor x 32,120,661 ops/sec ±0.99% (87 runs sampled)
    
  • Polymorphic benchmark results are unreproducible on V8's d8 CLI, so there might be something weirder going on. A note saying this could be attached to that specific section as it's not as easily fixable, like the following (but feel free to change it):

    Edit: We have been informed by the V8 team that the results for this specific benchmark are not reliably reproducible. The results and the subsequent analysis should therefore be taken as reference only, not at face value.

/cc @davidmarkclements @mcollina Would it be possible for you to re-run the updated benchmarks (davidmarkclements/v8-perf#6)?

@vsemozhetbyt
Copy link

Re https://www.nearform.com/blog/node-js-is-getting-a-new-v8-with-turbofan/:
Besides the notes in the comments, there are some draft fragments in the text:

(Maybe a sentence about the BigInt proposal should be included here?)

Grabbing all of an objects values (properties?)

@vsemozhetbyt
Copy link

Re "ITERATING OVER OBJECTS" plot: the orange line and the red line are blended so it seems the red line is missed. Maybe it is worth to be mentioned explicitly.

@vsemozhetbyt
Copy link

Re "CREATING OBJECTS": posible typo (of -> or):

less than half the speed of using an object literal of a constructor

@mcollina
Copy link
Member

Thanks everyone for reviewing! I don't think this issue is the right place for discussing those changes. Can you please open issues to the repo, so that we can easily reply and discuss each one individually?

We will address them asap.

@Fishrock123
Copy link

So the plan for this is with 6.0 next week or the week after? Is that correct?

@Trott
Copy link
Member

Trott commented Jul 29, 2017

So the plan for this is with 6.0 next week or the week after? Is that correct?

@Fishrock123 My understanding is that we intend to release Node.js 8.3.0 this upcoming week with V8 6.0. @nodejs/release

I'm going to remove the ctc-agenda label from this since, as far as I know, no one is opposed to that plan. Feel free to re-add it if I'm wrong or if there is some other aspect that needs to be addressed at the meeting. @MylesBorins

@Trott Trott removed the ctc-agenda label Jul 29, 2017
@ZibbyKeaton
Copy link

@MylesBorins are you still planning to write a blog around this for Node.js Collection?

@Trott
Copy link
Member

Trott commented Aug 13, 2017

Since 8.3.0 landed with TF/I, I think this can be closed. Comment or re-open if I'm wrong. Thanks.

@Trott Trott closed this as completed Aug 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests