-
Notifications
You must be signed in to change notification settings - Fork 29k
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
src: enable more detailed memory tracking #21742
Conversation
src/memory_tracker-inl.h
Outdated
|
||
void MemoryTracker::Track(const MemoryRetainer* value) { | ||
v8::HandleScope handle_scope(isolate_); | ||
value->MemoryInfo(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just thinking... some trace event output in here might be interesting from a diagnostics point of view... not necessarily in this PR, but at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably do that but I’m not sure what to track here – this is static information, not something event-related? We could maybe track the heap dump in progress, but that is already a diagnostic tool by itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll play around with it to see if we can get some interesting data out of it after this lands :-)
82c79fe
to
5641d0e
Compare
This will enable more detailed heap snapshots based on a newer V8 API. This commit itself is not tied to that API and could be backported.
Trying something else to hopefully make MSVC happy: https://ci.nodejs.org/job/node-test-pull-request/15844/ |
Windows re-run, this time it looks like it’s just a flake: https://ci.nodejs.org/job/node-test-commit-windows-fanned/19268/ |
Nice, Windows re-run came back good. Will land this soon if there are no objections. |
Landed in 57e3015 |
This will enable more detailed heap snapshots based on a newer V8 API. This commit itself is not tied to that API and could be backported. PR-URL: #21742 Reviewed-By: James M Snell <jasnell@gmail.com>
This will enable more detailed heap snapshots based on a newer V8 API. This commit itself is not tied to that API and could be backported. PR-URL: #21742 Reviewed-By: James M Snell <jasnell@gmail.com>
Splitting out from #21741:
This will enable more detailed heap snapshots based on
a newer V8 API.
This commit itself is not tied to that API and could
be backported.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes