Conversation
nodejs-github-bot
added
c++
IlyasShabi
force-pushed
the
ishabi/v8_heap_profile
branch
from
44720ea to
0221ebd
Compare
IlyasShabi
force-pushed
the
ishabi/v8_heap_profile
branch
from
0221ebd to
5861bc1
Compare
|
cc @nodejs/diagnostics |
| 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
| return v8::Just(mode); | ||
| } | ||
|
|
||
| static void buildHeapProfileNode(Isolate* isolate, |
There was a problem hiding this comment.
| static void buildHeapProfileNode(Isolate* isolate, | |
| static void BuildHeapProfileNode(Isolate* isolate, |
| writer.json_arrayend(); | ||
|
|
||
| writer.json_objectstart("head"); | ||
| buildHeapProfileNode(isolate, profile->GetRootNode(), &writer); |
There was a problem hiding this comment.
| buildHeapProfileNode(isolate, profile->GetRootNode(), &writer); | |
| BuildHeapProfileNode(isolate, profile->GetRootNode(), &writer); |
Qard
left a comment
Qard
left a comment
There was a problem hiding this comment.
Some DX and future-looking recommendations, but otherwise LGTM.
| 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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62273 +/- ##
==========================================
+ Coverage 89.67% 89.70% +0.02%
==========================================
Files 676 677 +1
Lines 206555 206734 +179
Branches 39554 39571 +17
==========================================
+ Hits 185230 185449 +219
+ Misses 13461 13424 -37
+ Partials 7864 7861 -3
🚀 New features to boost your workflow:
|
5 participants
This PR succeeds #60231 by @theanarkh and adds parameter support for heap sampling on both the main thread and workers.