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

docs: add heap profiling tutorial #504

Merged
merged 4 commits into from
Oct 19, 2021
Merged

docs: add heap profiling tutorial #504

merged 4 commits into from
Oct 19, 2021

Conversation

RafaelGSS
Copy link
Member

@RafaelGSS RafaelGSS commented Oct 11, 2021

Address #502

@RafaelGSS RafaelGSS marked this pull request as ready for review October 11, 2021 20:45
@mhdawson
Copy link
Member

@RafaelGSS thanks for working on this>. I think we should also cover generating a heap dump with https://nodejs.org/api/v8.html#v8_v8_getheapsnapshot and the related methods.

@RafaelGSS
Copy link
Member Author

@mhdawson I think that it was covered in the step2

@mhdawson
Copy link
Member

@RafaelGSS thanks for clarifying. I think we could use an intro under the top level heading to explain what this doc is covering and introduce the top options that will be covered.

@RafaelGSS
Copy link
Member Author

@mhdawson Just added an introduction, let me know if that's what you expect.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@mmarchini mmarchini left a comment

Choose a reason for hiding this comment

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

LGTM. Might be worth pointing out that both tools can also be used in production with external dependencies (heap-profile, @mmarchini/observe).

@RafaelGSS RafaelGSS merged commit e79ceaf into main Oct 19, 2021
@RafaelGSS RafaelGSS deleted the docs-heap-profiling branch October 19, 2021 12:22
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.

4 participants