Call with V8 team #62

indutny · 2017-01-17T16:13:04Z

Hello everyone!

I think we should coordinate and schedule a joint call between Node.js CTC and V8 team to discuss following topic:

GYP vs GN, and how both projects could move forward on this
General performance feedback

cc @nodejs/ctc @bmeurer

sam-github · 2017-01-17T16:21:14Z

Object.assign()!

hashseed · 2017-01-17T17:21:10Z

cc @ofrobots

bmeurer · 2017-01-17T19:21:41Z

Would be really interested to hear feedback for Object.assign(). We spend some time optimizing that last year, but maybe there's more we can do!

sam-github · 2017-01-17T20:41:33Z

@cjihrig @Trott Why do we think its slower than util._extend(), and to be avoided?

cjihrig · 2017-01-17T21:14:56Z

I think every time someone benchmarked the two, util._extend() came out faster. That's not to say that Object.assign() might have improved since last we checked. Here are some related PRs:

People have reported seeing util._extend() win by up to 50%, although, again, that might not currently be the case. FWIW, I'm in favor of using Object.assign() if the performance has improved enough.

EDIT: A benchmark was provided in nodejs/node#7255.

Trott · 2017-01-17T21:47:02Z

Here are two different benchmarks with conflicting results. So which one is faster may depend on your use case.

Here's the one showing `util._extend()` being faster.

Here is a slightly modified version of a benchmark originally written by @jasnell.

Code

'use strict';

const common = require('../common');
const util = require('util');

const bench = common.createBenchmark(main, {
  method: ['Object.assign()', 'util._extend()'],
  size: [5, 10, 15, 30],
  n: [1e5]
});

function makeObject(size) {
  var m = {};
  for (var n = 0; n < size; n++)
    m[`key${n}`] = n;
  return m;
}

function useObjectAssign(n, obj) {
  bench.start();
  for (var i = 0; i < n; i++) {
    Object.assign({}, obj);
  }
  bench.end(n);
}

function useUtilExtend(n, obj) {
  bench.start();
  for (var i = 0; i < n; i++) {
    util._extend({}, obj);
  }
  bench.end(n);
}

function main(conf) {
  const n = +conf.n;
  const obj = makeObject(+conf.size);

  switch (conf.method) {
    case 'Object.assign()':
      useObjectAssign(n, obj);
      break;
    case 'util._extend()':
      useUtilExtend(n, obj);
      break;
    default:
      throw new Error('Unexpected method');
  }
}

Results

es/more-different-object-assign.js n=100000 size=5 method="Object.assign()": 1,250,221.8987592561
es/more-different-object-assign.js n=100000 size=10 method="Object.assign()": 631,233.9839762895
es/more-different-object-assign.js n=100000 size=15 method="Object.assign()": 370,360.6970976447
es/more-different-object-assign.js n=100000 size=30 method="Object.assign()": 91,588.78644709584
es/more-different-object-assign.js n=100000 size=5 method="util._extend()": 4,757,977.84419069
es/more-different-object-assign.js n=100000 size=10 method="util._extend()": 2,845,257.808810056
es/more-different-object-assign.js n=100000 size=15 method="util._extend()": 2,240,614.63106566
es/more-different-object-assign.js n=100000 size=30 method="util._extend()": 253,510.71525401654

Here's the one showing `Object.assign()` being faster.

This is a modified version of a benchmark originally written by @suryagh.

Code

'use strict';

const common = require('../common.js');
const util = require('util');

const bench = common.createBenchmark(main, {
  n: [1, 10, 100, 1000],
  method: ['_extend', 'assign']
});

const methods = {
  '_extend': util._extend,
  'assign': Object.assign
}

function main(conf) {
  const n = +conf.n;
  const method = methods[conf.method];

  const target = {};

  bench.start()
  for (var i = 0; i < n; i++) {
    method(target, {i: i});
  }
  bench.end(n)
}

Results

es/object-assign.js method="_extend" n=1: 11,816.559726801139
es/object-assign.js method="assign" n=1: 13,891.590031394993
es/object-assign.js method="_extend" n=10: 73,593.80634525798
es/object-assign.js method="assign" n=10: 74,392.03112562583
es/object-assign.js method="_extend" n=100: 646,663.2177961718
es/object-assign.js method="assign" n=100: 716,707.1606212418
es/object-assign.js method="_extend" n=1000: 2,613,279.116502596
es/object-assign.js method="assign" n=1000: 3,053,313.914256839

andrasq · 2017-01-17T22:27:51Z

they're different tests -- one copies an object with 10,15,30 fields, the other
transfers 10,100,1000 properties to an existing object one property at a time.

Trott · 2017-01-17T23:46:35Z

they're different tests -- one copies an object with 10,15,30 fields, the other
transfers 10,100,1000 properties to an existing object one property at a time.

Yes, I probably shouldn't have said "conflicting". I didn't mean "contradictory" or "paradoxical" or anything like that. I just meant what results you get depend on what you're testing. Although I guess I should also add that the one showing util._extend() faster is probably the more realistic/typical use.

bmeurer · 2017-01-18T04:51:50Z

cc @verwaest who did the Object.assign() improvements.

jasnell · 2017-01-18T05:05:40Z

I was just playing with this a bit earlier this week. util._extend() consistently outperformed Object.assign() by about 200-300%. The different was significant. I included a new benchmark test in nodejs/node#10849

andrasq · 2017-01-19T19:03:30Z

I can confirm the above, _extend is 2-5x faster. The exceptions are the
empty object {} (assign 2x faster) and objects with a single property (30% faster),
and object whose property names are numeric strings. The last two are why
the test that copies properties singly favors assign.

Also, there is an order of magnitude speed difference between copying objects
whose properties are indexed by numeric strings "123" vs words "a123"
(indexing by words up to 12x faster), but this has to be due to v8 internals.
The speed to copy gradually slows for large numeric values up to 4 billion (2^32);
above 2^32 numeric strings run as fast as words. This quirk can bias benchmarks.

E.g., extend/assign relative rates for an object with 10 properties:
property names numeric strings '0' .. '9':
extend: 1.00x
assign: 1.25x
property names '1230' .. '1239':
extend: 1.18x
assign: 0.90x
property names '4000000000' ... '4000000009':
extend: 0.45x
assign: 0.80x
property names words 'a0' .. 'a9':
extend: 12x
assign: 1.9x
(timed with node-v7.4.0)

(Unrelated, but surprisingly _extend reverses the order of the properties;
_extend({}, {a:1, b:2}) yields the unexpected {b:2, a:1}.)

verwaest-zz · 2017-01-19T20:33:03Z

There is definitely still room for improvement in Object.assign. We'll should to look into it again sometime soon. OTOH you have to be careful with microbenchmarks for hand rolled versions because they may look fast in isolation but won't always give good perf when used in an application. They will quickly really on a shared cache that only starts conflicting when there are other users as well. Object.assign doesn't have that issue.

…

On Thu, Jan 19, 2017, 20:03 Andras ***@***.***> wrote: I can confirm the above, _extend is 2-5x faster. The exceptions are the empty object {} (assign 2x faster) and objects with a single property (30% faster), and object whose property names are numeric strings. The last two are why the test that copies properties singly favors assign. Also, there is an order of magnitude speed difference between copying objects whose properties are indexed by numeric strings "123" vs words "a123" (indexing by words up to 12x faster), but this has to be due to v8 internals. The speed to copy gradually slows for large numeric values up to 4 billion (2^32); above 2^32 numeric strings run as fast as words. This quirk can bias benchmarks. E.g., extend/assign relative rates for an object with 10 properties: property names numeric strings '0' .. '9': extend: 1.00x assign: 1.25x property names '1230' .. '1239': extend: 1.18x assign: 0.90x property names '4000000000' ... '4000000009': extend: 0.45x assign: 0.80x property names words 'a0' .. 'a9': extend: 12x assign: 1.9x (timed with node-v7.4.0) (Unrelated, but surprisingly _extend reverses the order of the properties; _extend({}, {a:1, b:2}) yields the unexpected {b:2, a:1}.) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#62 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASgHlve2POgC9NwuTk2LmGBd5lc5p-gks5rT7OFgaJpZM4Ll2Fv> .

ofrobots · 2017-01-19T21:39:16Z

I think the discussion on Object.assign is potentially interesting, but it seems that it has hijacked the original intent of this issue. I do think it would good to have a call w/ the V8 team. Any interest from the CTC on that?

As for logistics, since most of the V8 team is Central European time-zone, perhaps the standard CTC afternoon Europe / morning Pacific time slot could work, but on a different day?

Trott · 2017-01-19T22:05:19Z

I think the discussion on Object.assign is potentially interesting, but it seems that it has hijacked the original intent of this issue. I do think it would good to have a call w/ the V8 team. Any interest from the CTC on that?

o/

As for logistics, since most of the V8 team is Central European time-zone, perhaps the standard CTC afternoon Europe / morning Pacific time slot could work, but on a different day?

That would be the 16:00 UTC time which (depending on daylight savings status) is 8am PST, 9am PDT, 5pm CET, and 6pm CEST.

(For anyone who cares: Confusingly, the "S" in PST means "standard" so that's the time in winter as compared to "daylight savings" time in summer. The "S" in CEST is for "summer" so it's basically the opposite of the "S" in PST.)

hashseed · 2017-01-20T07:47:09Z

5pm CET and later generally work well for me.

mhdawson · 2017-01-23T20:11:00Z

I'd be interested.

Trott · 2017-01-23T21:04:22Z

I think all that's needed at this point is someone willing to do the logistical work: possibly a Doodle poll, definitely schedule a time, probably put together an agenda...that's probably about it. Decisions like Hangout vs. Uberconference etc. can be made later based on how many people are attending and who they are.

Fishrock123 · 2017-01-30T16:11:46Z

Is this still being planned? Should we open a Doodle to find out times?

Trott · 2017-01-31T05:08:36Z

Is this still being planned? Should we open a Doodle to find out times?

I think all that's needed at this point is someone willing to do the logistical work: possibly a Doodle poll, definitely schedule a time, probably put together an agenda...that's probably about it.

hashseed · 2017-01-31T06:29:40Z

I've set up a doodle here: http://doodle.com/poll/r2wz2vqqr9rk925i
I added six days from Feb 13 to Feb 21, for 8am, 9am, or 10am. I'm proposing to use Google Hangouts, but am really open for any alternative.
Regarding agenda, I'd like to talk about:

Future of GYP
Performance bottlenecks
V8 features that could be leveraged by Node.js, e.g. startup snapshots
How to prepare Node.js for API deprecations, e.g. debug context API
Whether to make this meeting a regular thing

joshgav · 2017-01-31T19:25:31Z

Is this open to all collaborators? I'd like to join. Thanks!

hashseed · 2017-02-10T22:45:39Z

Fewer people than I hoped actually signed up on the Doodle poll. If we want to go through with this nevertheless, I suggest doing the call on Feb 15, 10am PST.

I prepared a document on the GYP issue as to get everyone on the same page for discussion: https://docs.google.com/document/d/1gvHuesiuvLzD6X6ONddxXRxhODlOJlxgfoTNZTlKLGA/edit#

@natorion could you help me setting up a public Hangouts session?

joshgav · 2017-02-11T00:57:00Z

@hashseed

Fewer people than I hoped actually signed up on the Doodle poll.

From my experience around here, 13 is a pretty nice turnout 😄 .

cc @digitalinfinity @kunalspathak

hashseed · 2017-02-11T07:39:18Z

Well half are from the V8 team. Talking among the V8 team is not really the goal :)

evanlucas · 2017-02-11T12:57:20Z

Sorry, I somehow missed this. I added myself to the doodle, but if I'm too late, then that's ok.

natorion · 2017-02-11T17:18:50Z

I suppose it makes most sense to make the meeting public id public and post it here? Than everybody can join easily.

hashseed · 2017-02-11T19:05:16Z

Wednesday 10am PST still winning. Making the hangout public sgtm.

bnoordhuis · 2017-02-11T21:13:41Z

Future of GYP

I guess discussion of gyp is a good reason to join... I expect you will be dropping the gyp files at some point?

joshgav · 2017-02-13T15:54:06Z

Added to calendar: direct event link.

mhdawson · 2017-02-13T22:52:32Z

I know this is listed as CTC meeting with v8 team but I'm wondering if it would be ok if @jbajwa were to join in (possibly as an observer) as he is involved in the v8 ports to PPC and s390 in the core V8 repos and the Node.js repos. I'm asking becuse its not clear if the meeting is open to all or just CTC members.

hashseed · 2017-02-14T06:04:44Z

@mhdawson I don't really mind. Given that the GYP migration probably affects less popular platforms more than popular ones, I think it actually makes sense.

williamkapke · 2017-02-14T22:15:33Z

Will this be broadcast live? or just a google hangout?

I'll want to watch live or maybe after it is over if I miss it.
Can we get this on the Node.js YouTube channel?... or is it going to some v8 channel?

natorion · 2017-02-15T14:47:19Z

I didn't set up any live streaming and I was not planning to record it. There is definitely the possibility for the latter though.

hashseed · 2017-02-15T19:13:23Z

Thanks for joining the meeting! It was very valuable to us! A few quick notes on what we discussed. Please correct me if I misunderstood anything.

GYP deprecation:

Native modules will keep using GYP in the foreseeable future.
The build system of Node core is decoupled from native modules though.
GN seems not to be a suitable option.
General sentiment leans towards the "legacy" and "transpiler" options. This puts the burden on whoever updates V8, and makes CI difficult.
Big interest in CI or Nightly builds. @targos was mentioned for currently working on this.
IBM has a solution for building for PPC with GN.
Discussion on this needs to be taken public.

Snapshot and code cache

Startup snapshot and code caching seem interesting, and worth keeping an eye on.
But probably not that important for Node.js, since there are still other low-hanging fruits to pick.

Debug context deprecation

Debug context is being deprecated at the end of the year.
Inspector protocol is the alternative, and can replicate most functionality of the debug context.
Mirror objects are going away with it as well.
Maintainers of native modules that use the debug context need to be warned.
LiveEdit is going to be reimplemented, but will remain usable through the inspector protocol.

Misc

Separate meeting regarding ES6 modules.
Repeat this format in four weeks. New Doodle to be set up.

@ofrobots could you make the recording of the meeting available? Thanks.

ofrobots · 2017-02-16T15:23:25Z

@ofrobots could you make the recording of the meeting available?

I am not sure if I can do that. Apparently we needed consent from everyone involved 😓.

rvagg · 2017-02-20T09:19:56Z

Inspector protocol is the alternative, and can replicate most functionality of the debug context.

Just out of interest. What are we losing here (noting "most")? Anything that might upset people using the debugger or using it from add-ons?

hashseed · 2017-02-21T15:03:43Z

I don't have a full list of missing features. We were able to migrate all of V8's debugger tests that use the debug context to the inspector protocol, with a wrapper to emulate the debug context API.

Notably missing is true mirror support. You can still do most of the things that mirrors offer, but you can't get access to the actual object you created the mirror for. That's because of the limitations of the inspector protocol. There are a few smaller features that have no inspector counter part, which are implemented as runtime functions (prefixed with %) in the wrapper.

hashseed · 2017-03-01T06:41:10Z

Since the last meeting was reasonably well-received, and we did agree on repeating it, here's the Doodle link for the next one: http://doodle.com/poll/h8td28wnzt3kdmmr

@bmeurer wants to talk about performance pain points for Node. Other agenda items are welcome too.

MylesBorins · 2017-03-01T16:08:49Z

@hashseed what timezone is the doodle in? All I see is either 6 am or 7 am...

targos · 2017-03-01T16:26:05Z

@MylesBorins 6/7 am would be CET. You can change the timezone, though. See on the right:

MylesBorins · 2017-03-01T16:27:25Z

Thanks!

…

On Wed, Mar 1, 2017 at 11:26 AM, Michaël Zasso ***@***.***> wrote: @MylesBorins <https://github.com/MylesBorins> 6/7 am would be CET. You can change the timezone, though. See on the right: [image: image] <https://cloud.githubusercontent.com/assets/2352663/23469493/0d5593ec-fea4-11e6-945e-b576951a92e8.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#62 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAecV3D2Wrd-OOJHhXIhu3S1TV3qYCtcks5rhZwegaJpZM4Ll2Fv> .

joshgav · 2017-03-01T18:27:43Z

@hashseed thanks! It might be more discoverable and easier to follow the thread if you open a new issue for the new meeting.

mhdawson · 2017-03-06T20:56:15Z

I'd like to attend but the proposed times are the middle of the night for me (12/1 AM).

hashseed · 2017-03-06T21:12:09Z

Yes. These times are not ideal for everyone. I'd like to switch to other times on a follow up meeting if there is interest. But for this one let's keep the current choices.

hashseed · 2017-03-14T12:28:09Z

Just in case the other issue has low visibility: looks like March 30th, 6am CET, is the winner for the second meeting. Hangouts link.

joshgav · 2017-03-14T21:13:10Z

Let's use #76 to track the new agenda and close this one now. Thanks!

`Object.assign` is much slower than `util._extend` according to the refs. This change is to convert the `Object.assign` to use `util._extend` in url module for improving profermance. Refs: nodejs/CTC#62

`Object.assign` is much slower than `util._extend` according to the refs. This change is to convert the `Object.assign` to use `util._extend` in url module for improving profermance. PR-URL: #16081 Refs: nodejs/CTC#62 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>

`Object.assign` is much slower than `util._extend` according to the refs. This change is to convert the `Object.assign` to use `util._extend` in url module for improving profermance. PR-URL: nodejs/node#16081 Refs: nodejs/CTC#62 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>

`Object.assign` is much slower than `util._extend` according to the refs. This change is to convert the `Object.assign` to use `util._extend` in url module for improving profermance. PR-URL: #16081 Refs: nodejs/CTC#62 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>

indutny changed the title ~~Call with a V8 team~~ Call with V8 team Jan 17, 2017

outsideris mentioned this issue Feb 11, 2017

Summary tag? outsideris/issue-translator-extension#9

Closed

ericmdantas mentioned this issue Feb 14, 2017

lib: remove unnecessary assignments with _extend nodejs/node#11364

Closed

2 tasks

This was referenced Feb 15, 2017

Don't include common.gypi from node source tree nodejs/node-gyp#1118

Open

Node v7.x seems to be unsupported node-inspector/v8-debug#32

Open

joshgav closed this as completed Mar 14, 2017

ofrobots mentioned this issue Apr 11, 2017

doc: deprecate vm.runInDebugContext nodejs/node#12243

Closed

4 tasks

hashseed mentioned this issue Apr 12, 2017

Next V8 - CTC call #101

Closed

hashseed mentioned this issue Jul 19, 2017

Call with V8 team, #3 #156

Closed

starkwang mentioned this issue Oct 8, 2017

url: using util._extend for improving profermace nodejs/node#16081

Closed

3 tasks

Call with V8 team #62

Call with V8 team #62

Comments

indutny commented Jan 17, 2017

sam-github commented Jan 17, 2017

hashseed commented Jan 17, 2017

bmeurer commented Jan 17, 2017

sam-github commented Jan 17, 2017

cjihrig commented Jan 17, 2017 • edited Loading

Trott commented Jan 17, 2017

Code

Results

Code

Results

andrasq commented Jan 17, 2017

Trott commented Jan 17, 2017

bmeurer commented Jan 18, 2017

jasnell commented Jan 18, 2017

andrasq commented Jan 19, 2017

verwaest-zz commented Jan 19, 2017 via email

ofrobots commented Jan 19, 2017

Trott commented Jan 19, 2017

hashseed commented Jan 20, 2017

mhdawson commented Jan 23, 2017

Trott commented Jan 23, 2017

Fishrock123 commented Jan 30, 2017

Trott commented Jan 31, 2017

hashseed commented Jan 31, 2017

joshgav commented Jan 31, 2017

hashseed commented Feb 10, 2017

joshgav commented Feb 11, 2017

hashseed commented Feb 11, 2017

evanlucas commented Feb 11, 2017

natorion commented Feb 11, 2017 • edited Loading

hashseed commented Feb 11, 2017

bnoordhuis commented Feb 11, 2017

joshgav commented Feb 13, 2017

mhdawson commented Feb 13, 2017

hashseed commented Feb 14, 2017

williamkapke commented Feb 14, 2017

natorion commented Feb 15, 2017 • edited Loading

hashseed commented Feb 15, 2017

ofrobots commented Feb 16, 2017

rvagg commented Feb 20, 2017

hashseed commented Feb 21, 2017

hashseed commented Mar 1, 2017 • edited Loading

MylesBorins commented Mar 1, 2017

targos commented Mar 1, 2017

MylesBorins commented Mar 1, 2017 via email

joshgav commented Mar 1, 2017

mhdawson commented Mar 6, 2017

hashseed commented Mar 6, 2017

hashseed commented Mar 14, 2017

joshgav commented Mar 14, 2017

cjihrig commented Jan 17, 2017 •

edited

Loading

natorion commented Feb 11, 2017 •

edited

Loading

natorion commented Feb 15, 2017 •

edited

Loading

hashseed commented Mar 1, 2017 •

edited

Loading