Skip to content
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

Improvements to CPU profiler #73

Closed

Conversation

OleksandrChekhovskyi
Copy link
Contributor

Expose more useful data from node.js

@OleksandrChekhovskyi
Copy link
Contributor Author

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

@3y3
Copy link
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
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
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