Skip to content

Improvements to CPU profiler#73

Closed
OleksandrChekhovskyi wants to merge 2 commits intonode-inspector:masterfrom
OleksandrChekhovskyi:master
Closed

Improvements to CPU profiler#73
OleksandrChekhovskyi wants to merge 2 commits intonode-inspector:masterfrom
OleksandrChekhovskyi:master

Conversation

@OleksandrChekhovskyi
Copy link
Copy Markdown
Contributor

Expose more useful data from node.js

@OleksandrChekhovskyi
Copy link
Copy Markdown
Contributor Author

Some builds are failing because of errors downloading node.js releases, not because of problems with patches.

@3y3
Copy link
Copy Markdown
Member

3y3 commented Aug 30, 2015

@OleksandrChekhovskyi , thank you for contribution.
Please look at my inline comments.

Oleksandr Chekhovskyi added 2 commits August 31, 2015 08:52
Now idle time is marked as "(idle)" instead of just "(program)".
@OleksandrChekhovskyi
Copy link
Copy Markdown
Contributor Author

I have updated the first patch, because I've noticed a compiler warning in CI builds. Also see my replies to inline comments.

Edit: For some reason, GitHub does not show 'commented on outdated diff' in this PR. Did you comment not from PR? It looks like it just lost the comments after force push, unfortunately.
You will probably still see replies in the email, but the important part was that v8::CpuProfiler::StopProfiling stops the last active profile in case of name being an empty string. That's why there are checks for name being empty.

@3y3
Copy link
Copy Markdown
Member

3y3 commented Aug 31, 2015

Reasonable comments.
Landed as 510542c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants