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

worker: add function to get address of current isolate #43810

Closed
kvakil opened this issue Jul 13, 2022 · 4 comments
Closed

worker: add function to get address of current isolate #43810

kvakil opened this issue Jul 13, 2022 · 4 comments
Labels
feature request Issues that request new features to be added to Node.js. stale worker Issues and PRs related to Worker support.

Comments

@kvakil
Copy link
Contributor

kvakil commented Jul 13, 2022

What is the problem this feature will solve?

When running NodeJS with --trace-gc (or --trace-gc-nvp /
--trace-gc-verbose), you get a line which looks like this:

[1847:0x5dad670]       41 ms: Scavenge 5.0 (5.4) -> 4.1 (5.7) MB, 1.3 / 0.0 ms  (average mu = 1.000, current mu = 1.000) allocation failure 

The first number 1847 is the PID of the NodeJS process, and the second hex
number 0x5dad670 is the address of the NodeJS isolate.

Unfortunately, when running with worker threads, multiple isolates exist, and
so it can be difficult to correlate application-level logging with the output
of --trace-gc.

What is the feature you are proposing to solve the problem?

Add a new function to worker_threads (or v8?) which returns the address of
the current NodeJS isolate as a string. This way application developers can
correlate structured log lines with these GC trace lines.

What alternatives have you considered?

Per #42346, trace-gc is a "Tier 1" tool, and this issue makes it
significantly less useful with worker threads. At $WORK we have a native
addon which does this currently, but I think it's not unreasonable for
this to be in core NodeJS?

Anyway, I defer to the wisdom of the crowds.

@kvakil kvakil added the feature request Issues that request new features to be added to Node.js. label Jul 13, 2022
@kvakil
Copy link
Contributor Author

kvakil commented Jul 13, 2022

(If it's not obvious from the issue description; I'm happy to make a PR with an implementation & tests if such a patch would be welcome.)

@VoltrexKeyva VoltrexKeyva added the worker Issues and PRs related to Worker support. label Jul 13, 2022
@bnoordhuis
Copy link
Member

Sounds reasonable to me, go for it.

An alternative approach would be to scan the command line for flags and when they're present make node print the relevant information (thread id?)

Neither approach seems strictly superior, so 🤷

kvakil added a commit to kvakil/node that referenced this issue Jul 16, 2022
This change adds two functions:
1. `require('worker_threads').getIsolateAddress()`: gets the address of
   the isolate executing currently.
2. `worker.getIsolateAddress()`: gets the address of the isolate
   corresponding to the worker.

Fixes: nodejs#43810
kvakil added a commit to kvakil/node that referenced this issue Jul 19, 2022
This change adds an experimental function `v8.getIsolateAddress()`,
which returns the address of the currently executing isolate. It is
intended to be used by application developers to correlate the output of
`--trace_gc` with application-level log lines.

Fixes: nodejs#43810
kvakil added a commit to kvakil/node that referenced this issue Jul 19, 2022
This change adds an experimental function `v8.getIsolateAddress()`,
which returns the address of the currently executing isolate. It is
intended to be used by application developers to correlate the output of
`--trace_gc` with application-level log lines.

Fixes: nodejs#43810
kvakil added a commit to kvakil/node that referenced this issue Jul 19, 2022
This change adds an experimental function `v8.getIsolateAddress()`,
which returns the address of the currently executing isolate. It is
intended to be used by application developers to correlate the output of
`--trace_gc` with application-level log lines.

Fixes: nodejs#43810
kvakil added a commit to kvakil/node that referenced this issue Jul 20, 2022
This change adds an experimental getter `v8.isolateAddress`, which
returns the address of the currently executing isolate. It is intended
to be used by application developers to correlate the output of
`--trace_gc` with application-level log lines.

Fixes: nodejs#43810
@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jan 11, 2023
@github-actions
Copy link
Contributor

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. stale worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants