[refactor] initial rewrite to use native async_hooks #19
Conversation
visualizer/index.html
Outdated
| @@ -56,7 +56,7 @@ | |||
| section#info #stats { | |||
| float: right; | |||
| width: 155px; | |||
| height: 119px; | |||
| height: 135px; | |||
There was a problem hiding this comment.
Needs a better way to fit the info on the screen. My CSS is very rusty but Ideally we'd place the dprof version and whatnot to the left or right of the other info.
dprof.js
Outdated
|
|
||
| // Ignore our nextTick for the root duration | ||
| // TODO(Fishrock123): detect this better. | ||
| if (uid === 2) return |
There was a problem hiding this comment.
highlighting this
dprof.js
Outdated
| this.name = handle.constructor.name; | ||
| function Node(handle, id, name, stack) { | ||
| this.name = name; | ||
| this._id = id; |
There was a problem hiding this comment.
should maybe be this._uid = uid
dprof.js
Outdated
| } | ||
|
|
||
| function asyncBefore(uid) { | ||
| // Ignore our nextTick for the root duration | ||
| // TODO(Fishrock123): detect this better. |
There was a problem hiding this comment.
why is asyncHook.disable() not enough?
There was a problem hiding this comment.
@AndreasMadsen disable/enable are no longer tree based, and so you will still get the before/after/destroy.
There was a problem hiding this comment.
Ah yes, forgot about that. However stateMap can be used as a set of all seen init events. That should allow us to ignore all before, after, and destroy events where the init event was "disabled" away.
There was a problem hiding this comment.
@AndreasMadsen so if state === undefined skip?
There was a problem hiding this comment.
Yes. I was originally thinking stateMap.has(uid), but I guess your version is a little faster.
Fishrock123
force-pushed
the
native-async_hooks
branch
from
cf42e80
to
09641f9
Compare
|
Rebased on |
|
Great! It would be awesome if you could fix #10 simultaneously. That would also give us a change to evaluate |
|
@AndreasMadsen are you looking for the conceptual parent or technical one? As I understand it the |
|
@Fishrock123 the conceptual one, that is at least the current behaviour. It's almost what is implemented here, but I would like to avoid using const source = parentUid === 1 ? asyncHooks.currentId() : parentUid;By the way, do you have a good source for the |
Think I put it in there as a "note". Not as an actual section. Will improve that. Short of it is there are two reserved id's.
The code you have there is basically the same as changes I'm currently hammering out. |
dprof.js
Outdated
| const root = new Node( | ||
| 0, { 'constructor': { name: 'root' } }, | ||
| 0, |
There was a problem hiding this comment.
According to what @trevnorris said about the parentId, this should be 1.
Fishrock123
force-pushed
the
native-async_hooks
branch
from
09641f9
to
f54aeb3
Compare
Includes lots of debug statements.
We should probably wait to merge this until nodejs/node#8531 is merged.
cc @AndreasMadsen