Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

src: expose libuv handle/req id on process object #8090

Closed
wants to merge 4 commits into from

Conversation

bnoordhuis
Copy link
Member

Assign numeric identifiers to libuv handle and request wrappers and
expose them to JS land as process._uvContextId[0]. Provides an ersatz
"thread-local" state for tracking execution across asynchronous calls.

Future optimization: reuse identifiers so they keep fitting in a SMI?

R=@trevnorris?

This PR is a conversation starter. The goal is to have a cheap way of tracking the flow of execution across callbacks for instrumentation purposes.

@tjfontaine
Copy link

I think for now we should postpone landing this PR until after the meeting on friday, though most of us had already converged on something similar to this concept

@bnoordhuis
Copy link
Member Author

I understand that @othiym23 pointed out that management of IDs is problematic if there is no close event notification so I've added that (for handles) in commit af669a5.

Does it make sense to do the same thing for requests? They have a clearly delineated life cycle (they "close" when the callback is made) so it didn't seem that useful to me.

@trevnorris
Copy link

@bnoordhuis Let's wait until after #8110 lands.

The JS that's being removed (including the JS that wasn't merged in from my update) assigned a unique id in JS (see https://github.com/trevnorris/node/blob/remove-al-js/src/async-wrap-inl.h#L87). Though it would only assign a unique ID to the AsyncWrap classes that actually had something to listen on.

Do you still want a unique id to be assigned to every instance, despite whether it's being traced or not?

@trevnorris
Copy link

@bnoordhuis I've added a commit that still assigns a new unique id to every AsyncWrap instance, but the retrieval of the value takes a different approach (trevnorris/node@1716fce)

The performance for retrieving the value is minimal on the JS side (sub 20ns), and is less expensive than setting an object property in C++.

@mjsalinger
Copy link

With this proposed ability to track callbacks, it would also be very useful/helpful to have a method to list and inspect all of the callbacks currently scheduled for execution (i.e. to iterate through them).

@bnoordhuis
Copy link
Member Author

@mjsalinger Do process._getActiveHandles() and process._getActiveRequests() cover your needs? If not, can you go into more detail about what you're looking for?

Assign numeric identifiers to libuv handle and request wrappers and
expose them to JS land as process._uvContextId[0].  Provides an ersatz
"thread-local" state for tracking execution across asynchronous calls.

Future optimization: reuse identifiers so they keep fitting in a SMI?
Called whenever a libuv handle is closed.  The callback receives a
single argument, the id of the handle that was closed.
@jasnell
Copy link
Member

jasnell commented Aug 13, 2015

@bnoordhuis ... any further thoughts on this? is it still necessary?

@bnoordhuis
Copy link
Member Author

We can revisit this, it's useful, but I'll close it for now.

@bnoordhuis bnoordhuis closed this Aug 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants