Conversation
44720ea to
0221ebd
Compare
0221ebd to
5861bc1
Compare
|
cc @nodejs/diagnostics |
doc/api/v8.md
Outdated
| console.log(profile); | ||
| ``` | ||
|
|
||
| ## `v8.heapProfilerConstants` |
There was a problem hiding this comment.
We very rarely expose bitfields and instead generally just let users pass us e.g. arrays of flags or options that we then translate in the JS wrapper. I could see exceptions made for highly performance-sensitive APIs, but I don't think this is an example of that
There was a problem hiding this comment.
I was looking for a proper way to do this while preserving the same naming as V8.
Fixed
Qard
left a comment
There was a problem hiding this comment.
Some DX and future-looking recommendations, but otherwise LGTM.
doc/api/v8.md
Outdated
| These constants can be combined using bitwise OR to pass as the `flags` | ||
| parameter. | ||
|
|
||
| ## `v8.startHeapProfile([sampleInterval[, stackDepth[, flags]]])` |
There was a problem hiding this comment.
It might be better to take an object given that there's no particular dependency between any of these parameters--one could want to set any of them independently of the others.
| added: REPLACEME | ||
| --> | ||
|
|
||
| * Returns: {string} |
There was a problem hiding this comment.
Might be better to return a Buffer instead. It'd give us more flexibility to add binary formats in the future. I'd like to add support at some point in the future for the pprof-derived format being defined in OpenTelemetry at the moment, which is a binary format.
There was a problem hiding this comment.
IIRC worker.stopHeapProfile() already returns a JSON since v22, so changing the return type to Buffer would be a breaking change. For now this PR keeps the main thread consistent with the worker.
We could add a format option { format: 'json' | 'pprof' } in a follow up PR when binary format support is needed. wdyt?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62273 +/- ##
==========================================
- Coverage 89.67% 88.77% -0.91%
==========================================
Files 676 677 +1
Lines 206555 206737 +182
Branches 39554 39482 -72
==========================================
- Hits 185230 183522 -1708
- Misses 13461 15279 +1818
- Partials 7864 7936 +72
🚀 New features to boost your workflow:
|
This PR succeeds #60231 by @theanarkh and adds parameter support for heap sampling on both the main thread and workers.