add --track-heap-objects by bmeck · Pull Request #2135 · nodejs/node

bmeck · 2015-07-08T17:43:05Z

Right now we do not have a way to start iojs with heap object tracking.

Calling v8::Profiler::StartTrackingHeapObjects after node has started up will not include the reference info for objects in node core. This can be seen by running node-heapdump with the flag turned on. This information is useful for debugging tools and would be visible in Chrome DevTools in the same way that "Record heap allocation stack traces" functions from the DevTools settings pane.

ofrobots · 2015-07-08T18:58:12Z

Does doc/iojs.1 also need to be modified?

thefourtheye · 2015-07-08T19:03:19Z

I am not able to understand why we need the whitespace changes.

bmeck · 2015-07-08T19:05:45Z

@thefourtheye it was to keep 2 spaces between the switch and the description and keeping all of the descriptions aligned

bmeck · 2015-07-08T19:07:34Z

@ofrobots i can but I saw that --trace-sync-io was not in there either

thefourtheye · 2015-07-08T19:09:18Z

@bmeck Ah, I see what you are saying now. " --track-heap-objects track heap object allocations for heap snapshots\n", correct? Not a v8 expert, but the changes look straight-forward.

bmeck · 2015-07-08T19:11:37Z

@thefourtheye correct

thefourtheye · 2015-07-08T19:11:45Z

Shouldn't we explicitly set this to False, in the else part?

none of the other switches work that way

Does it even need to be exposed to JS land?
(I guess the same question applies to .traceSyncIO, /cc @trevnorris.)

Yup. You're right. I can't think of a useful case to expose either of them to JS.

Good to know. @bmeck Can you remove the .trackHeapObjects property (and .traceSyncIO in a separate commit if you want)?

bmeck · 2015-07-09T15:10:05Z

@bnoordhuis removed trackHeapObjects from process, separate PR for traceSyncIO #2143

bnoordhuis · 2015-07-09T16:07:54Z

LGTM

trevnorris · 2015-07-09T16:08:45Z

@bmeck Not adding that extra documentation was a failure on my part. If all other options are listed then this and the other should as well.

EDIT: comment was meant for @ofrobots

trevnorris · 2015-07-09T16:10:45Z

LGTM

thefourtheye · 2015-07-09T16:52:45Z

Do we need a test for this?

bmeck · 2015-07-09T16:55:09Z

@thefourtheye node itself does not include a way to dump heap snapshots, we could write one up but it would be akin to including node-heapdump in core.

- This makes v8 add .trace_function_info to the serialized form of snapshots from v8::HeapSnapshot::Serialize - .trace_funciton_info combined with .trace_node in snapshots tells the JS location that allocated a specific object

- This makes v8 add .trace_function_info to the serialized form of snapshots from v8::HeapSnapshot::Serialize - .trace_funciton_info combined with .trace_node in snapshots tells the JS location that allocated a specific object PR-URL: #2135 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Trevor Norris <trev.norris@gmail.com>

trevnorris · 2015-07-10T21:29:47Z

Squashed and landed in cf14a24.

Fishrock123 · 2015-07-12T00:13:07Z

Tagged as semver-minor, 2.4.0 ho! :)

Notable changes * src: Added a new `--track-heap-objects` flag to track heap object allocations for heap snapshots (Bradley Meck) nodejs#2135. * readline: Fixed a freeze that affected the repl if the keypress event handler threw (Alex Kocharin) nodejs#2107. * npm: Upgraded to v2.13.0, release notes can be found in https://github.com/npm/npm/releases/tag/v2.13.0 (Forrest L Norvell) nodejs#2152. PR-URL: nodejs#2189

Fishrock123 added c++ Issues and PRs that require attention from people who are familiar with C++. v8 engine Issues and PRs related to the V8 dependency. labels Jul 8, 2015

thefourtheye reviewed Jul 8, 2015
View reviewed changes

bmeck force-pushed the track-heap-objects branch from 3fb4779 to eaa89b7Compare July 9, 2015 16:24

bmeck added 3 commits July 10, 2015 16:06

add --track-heap-objects
9748d60
- This makes v8 add .trace_function_info to the serialized form of snapshots from v8::HeapSnapshot::Serialize - .trace_funciton_info combined with .trace_node in snapshots tells the JS location that allocated a specific object

style changes, move tracking to earlier start
8e2490c

-h space between heapsnapshot
bd77a59

bmeck force-pushed the track-heap-objects branch from eaa89b7 to bd77a59Compare July 10, 2015 21:24

trevnorris closed this Jul 10, 2015

Fishrock123 added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 12, 2015

Fishrock123 mentioned this pull request Jul 16, 2015
Release Proposal: v2.4.0 #2189
Merged

snyk-bot mentioned this pull request Apr 1, 2020
[Snyk] Upgrade prismjs from 1.17.1 to 1.19.0 O330oei/node#14
Open

Uh oh!

Uh oh!

Conversation

bmeck commented Jul 8, 2015

Uh oh!

ofrobots commented Jul 8, 2015

Uh oh!

thefourtheye commented Jul 8, 2015

Uh oh!

bmeck commented Jul 8, 2015

Uh oh!

bmeck commented Jul 8, 2015

Uh oh!

thefourtheye commented Jul 8, 2015

Uh oh!

bmeck commented Jul 8, 2015

Uh oh!

thefourtheyeJul 8, 2015

Choose a reason for hiding this comment

Uh oh!

bmeckJul 8, 2015

Choose a reason for hiding this comment

Uh oh!

thefourtheyeJul 8, 2015

Choose a reason for hiding this comment

Uh oh!

bnoordhuisJul 8, 2015

Choose a reason for hiding this comment

Uh oh!

trevnorrisJul 9, 2015

Choose a reason for hiding this comment

Uh oh!

bnoordhuisJul 9, 2015

Choose a reason for hiding this comment

Uh oh!

bmeck commented Jul 9, 2015

Uh oh!

bnoordhuis commented Jul 9, 2015

Uh oh!

trevnorris commented Jul 9, 2015

Uh oh!

trevnorris commented Jul 9, 2015

Uh oh!

thefourtheye commented Jul 9, 2015

Uh oh!

bmeck commented Jul 9, 2015

Uh oh!

trevnorris commented Jul 10, 2015

Uh oh!

Fishrock123 commented Jul 12, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants