Node Foundation TSC Meeting 2015-09-02

@bnoordhuis

Time

UTC Wed 02-Sep-15 20:00:

San Francisco: Wed 02-Sep-15 13:00
Amsterdam: Wed 02-Sep-15 22:00
Moscow: Wed 02-Sep-15 23:00
Sydney: Thu 03-Sep-15 06:00
Tokyo: Thu 03-Sep-15 05:00

Or in your local time:

Links

Minutes Google Doc: https://docs.google.com/document/d/1rXBdtsD9PJTExNXgzNZ9bez9oOjW45kLPjun4zdt0dY
Previous Minutes Google Doc: https://docs.google.com/document/d/1aB76ClCgdjZUw3p-gHq9j6YwU11zWgJ4pmYPEWk6gjE

Agenda

Extracted from tsc-agenda labelled issues and pull requests from the nodejs org prior to meeting.

nodejs/node

deps: update v8 to 4.5.103.30 #2632
Inspecting Node.js with Chrome DevTools #2546
Node.js v4 Release Timeline #2522

nodejs/build

Merge job overwrites metadata #179

Invited

@bnoordhuis (TSC)
@chrisdickinson (TSC)
@cjihrig (TSC)
@domenic (observer)
@Fishrock123 (TSC)
@indutny (TSC)
@jasnell (TSC)
@mhdawson (TSC)
@misterdjules (TSC)
@mikeal (observer)
@mscdex (TSC)
@orangemocha (TSC)
@piscisaureus (TSC)
@rvagg (TSC)
@shigeki (TSC)
@srl295 (TSC)
@trevnorris (TSC)

Notes

The agenda comes from issues labelled with tsc-agenda across all of the repositories in the nodejs org. Please label any additional issues that should be on the agenda before the meeting starts. I'm using a tool to fetch the list so it's not a big deal to collate.

Joining the meeting

Uberconference; participants should have the link & numbers, contact me if you don't.

Would love it if we could include a quick discussion on nominating new TSC members.

(At least just to pull together a short list, not necessarily to decide on today)

@indutny

A summary for the TS of Inspecting Node.js with Chrome DevTools #2546 would be appreciated from anyone involved in there, particularly what is it that the TSC needs to care about and are we being called upon to make decisions here. Preferably this should be given here before the meeting so we don't end up wasting all our time trying to grok it all.

I tagged deps: update v8 to 4.5.103.30 #2632 because there is an outstanding question of whether we want to try and slip V8 4.5 into v4 since it (surprisingly) went stable today/yesterday. There's pros and cons to this. Pros: we get a supported V8 for longer, we get some bugs fixed and some new features like arrow functions (a weak justification but I like how this adds to the "upgrade to v4" story that we need to be pushing), possibly some perf improvements (including a potential backport or @indutny's patch that just landed there). Cons: Uncertain at this stage if NAN supports this version, it was supposed to but it looks like the V8 team may have slipped a breaking API change in that wasn't pre-announced, there's also some potential pain for addon authors who may think they are ready when in fact they are not since there has been a full s/Handle/Local and although we knew about this, it wasn't widely known to addon authors who could get away with liberal Handle usage. Also cons: limited testing, while we've had v3.x using 4.4 for a while now.

TSC expansion discussion absolutely, I started an email thread about that but we should use some of the private time at the begining of the meeting to talk openly about it, I'd love to see us bring in more of the quality collaborators we have and get some more diverse perspectives and skills!

it looks like the V8 team may have slipped a breaking API change in that wasn't pre-announced

It would be good if this could be made a bit more concrete. Based on the thread it looks like this is referring to Local<Value> v8::Function::Call() being marked V8_DEPRECATE_SOON, which is two steps away from an actual breaking change (first, move to V8_DEPRECATE; second, actually remove).

it wasn't widely known to addon authors who could get away with liberal Handle usage.

Just want to point out that this should have been giving deprecation warnings, so there's at least some chance they'd be aware. Although there's no guarantee addon authors have been compiling with new enough V8, so I understand the general point.

@domenic

@domenic sorry, it's not Function::Call(), I was talking about Function::New() but looking again it and seeing the full section of the code it seems that this might be what @trevnorris was talking about too.

-  static Local<Function> New(Isolate* isolate,
-                             FunctionCallback callback,
-                             Local<Value> data = Local<Value>(),
-                             int length = 0);
+  static MaybeLocal<Function> New(Local<Context> context,
+                                  FunctionCallback callback,
+                                  Local<Value> data = Local<Value>(),
+                                  int length = 0);
+  static V8_DEPRECATE_SOON(
+      "Use maybe version",
+      Local<Function> New(Isolate* isolate, FunctionCallback callback,
+                          Local<Value> data = Local<Value>(), int length = 0));

Unfortunately I haven't been able to do any testing on this, no time at all. If anyone else has time it'd be great to get some testing of NAN on this version. @kkoopa do you have time to verify the NAN test suite against #2632 on top of vee-eight-4.5?

it wasn't widely known to addon authors who could get away with liberal Handle usage.

Handle is defined like this in the header file:

#if !defined(V8_IMMINENT_DEPRECATION_WARNINGS)
// Local is an alias for Local for historical reasons.
template <class T>
using Handle = Local<T>;
#endif

If I understand it well, as long as we don't enable V8_IMMINENT_DEPRECATION_WARNINGS in v4, addons using it will still work.

@targos

@targos my point is about all of the core APIs now exporting Local instead of Handle, but perhaps that's moot since it should just be an alias. Again, some testing would be nice. Those who would like to see 4.5 in should be out there testing addons and seeing what kind of work is required. I'd start by looking at buffertools, bignum, leveldown (ones that the TSC has some connection with), also interesting would be anything in npm that's depending on nan@2.x if you can figure that out. If we can get a level of confidence that there's not going to be much/any pain from this then it's more likely this will make it in.

@rvagg

@rvagg here is a list of all npm packages where the nan dependency satisfies 2.0.8:

buffer-indexof-fast
ed25519-supercop
gigecamera-simulator
ifx_db
iobroker.terminal
linux-ns
lru-addon
magickwand
mmap-io
node-shared-cache
nodeos-mount
qrencode-raw
sd-daemon
sdb
sypexgeo-nan
usage-nan
wocr
xenstat
zlib-raw-sync
bignum
blake2
forsake
imageplus
memwatch-next
mitie
nsfw
snappy
sse4_crc32
xz
fsevents
blackbone-node
png-img
utun
v8-debug
addon-emitter
bluetooth-hci-socket
bufferutil
capture-window
edgeswim-uvmon
farmhash
fds
fs-xattr
function-origin
gc-stats
gdx
hackrf
hiredis
inotify
macos-alias
modern-syslog
naio-test
natrium
node-minizip
node-openalpr
node-opencv-zbar
oid
pattimura
phash-image
sha3
torium
utf-8-validate
weak
webkitgtk
xml-conv-json
xpc-connection
deasync
cap
freetype2
lodepng
mknod
mmmagic
nbind
node-expat
node-gpgme
node-sass
re2
sharp
number-smusher
npool
neventemitter
keybase-sodium
sodium-prebuilt
kbd
sleep
lzma-native
node-libcurl
leveldown
nodejieba
pcsclite
traceview-bindings
zmq
iconv
node-ios-device
async_bench
effective
epoll
geoip
mosca
v8-profiler
arc4random
libxl
unix-dgram
arrayfire_js
electron-ref
ref
time
kinect2
microtime
mkfifo
runas
runas-compiled
userinfo

@domenic

All Function::New() does is call FunctionTemplate::New()->GetFunction(). It is probably not even in common use, as it was not available before 0.12.

There are four ways of fixing it:

Patch V8 to remove the offending commit
Patch NAN and keep the existing behavior
Patch NAN and update to the new behavior
Patch NAN and remove it completely.

Witchever the resolution is, I strongly oppose bumping the major version of NAN for this uselessness. 2.x is here to stay for at year, breakage be damned.

The Handle stuff is a non-issue. It is trivial to fix, it has been known for over two years that it needs to be fixed, and there is no promise of forward compatibility. It was well known already before the release of io.js 3.0.

On September 2, 2015 4:07:59 PM GMT+03:00, Rod Vagg notifications@github.com wrote:

@domenic sorry, it's not Function::Call(), I was talking about
Function::New() but looking again it and seeing the full section of
the code it seems that this might be what @trevnorris was talking about
too.

-  static Local<Function> New(Isolate* isolate,
-                             FunctionCallback callback,
-                             Local<Value> data = Local<Value>(),
-                             int length = 0);
+  static MaybeLocal<Function> New(Local<Context> context,
+                                  FunctionCallback callback,
+                                  Local<Value> data = Local<Value>(),
+                                  int length = 0);
+  static V8_DEPRECATE_SOON(
+      "Use maybe version",
+      Local<Function> New(Isolate* isolate, FunctionCallback callback,
+                          Local<Value> data = Local<Value>(), int
length = 0));

Unfortunately I haven't been able to do any testing on this, no time at
all. If anyone else has time it'd be great to get some testing of NAN
on this version. @kkoopa do you have time to verify the NAN test suite
against #2632 on top of vee-eight-4.5?

Reply to this email directly or view it on GitHub:
#2654 (comment)

Sorry, I can't make it today.

Most likely I won't be able to attend today.

@kkoopa

@kkoopa so my understanding of your four ways to fix it is that you would find patching NAN to keep the existing behavior or update to the new behavior both acceptable? Neither would require a major version bump, correct?

Anyway, again it sounds like this is not actually a removal, or even a deprecation, just a deprecation-soon. So NAN doesn't need to do anything at all, if it doesn't want to, at least for two more V8 releases. As @rvagg says though, getting some testing in would be crucial to confirm.

@domenic

@domenic Patching to update to new behavior would be a breaking change, but I still do not consider it significant enough to warrant a major version bump, regardless of semver this or semver that. The construct is probably not used outside of NAN's test suite.

@rvagg

@rvagg Summary for #2546:

The DevTools team has engaged us about separating v8-inspector from Blink and making a package that we could use. This would allow anyone to open Chrome, browse to chrome://inspect and point it at a node process. A few points from the discussion are:

The remote debugger API (that we'd consume) has remained fairly stable.
If it does break, it's still possible to load an old Chrome DevTools console to consume the older API. So this should continue to work in an LTS release.
From the thread: "On the DevTools side we’ll be providing generic API that would work for both Blink and Node.js".
The API will integrate pretty tightly with our existing code in order to collect all the information it needs. e.g. asynchronous stacks.
Node would have to fully support web sockets in order to communicate with Chrome.
Node would have to run another thread that also executes JS. Though I don't believe it would need the full event loop.

My main concern are the integration points. While many of them can be done in native, which makes the conditional practically a performance noop, we will have to add a conditional in nextTick(). Which performance has already degraded since additional logic has been added.

@paulirish listed some of the features that would be included: #2546 (comment)

I'm unsure of how this would conflict, or if it would conflict, with the work @nodejs/tracing is doing. @ofrobots Do you have any feedback here?

@rvagg

I can't make it today, let me know if there is anything you need from the
foundation and i'll take care of it.

On Wednesday, September 2, 2015, Trevor Norris notifications@github.com
wrote:

@rvagg https://github.com/rvagg Summary for #2546
#2546:

The DevTools team has engaged us about separating v8-inspector from Blink
and making a package that we could use. This would allow anyone to open
Chrome, browse to chrome://inspect and point it at a node process. A few
points from the discussion are:

The remote debugger API (that we'd consume) has remained fairly
stable.

If it does break, it's still possible to load an old Chrome DevTools
console to consume the older API. So this should continue to work in an LTS
release.

From the thread: "On the DevTools side we’ll be providing generic
API that would work for both Blink and Node.js".

The API will integrate pretty tightly with our existing code in
order to collect all the information it needs. e.g. asynchronous stacks.

Node would have to fully support web sockets in order to communicate
with Chrome.

Node would have to run another thread that also executes JS. Though
I don't believe it would need the full event loop.

My main concern are the integration points. While many of them can be done
in native, which makes the conditional practically a performance noop, we
will have to add a conditional in nextTick(). Which performance has
already degraded since additional logic has been added.

@paulirish https://github.com/paulirish listed some of the features
that would be included: #2546 (comment)
#2546 (comment)

I'm unsure of how this would conflict, or if it would conflict, with the
work @nodejs/tracing https://github.com/orgs/nodejs/teams/tracing is
doing. @ofrobots https://github.com/ofrobots Do you have any feedback
here?

—
Reply to this email directly or view it on GitHub
#2654 (comment).

@trevnorris

@trevnorris I don't see much conflict between v8-inspector and trace-event. Blink already uses both trace-event and DevTools.

The TSC call was yesterday? 😕

@mscdex

@mscdex I think that's just a typo.

@mscdex

@mscdex sorry, dates are hard .. I always have to wind back by a day when I do these cause I'm a day ahead, I obviously wound back by two!

@jkrems: yes, we've switched to being more timely now we have everything in place, the last minutes were merged with a soundcloud link in it and I'm going to try and be quicker in future in putting in the PRs. If you look in the doc/tsc-minutes/ directory you'll see SoundCloud links on all of those.

@rvagg

@rvagg The thing is - the meeting issues are easy to find (for me at least) but even after you told me that the meeting notes were merged with the soundcloud link, I wasn't able to find them (without cheating and using the exact directory path in your comment). E.g. Google only leads to https://nodejs.org/en/foundation/tsc/minutes/. I think just adding a short "Meetings notes will be made available in doc/tsc-minutes and audio recordings on SoundCloud." at the end of the issue template would go a long way. Just to make it easier to discover for people outside the loop. :)

Was confirmed that there's no need to add async stack trace support from the beginning: #2546 (comment)

@jkrems I'll try and keep that in mind for next week, feel free to hassle me if I've forgotten tho!

https://soundcloud.com/node-foundation/tsc-meeting-2015-09-02 is audio for this meeting, PRing in the minutes now

@trevnorris

@trevnorris

I'm unsure of how this would conflict, or if it would conflict, with the work @nodejs/tracing is doing.

Note that we already expose tracing over the remote debugging protocol. DevTools Timeline panel is implemented based on that. If it makes sense for Node.js too we can consider adding it to v8-inspector. The protocol implementation for tracing domain is fairly simple.

@rvagg

@rvagg Can this be closed now?

Navigation Menu

Repository navigation

Node Foundation TSC Meeting 2015-09-02 #2654

Time

Links

Agenda

nodejs/node

nodejs/build

Invited

Notes

Joining the meeting

3 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Node Foundation TSC Meeting 2015-09-02 #2654

Description

Time

Links

Agenda

nodejs/node

nodejs/build

Invited

Notes

Joining the meeting

Activity

jasnell commented on Sep 2, 2015

jasnell commented on Sep 2, 2015

rvagg commented on Sep 2, 2015

rvagg commented on Sep 2, 2015

domenic commented on Sep 2, 2015

rvagg commented on Sep 2, 2015

targos commented on Sep 2, 2015

rvagg commented on Sep 2, 2015

targos commented on Sep 2, 2015

kkoopa commented on Sep 2, 2015

shigeki commented on Sep 2, 2015

orangemocha commented on Sep 2, 2015

domenic commented on Sep 2, 2015

kkoopa commented on Sep 2, 2015

3 remaining items

trevnorris commented on Sep 2, 2015

mikeal commented on Sep 2, 2015

ofrobots commented on Sep 2, 2015

mscdex commented on Sep 2, 2015

cjihrig commented on Sep 2, 2015

rvagg commented on Sep 2, 2015

hybrist commented on Sep 2, 2015

trevnorris commented on Sep 2, 2015

rvagg commented on Sep 3, 2015

rvagg commented on Sep 3, 2015

yury-s commented on Sep 3, 2015

thefourtheye commented on Sep 6, 2015

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions