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

Implement MemoryTracker for Heap Snapshots #1584

Merged
merged 1 commit into from
Feb 1, 2024
Merged

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jan 29, 2024

This is adapted from Node.js' implementation.

This extends workerd's v8::HeapSnapshot collection process to allow including more detailed information about internal/native objects. Best illustrated with a screenshot...
image

This PR includes the core part of the implementation. It provides a straightforward implementation of jsg::Objects / jsg::Wrappable snapshot tracing along with a handful of other types. It does *NOT add all of the instrumentation to all of the various objects.

Those familiar with the GC tracing mechanism should find this familiar.

Look at the memory-test.c++ for an example of how it's used.

To keep things as simple as possible, to instrument your typical jsg::Object type (anything using JSG_RESOURCE_TYPE, you only need to provide an implementation of the visitForMemoryInfo(...) method:

struct Foo: public Object {
  kj::String bar = kj::str("test");

  JSG_RESOURCE_TYPE(Foo) {}
  
void visitForMemoryInfo(MemoryTracker& tracker) const {
    tracker.trackField("bar", bar);
  }
};

This should be familiar to anyone who has implemented support for GC tracing (using the visitForGc(...) method).

Any fields you want to include in the heap snapshot are registered using trackField(...) or trackFieldWithSize(...).

If you want anything else to be trackable, then it will need to implement all three of the following methods:

kj::StringPtr getMemoryInfoName() const { return "TheName"_kjc; }
size_t getMemoryInfoSelfSize() const { return sizeof(ThisThing); }
void getMemoryInfo(MemoryTracker& tracker) const {
   // register the fields here
}

To check to see if a type if trackable, you can test it using something like:

static constexpr bool IS_TESTABLE = MemoryRetainer<T>;

From the typical day-to-day use there wouldn't be much more than that. There a few more advanced cases to consider and we might want to expand the trackField(...) implementations in memory.h to add support for various types, but for the most part this was designed to be as minimally intrusive as possible.

@jasnell jasnell force-pushed the jsnell/jsg-memorytracker branch 4 times, most recently from 57aab80 to a28cc8e Compare January 29, 2024 18:10
@jasnell jasnell marked this pull request as ready for review January 29, 2024 18:10
@jasnell jasnell requested review from a team as code owners January 29, 2024 18:10
@jasnell jasnell force-pushed the jsnell/jsg-memorytracker branch from a28cc8e to c930f23 Compare January 29, 2024 18:33
Copy link
Collaborator

@dom96 dom96 left a comment

Choose a reason for hiding this comment

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

This is super cool!

One concern: what overhead does this add? Is it something that we disable by default so it shouldn't affect our performance upstream?

src/workerd/jsg/iterator.h Outdated Show resolved Hide resolved
src/workerd/jsg/jsg.h Outdated Show resolved Hide resolved
src/workerd/jsg/memory.h Show resolved Hide resolved
src/workerd/jsg/memory.h Show resolved Hide resolved
src/workerd/jsg/memory.h Show resolved Hide resolved
@jasnell
Copy link
Member Author

jasnell commented Jan 30, 2024

One concern: what overhead does this add? Is it something that we disable by default so it shouldn't affect our performance upstream?

This code is only engaged when a heap snapshot is taken so the overhead will be minimal.

@jasnell jasnell force-pushed the jsnell/jsg-memorytracker branch from 4663bab to fcc6e23 Compare February 1, 2024 00:50
@jasnell jasnell merged commit fffff79 into main Feb 1, 2024
11 checks passed
@jasnell jasnell deleted the jsnell/jsg-memorytracker branch February 1, 2024 11:17
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