doc: add 2016-10-05 diag WG meeting notes

wg-meetings/2016-10-05.md

@@ -0,0 +1,206 @@
+# Node.js Diag WG Meeting
+
+* Date: 2016-10-05
+
+## Attendees
+
+* Josh Gavant (@joshgav)
+* Thomas Watson (@watson)
+* Matt Loring    (@mattloring)
+* Kelvin Jin (@kjin000)
+* Andreas Madsen (@andreasmadsen)
+* Michael Dawson (@mhdawson)
+* Daniel Khan (@danielkhan)
+
+
+# Agenda
+
+## Previous Meeting
+
+### async_hooks
+
+Next steps:
+
+* One significant change remains to enable multiple listeners.
+* @trevnorris is working on a JS wrapper module for async_hooks to make it more useable for end users.
+
+### trace controller
+
+Next steps:
+
+* Matt and Ali continue working on V8 implementation.
+* Add trace points throughout Node core codebase.
+* Define and implement interfaces for JS and native modules to read from and write to trace system.
+
+
+## Today
+
+### Inspector & Chrome Debugging Protocol
+
+### Debug already running process [[node#8464](https://github.com/nodejs/node/issues/8464)]
+
+Do users need to connect to and diagnose already running processes?
+
+Need to determine what the use case is, and what perf impact this would have on running processes?
+
+What is the impact of enabling the debugger in a process? This will determine whether users would consider using it in production.
+
+Would there be need to debug running processes at dev time?
+
+Other diagnostic info also flows over this protocol, including trace (potentially in the future). That’s another reason to make this work.
+
+---
+
+### Message text for `--inspect` [[node#7182](https://github.com/nodejs/node/pull/7182)]
+
+1. Suggest a generic comment and/or URLs for many different tools.
+
+  Trouble is that long-term we’d have a lot to list and URLs are long. Also, most tools aren’t accessed with URLs.
+
+  Do we want Node.js to advertise tools?
+
+2. Provide a generic ws:// URL along with a URL pointing to deeper instructions in Node.js help pages.
+
+3. Could we have a specific protocol descriptor, e.g. node-tool://? Similar to what the chrome scheme currently is?
+
+**Next steps**:
+
+* @joshgav to work on Node.js wiki page.
+
+---
+
+### Profile for Node? [[diagnostics#52](https://github.com/nodejs/diagnostics/issues/52)]
+
+  Node’s profile is copied from V8’s. We should start with this and only modify if necessary for other engines.
+
+  Leave this until something makes us think about it again.
+
+**Next steps**:
+
+* Close issue until needed.
+
+---
+
+### V8 inspector with other runtimes [see [node#7393](https://github.com/nodejs/node/issues/7393)]
+
+  As Chakra and other engines begin using we can address concerns.
+
+**Next steps**:
+
+* Close until needed.
+
+---
+
+### CLI debugger for new protocol [node#7266](https://github.com/nodejs/node/issues/7266)
+
+Here: <https://github.com/buggerjs/node-inspect>
+
+How do you connect with this debugger?
+
+Copied lib/_debugger.js from Node. Old debugger is started with `node debug script.js`, which forks script.js and runs _debugger.js in original process, connects to target process.
+
+New impl does the same but passes `--inspect` instead of `--debug` to forked script.
+
+Current proposal is no worse than old implementation. Seems ideal to start with direct port.
+
+@jkrems: New protocol is more likely to change over time. Old protocol is stable.
+
+@mhdawson: Long term we need to plan for a transition.
+
+Core idea is to provide a `node inspect` command to parallel `node debug`.
+
+Eventually need to deprecate old one, plus introduce new features available in new protocol.
+
+Jan’s work now implements all that the old debugger did, but lacks automated test coverage, cleanup, review, etc.
+
+Can we reuse tests from previous version?
+
+@mhdawson: Why does this need to be in nodejs/node?
+
+@jkrems: In theory it’s similar to readable-stream and doesn’t require anything in nodejs/node.
+
+@mhdawson: nodereport isn’t in core, but we made it a project in Node.js. Should we do the same in this case?
+
+No opposition.
+
+Process: add governance artifacts like other repos (see nodereport repo for an example), then open an issue in TSC.
+
+Next steps:
+
+* All review Jan’s implementation.
+* Jan to prepare repo for inclusion in nodejs org.
+
+---
+
+### V8 TraceController
+
+Current: https://github.com/matthewloring/node/tree/tracing/src/tracing
+
+Current V8 implementation for adding new trace events is dependent on parts of V8 source which we can’t utilize. So Matthew copied these macros from V8 to node.
+
+This file must be copied: https://github.com/matthewloring/node/blob/tracing/src/tracing/trace_event.h
+
+Includes Tracing Controller work but no hooks into Node core. For that would like collaborative effort with Node.
+
+Matthew to add demo of instrumenting event loop to start collaboration process.
+
+@mhdawson: How hard is it to implement similar functionality in other VMs?
+
+@matthewloring: All traces generated by V8, Node, user modules all end up in one place.
+
+@mhdawson: Ideally we’d define a “Node” API which translates to the V8 one (and others).
+
+@matthewloring: This insight would be welcome when the PR is open.
+
+@mhdawson: Should we define a subset of the macros for module authors to use?
+
+@matthewloring: Full breadth has been used by V8 team, but fewer would be easier to submit in a Node patch.
+
+Next steps:
+
+* Review Matt’s PR now.
+* PR to Node to come soon.
+
+---
+
+### ES modules
+
+- [@jasnell’s post](https://hackernoon.com/node-js-tc-39-and-modules-a1118aecf95e#.yfla6a269)
+
+See “Idempotency Concerns”, specifically:
+
+> There are a wide variety of scenarios where this idempotency rule causes issues. Mocking, APM, and spying for testing purposes are primary examples. Fortunately, there are a number of ways this limitation can be addressed. One approach is to add hooks into the loading phase that would allow an ESM’s exports to be wrapped. Another is for TC-39 to allow loaded ESM’s to be replaced after they are loaded. Several mechanisms are being considered here. The good news is that while intercepting ESM’s will be different than intercepting CommonJS modules, interception will be possible.
+
+Editor: There wasn't time to discuss in depth at this time.
+
+---
+
+### async_hooks
+
+- AsyncWrap EP (node-eps#18)[https://github.com/nodejs/node-eps/pull/18]
+- async_hooks implementation (node#8531)[https://github.com/nodejs/node/pull/8531]
+
+- Name changed to async hooks :)
+- The AsyncWrap EP has been accepted by the CTC. Just needs to to merged, perhaps with some minor corrections.
+- There is still some discussion about how Promises should behave. Short story: two things are required for the .then(cb) "callback" to be called. 1) calling .then() 2) calling done(result) in the promise, so which is the natural parent?
+- @trevnorris has opened a PR nodejs/node#8531
+- @Fishrock123 has investigated the Timer behavior (with dprof), particularly active and enroll. A solution has been found and implemented.
+- Missing Embedder API, focus right now is on performance (I guess)
+- Once Embedder API is in place, we should look into NAN integration.
+- I'm sure @trevnorris would like any updates on Promise/Microtask integration.
+
+@andreasmadsen: Request for comments in [EP](https://github.com/nodejs/node-eps/pull/18)
+
+- Questions to answer:
+
+* Promises have 2 parents, which parent do people actually want?
+* How to integrate promises, since there are not yet hooks from V8?
+
+---
+
+## Logistics
+
+- Next meeting: Doodle for first week of November? Or prefer fixed time slot?
+
+Use a Doodle.
+