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

Issue for Feedback on Diagnostic report #280

Closed
mhdawson opened this issue Mar 15, 2019 · 6 comments
Closed

Issue for Feedback on Diagnostic report #280

mhdawson opened this issue Mar 15, 2019 · 6 comments

Comments

@mhdawson
Copy link
Member

Since Diagnostic report has been recently added to core I think it is good to have an issue where people can post feedback on it's use. Opening this issue for that purpose. @gireeshpunathil will be putting out a blog post that talks about Diagnostic report and will mention this issue as one place to provide feedback.

@cjihrig
Copy link

cjihrig commented Mar 15, 2019

Can I recommend using nodejs/node#26293? The topics are related and it will be easier to keep all discussion in one place.

@mhdawson
Copy link
Member Author

@cjihrig to me they have different audiences. I do agree that feedback from people using the tool should inform #26293 but I'm worried that having that as the topic might be confusing for people that come to comment.

@bertyhell
Copy link

When the nodejs process encounters an "out of memory" exception, it would be nice if the report could still capture the javastack.

Maybe by reserving a small part of the memory specifically for the purpose of error reporting. This part of the memory would not be accessible for the regular application, and can only be used by the nodejs core error handling code.

But i can imagine the technical implementation of this would be quite hard.

@mhdawson
Copy link
Member Author

@bertyhell interestingly we had a discussion on this topic last week at the Diagnostic Report at NodeConfEU last week. One of the points made was that the stack trace when you run out of memory is not necessarily related to where memory is being use most heavily...

There is a sampling profiler and having it enabled at a level that is acceptable performance wise and including that in the diagnostic report might be more helpful.

We did also discuss exploring reserving memory as you mention, but as you say there will likely be some challenges with that. @gireeshpunathil FYI.

@gireeshpunathil
Copy link
Member

IIRC it was @bertyhell himself who raised this at NodeConfEU

cases where call stack is useless / irrelevant / misleading

  • the failing stack is just a victim, bad allocations happened in the past
  • a large allocation request seen in the stack, but it legitimate, can easily mislead a user

cases where call stack helps proper PD:

  • the heap is healthy otherwise, a huge allocation request comes and fails.
  • the heap is failing through a slow leak, and the leaking site === call stack

I will take it up; thanks!

@gireeshpunathil
Copy link
Member

the report is now stable in core, so closing. pls open feature requests if you have an improvement suggestion, or a bug report if there an issue.

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

No branches or pull requests

4 participants