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

support taking HeapSnapshots #298

Closed
bartlomieju opened this issue Mar 5, 2020 · 4 comments
Closed

support taking HeapSnapshots #298

bartlomieju opened this issue Mar 5, 2020 · 4 comments

Comments

@bartlomieju
Copy link
Member

To figure out performance problems with deno_tcp.ts (which are GC problems) we need to inspect what's going on on V8 heap.

This is partially related to #205 but can be implemented independently and used without full inspector support.

There's node-heapdump which implements that functionality fo node.

Grepping through node-heapdump/heapdump.cc it seems that APIs that we should port are:

  • Isolate::GetHeapProfiler
  • HeapProfiler::TakeHeapSnapshot
  • HeapSnapshot::Serialize

CC @bnoordhuis for expert advice on the subject

@bnoordhuis
Copy link
Contributor

I can write bindings, that shouldn't be too complicated, but the question is where to send the data chunks to.

node-heapdump writes them out to disk to avoid having to round-trip through JS land because it's easy to hit out-of-memory conditions that way.

@bartlomieju
Copy link
Member Author

I can write bindings, that shouldn't be too complicated, but the question is where to send the data chunks to.

node-heapdump writes them out to disk to avoid having to round-trip through JS land because it's easy to hit out-of-memory conditions that way.

That'd be awesome thanks! My direct need for this feature is to take some dumps of deno_tcp.ts heap; I haven't put much thought how to call that API, but I guess I could just directly bind it to global object (globalThis.takeHeapSnapshot()) and write out to disk - but that would be done in deno_core

@bnoordhuis
Copy link
Contributor

I'll open a PR here tomorrow.

bnoordhuis added a commit to bnoordhuis/rusty_v8 that referenced this issue Mar 8, 2020
This doesn't really follow the current V8 API (it's pretty close to how
V8 used to be back in 2012 though.) However:

1. The C++ API is very C++-y and doesn't carry over well to Rust, and
2. It addresses the immediate need of being able to take heap snapshots.

I can expand it into something that mimics the V8 API more closely later
but right now it's Sunday afternoon and I want to go bouldering. :-)

Refs denoland#298
piscisaureus pushed a commit that referenced this issue Mar 9, 2020
This doesn't really follow the current V8 API (it's pretty close to how
V8 used to be back in 2012 though.) However:

1. The C++ API is very C++-y and doesn't carry over well to Rust, and
2. It addresses the immediate need of being able to take heap snapshots.

Refs #298
@bartlomieju
Copy link
Member Author

Done in #302

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

2 participants