FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: enable more detailed memory tracking by addaleax · Pull Request #21742 · nodejs/node · GitHub

/ node Public

src: enable more detailed memory tracking - #21742

Closed
addaleax wants to merge 3 commits into
nodejs:masterfrom
addaleax:heap-bp
ConversationCommits (3)ChecksFiles changed
Closed

src: enable more detailed memory tracking#21742
addaleax wants to merge 3 commits into
nodejs:masterfrom
addaleax:heap-bp

Conversation

addaleax commented Jul 10, 2018

Copy link
Copy Markdown
Member

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), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

addaleax added memory Issues and PRs related to the memory management or memory footprint. backport-requested-v8.x labels Jul 10, 2018

nodejs-github-bot commented Jul 10, 2018

Copy link
Copy Markdown
Collaborator

@addaleax build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/246/pipeline

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jul 10, 2018
addaleax mentioned this pull request Jul 10, 2018
Closed
jasnell reviewed Jul 10, 2018
View reviewed changes
Comment thread src/memory_tracker-inl.h Outdated

jasnell Jul 10, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

addaleax Jul 10, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

jasnell Jul 10, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'll play around with it to see if we can get some interesting data out of it after this lands :-)

addaleax force-pushed the heap-bp branch 2 times, most recently from 82c79fe to 5641d0e Compare July 11, 2018 16:23

jasnell commented Jul 12, 2018

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/15827/

addaleax added 3 commits July 13, 2018 14:40
src: enable more detailed memory tracking
7cddb03 
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.
fixup! src: enable more detailed memory tracking
736085f
fixup! fixup! src: enable more detailed memory tracking
00fd890

addaleax commented Jul 13, 2018

Copy link
Copy Markdown
Member Author

Trying something else to hopefully make MSVC happy: https://ci.nodejs.org/job/node-test-pull-request/15844/

addaleax commented Jul 13, 2018

Copy link
Copy Markdown
Member Author

Windows re-run, this time it looks like it’s just a flake: https://ci.nodejs.org/job/node-test-commit-windows-fanned/19268/

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 13, 2018

addaleax commented Jul 13, 2018

Copy link
Copy Markdown
Member Author

Nice, Windows re-run came back good. Will land this soon if there are no objections.

addaleax commented Jul 13, 2018

Copy link
Copy Markdown
Member Author

Landed in 57e3015

addaleax closed this Jul 13, 2018
addaleax added a commit that referenced this pull request Jul 13, 2018
src: enable more detailed memory tracking
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>
addaleax deleted the heap-bp branch July 13, 2018 18:02
targos pushed a commit that referenced this pull request Jul 14, 2018
src: enable more detailed memory tracking
712809e 
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>
targos mentioned this pull request Jul 17, 2018
Merged
This was referenced Jul 18, 2018
Closed
Closed
Closed
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. memory Issues and PRs related to the memory management or memory footprint.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL