Repository navigation
Rewrite to use actual async_hooks #17
Description
See nodejs/node#8531
I've already started on this and got it working, but there are some relational challenges with timers and some underlying resources such as the HTTP parser.
I'll probably need to fix #16 in the process and use a flat data file by UID, and convert some of the render ordering to use that. (To deal with re-used timers...)
Metadata
Metadata
Assignees
Labels
No labels
Activity
AndreasMadsen commentedon Sep 18, 2016
Yes!, was thinking about this too. I have previously discovered a few bugs in
async_wrapwithdprof. It would be great to have it working withasync_hooks, we might discover a few issues there too.Not quite sure how #16 is related to this. It sounds like you want to create a graph rather than a tree.
Fishrock123 commentedon Sep 18, 2016
@AndreasMadsen sorry, my knowledge of data structures is not the best.
Let's put it this way:
Handles with the same ID should display on the same line in the visualizer even if they are triggered from different points.
Now, that may not make 100% sense at the current time but I'm not sure how else to display timers is a conceptual and technically correct way.
That is, if you take the same timer object, do an
insertof it (active),unenrollit (remove it), and re-insertit it, it should probably keep the same ID while firinginit -> destroy -> init.AndreasMadsen commentedon Sep 18, 2016
Ah, okay. That should be fine. Though the idea of uids popping back into existence is a little confusion and perhaps dangerous.
Fishrock123 commentedon Sep 18, 2016
Take, for example, this.
The handles with the line in-between are actually the same, the timeout was created and the updated three times, from different places, causing re-insertion.
UIDs at the current time (with patches for timers): 12, 17, 20, then 26.
AndreasMadsen commentedon Sep 18, 2016
I think my understanding of the node.js timer implementation is vastly insufficient to understand how a timer can be updated without becoming a different timer. In normal JS I would write
createTimeout(timer); timer = setTimeout(fn, update), but obviously that is not what is going on here.In any case it does complicate things quite a bit, because a single handle will have multiple parents (no longer a tree). However because there is still a parent-child relationship, it should be possible to visualize it like it's done now, see Topological Sorting of a DAG.
Fishrock123 commentedon Sep 18, 2016
I suppose this depends on the definition of "timer".
Timers are not restricted to being
Timeoutsand can actually be any any object with an._onTimeout = function(){}property that is then passed totimers.enroll()andtimers.active().That being said, in any timers implementation, changing the timeout duration or timeout start means re-ordering the timer in an underlying queue.
In our case, due to optimizations, changing the duration may cause a new underlying TimerWrap to be created. Changing the timeout start will not.
... etc
Perhaps it really does only make sense to fire init/destroy once for these. Not 100% sure yet.
Fishrock123 commentedon Sep 18, 2016
Thus, adding to the confusion, the timer in the above picture is really a
Net.Socket.Fishrock123 commentedon Sep 19, 2016
Maybe just using the same UID without firing events for re-insertion before
destroy()is the way to go:(handle is orange and UID 12.)
Refs: nodejs/node#8531 (comment)
Fishrock123 commentedon Sep 19, 2016
#19
AndreasMadsen commentedon May 30, 2017
Thanks for the work, I've landed
async_hookssupport.