Call with V8 team, reloaded.

hashseed commented 7 years ago

I've been told that I should open a new issue for this.

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.

hashseed commented 7 years ago

Looks like the fifth option, March 30th, 6am CET, is the winner. The Hangouts link is here.

I'm looking forward to this!

joshgav commented 7 years ago

Added to Foundation calendar, click here for this individual event.

Note that 6am CET is 9pm US Pacific the previous day (Wednesday).

hashseed commented 7 years ago

Thanks for updating the calendar!

mscdex commented 7 years ago

Wouldn't that be 10pm Pacific time?

joshgav commented 7 years ago

@mscdex

Wouldn't that be 10pm Pacific time?

Till the last Sunday in March, yes. On March 26, summer time begins. :tada:

hashseed commented 7 years ago

Some preliminary agenda items that we want to bring up.

We came up with a plan for the future of GYP in Node.js as V8 is deprecating GYP support. I'd like to present it.
Part of Node.js internal JS is written to appease Crankshaft. We would like to contribute to change this post-V8-5.9. We'd like to discuss how, given that Node.js has not updated to 5.9 yet.
We'd like to find a set of benchmarks to monitor in order to find regressions in Node.js early.

If there are any other topics anyone is interested in, please add them.

Trott commented 7 years ago

Not to rehash this again and again, but the time is actually 6AM CEST and not CET, right? For the benefit of people using online convert-to-local-time utilities and whatnot...

hashseed commented 7 years ago

Correct. CEST 6am.

hashseed commented 7 years ago

This is now. https://staging.talkgadget.google.com/hangouts/_/google.com/node-ctc-v8

hashseed commented 7 years ago

Dumping meeting notes here. I may have confused who said what in some places. I was busy talking and typing.

Attendees

CTC: @trevnorris, @jasnell, @cjihrig, @Trott V8: @bmeurer, @hashseed

GYP to GN migration

The plan forward has been announced here as follow-up to previous meeting. V8 will introduce a GN wrapper to build the V8 build target. This can be used as alternative to the legacy GYP configs. The legacy GYP configs will then be removed from V8 upstream and need to be maintained by Node.js

@trevnorris: @jbergstroem, @indutny, @targos, and the build working group should be involved. @jasnell: What platforms does GN support? @hashseed: Ones supported by Chromium. In addition also PPC and MIPS. @jasnell: How hard is extending GN support? @hashseed: Have only gotten fuzzy answers from GN maintainers so far. May be fairly involved to set up GN configurations for platforms and toolchains. Devil is in the detail. @jasnell: Concern that niche platforms will require special maintenance soon. @jasnell: Any difference between binary produced by GN and GYP? @hashseed: There should be none. @hashseed: Maybe Node.js wants to get on track moving away from GYP altogether? @trevnorris: A lot of work would be involved here.

Contributing to Node.js

@hashseed: V8 has moved to a new compiling pipeline in 5.9. Some of the Javascript patterns in Node.js internal are tailored towards Crankshaft, and may regress with Turbofan. We want to address these issues by submitting PRs. This only applies to 5.9 and later. The window between Node.js officially updating to 5.9 and release is rather short, and we want to start working on this sooner. @jasnell: You could fork the Canary branch and hold off PRs to master until update to 5.9. @bmeurer shows example in the EventEmitter implementation. @bmeurer: We want to make sure Turbofan does not cause any regressions. Partly by replacing "Crankshaft-script" used in Node.js.

Some discussions on difficulties this brings to back merging fixes. Performance fixes due to new pipeline will need to be marked to not back merge.

@jasnell: Node 8 will stick with 5.7. Node 9 will get 5.9 or later. @hashseed: We could also simply accept regressions for now and fix them on master in the long run. @jasnell: Not preferable. Regressions should be fixed as early as possible. But waiting for 5.9 is what module implementers will need to do. @hashseed: V8 also maintains a vee-eight-lkgr branch. But unlike Canary it does not track Node master as closely. So it might make more sense to fork off Canary for these fixes. @jasnell: Make sure not to change the API signature during rewrite of internal JS. E.g. function length.

Benchmarking and Performance

@hashseed: There is a huge list of micro-benchmarks that run very long, unsuitable for CI. Is there anything else? @bmeurer: We track AcmeAir and Typescript compiler. Is there anything else representative of real workloads and not micro-benchmarks? Webpack, UglifyJS? @jasnell: @mscdex, @mhdawson, and the benchmarking working group are the right contacts

A list of things to improve in Node.js and V8 wrt performance:

On-heap ArrayBuffers in Node.js are not used at all because pointers to it can be exposed. Neutering buffers once they become obsolete improves performance if many buffers are involved. Trevor to come up with a benchmark to show this.
JSON.parse and JSON.stringify could probably be made faster. They are currently geared towards existing benchmarks. New benchmarks for Node.js would be awesome.
Call from C++ to JS is very expensive. Not tracked in traditional JS benchmarks. Blink may also benefit from making this faster for custom elements.
Streams benchmarks test parts of Node that are used often, and should be looked at.
Http, Path, Net, Url also fairly important.
Fs etc. not important.
Http exercises C++/JS boundary a lot.
Url.parse has been hand-optimized for Crankshaft. Make sure not to break algorithm when tuning for Turbofan.
TypedArrays are not only used for storage, but also used to store state. For some reason Float64Array is slower than Uin32Array in this setting. Trevor will come up with a benchmark to demonstrate this.
Constructing an object in C++ by setting properties is slow. With 3 or more properties it is faster to pay the JS-entry overhead and call a JS constructor. V8 should work on API ICs to solve this problem.
Using TypedArray to pass arguments and return value is often faster than pass JS objects. See heapStatisticsBuffer.
Turning Local\<Array> into a list of Local\<Value> is slow. Possibly due to crossing the API boundary repeatedly. This is often done to extract function arguments from an array before calling the function. Exposing spread call to API would help a lot.

refack commented 6 years ago

AFAICT the plan has two parts:

one time effort for a GYP triggers GN tool that V8 can use to CI
continuous effort to keep .gyp files in sync with .gn files

Who is responsible for (2)? /CC @targos

refack commented 6 years ago

/CC @seishun

MylesBorins commented 6 years ago

This should likely be opened on the tsc repo 😁

On Nov 3, 2017 2:42 PM, "Refael Ackermann" notifications@github.com wrote:

/CC @seishun https://github.com/seishun

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nodejs/CTC/issues/76#issuecomment-341807047, or mute the thread https://github.com/notifications/unsubscribe-auth/AAecVywbiZ3JEEXsl_S93pcZnGPb-QdCks5sy2ywgaJpZM4MQFYj .

mhdawson commented 6 years ago

Agreed a new issue probably makes sense.

nodejs / CTC