Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tracing: etw/perf/lttng/etc multi-isolate support #18074

Closed
bnoordhuis opened this issue Jan 10, 2018 · 4 comments
Closed

tracing: etw/perf/lttng/etc multi-isolate support #18074

bnoordhuis opened this issue Jan 10, 2018 · 4 comments
Labels
embedding Issues and PRs related to embedding Node.js in another project. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events.

Comments

@bnoordhuis
Copy link
Member

Continuing from nodejs/help#1031: src/node_counters.cc and its ilk need to have their initialization split out into a per-isolate and per-context step for embedding to work with them. Some thoughts:

  1. node.js won't know if initialization already happened if the isolate is created by the embedder, unless extra bookkeeping is added. Could hang off MultiIsolatePlatform::RegisterIsolate() - or could it?

  2. the tracing code itself isn't multi-isolate ready in the slightest, it's all globals that need to move to IsolateData and Environment

  3. some tracing flavors have (possibly inevitable) process-global state that should be protected by a uv_once_t or node::Mutex.

@bnoordhuis bnoordhuis added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events. embedding Issues and PRs related to embedding Node.js in another project. labels Jan 10, 2018
@Let0s
Copy link

Let0s commented Jan 25, 2018

I think this line and next one (which set callbacks to isolate) should not depends on Environment (embedder can create one isolate and set it for every Environment, that will be created). IMHO, adding callbacks should be somewhere in NodePlatform class, when it registers isolate.

@Trott
Copy link
Member

Trott commented Aug 2, 2019

@bnoordhuis This should remain open, yes? If so, is there someone particularly well-suited to try to move it forward? @nodejs/embedders maybe?

@bnoordhuis
Copy link
Member Author

Yes, it's still an issue. I don't have suggestions for volunteers but they're more than welcome.

@bnoordhuis
Copy link
Member Author

I think this issue has become obsolete after the removal of dtrace and etw in #43652 (and lttng and perfctr back in 2018.) I'll go ahead and close it.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
embedding Issues and PRs related to embedding Node.js in another project. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events.
Projects
None yet
Development

No branches or pull requests

4 participants