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.

Sign up for GitHub

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

Jump to bottom

Implement memory tracking for handful of classes #1625

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Implement memory tracking for handful of classes #1625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/workerd/api/cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ class Cache: public jsg::Object {
// Use RequestInfo type alias to allow `URL`s as cache keys
}

void visitForMemoryInfo(jsg::MemoryTracker& tracker) const {
tracker.trackField("cacheName", cacheName);
}

private:
kj::Maybe<kj::String> cacheName;

Expand Down Expand Up @@ -109,6 +113,10 @@ class CacheStorage: public jsg::Object {
JSG_READONLY_INSTANCE_PROPERTY(default, getDefault);
}

void visitForMemoryInfo(jsg::MemoryTracker& tracker) const {
tracker.trackField("default", default_);
}

private:
jsg::Ref<Cache> default_;
};
Expand Down
13 changes: 13 additions & 0 deletions src/workerd/api/cf-property.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ class CfProperty {

void visitForGc(jsg::GcVisitor& visitor);

JSG_MEMORY_INFO(CfProperty) {
KJ_IF_SOME(v, value) {
KJ_SWITCH_ONEOF(v) {
KJ_CASE_ONEOF(str, kj::String) {
tracker.trackField("value", str);
}
KJ_CASE_ONEOF(obj, jsg::JsRef<jsg::JsObject>) {
tracker.trackField("value", obj);
}
}
}
}

private:
kj::Maybe<kj::OneOf<kj::String, jsg::JsRef<jsg::JsObject>>> value;
};
Expand Down
20 changes: 20 additions & 0 deletions src/workerd/api/form-data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ class FormData: public jsg::Object {
void visitForGc(jsg::GcVisitor& visitor) {
visitor.visit(parent);
}

JSG_MEMORY_INFO(IteratorState) {
tracker.trackField("parent", parent);
}
};

public:
Expand All @@ -51,6 +55,18 @@ class FormData: public jsg::Object {
struct Entry {
kj::String name;
kj::OneOf<jsg::Ref<File>, kj::String> value;

JSG_MEMORY_INFO(Entry) {
tracker.trackField("name", name);
KJ_SWITCH_ONEOF(value) {
KJ_CASE_ONEOF(file, jsg::Ref<File>) {
tracker.trackField("value", file);
}
KJ_CASE_ONEOF(str, kj::String) {
tracker.trackField("value", str);
}
}
}
};

kj::ArrayPtr<const Entry> getData() { return data; }
Expand Down Expand Up @@ -142,6 +158,10 @@ class FormData: public jsg::Object {
}
}

void visitForMemoryInfo(jsg::MemoryTracker& tracker) const {
tracker.trackField("data", data.asPtr());
}

private:
kj::Vector<Entry> data;

Expand Down
8 changes: 8 additions & 0 deletions src/workerd/api/hyperdrive.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ class Hyperdrive : public jsg::Object {
JSG_METHOD(connect);
}

void visitForMemoryInfo(jsg::MemoryTracker& tracker) const {
tracker.trackField("randomHost", randomHost);
tracker.trackField("database", database);
tracker.trackField("user", user);
tracker.trackField("password", password);
tracker.trackField("scheme", scheme);
}

private:
uint clientIndex;
kj::String randomHost;
Expand Down
9 changes: 9 additions & 0 deletions src/workerd/api/kv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ class KvNamespace: public jsg::Object {
struct AdditionalHeader {
kj::String name;
kj::String value;

JSG_MEMORY_INFO(AdditionalHeader) {
tracker.trackField("name", name);
tracker.trackField("value", value);
}
};

// `subrequestChannel` is what to pass to IoContext::getHttpClient() to get an HttpClient
Expand Down Expand Up @@ -155,6 +160,10 @@ class KvNamespace: public jsg::Object {
});
}

void visitForMemoryInfo(jsg::MemoryTracker& tracker) const {
tracker.trackField("additionalHeaders", additionalHeaders.asPtr());
}

protected:
// Do the boilerplate work of constructing an HTTP client to KV. Setting a KvOptType causes
// the limiter for that op type to be checked. If a string is used, that's used as the operation
Expand Down
18 changes: 18 additions & 0 deletions src/workerd/api/queue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ class QueueMessage final: public jsg::Object {
});
}

void visitForMemoryInfo(jsg::MemoryTracker& tracker) const {
tracker.trackField("id", id);
tracker.trackField("body", body);
tracker.trackFieldWithSize("IoPtr<QueueEventResult>", sizeof(IoPtr<QueueEventResult>));
}

private:
kj::String id;
kj::Date timestamp;
Expand Down Expand Up @@ -182,6 +188,14 @@ class QueueEvent final: public ExtendableEvent {
});
}

void visitForMemoryInfo(jsg::MemoryTracker& tracker) const {
for (auto& message: messages) {
tracker.trackField("message", message);
}
tracker.trackField("queueName", queueName);
tracker.trackFieldWithSize("IoPtr<QueueEventResult>", sizeof(IoPtr<QueueEventResult>));
}

private:
// TODO(perf): Should we store these in a v8 array directly rather than this intermediate kj
// array to avoid one intermediate copy?
Expand Down Expand Up @@ -218,6 +232,10 @@ class QueueController final: public jsg::Object {
});
}

void visitForMemoryInfo(jsg::MemoryTracker& tracker) const {
tracker.trackField("event", event);
}

private:
jsg::Ref<QueueEvent> event;

Expand Down
4 changes: 2 additions & 2 deletions src/workerd/jsg/memory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ void MemoryTracker::trackField(
decCurrentNodeSize(sizeof(T));
}
pushNode(nodeName.orDefault(edgeName), sizeof(T), edgeName);
for (const auto item : value) {
for (const auto& item : value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this change do? Originally were we copying the value and now we don't? So:

  • The original code trigger a compile error if T doesn't have a compile error?
  • If T was a large value before, we would copy each entry just to calculate it's size?

Copy link
Member Author

Choose a reason for hiding this comment

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

Original was a bug. This template previously wasn't being used yet so it hadn't been caught.

trackField(nullptr, item, elementName);
}
nodeStack_.pop();
Expand All @@ -554,7 +554,7 @@ void MemoryTracker::trackField(
decCurrentNodeSize(sizeof(T));
}
pushNode(nodeName.orDefault(edgeName), sizeof(T), edgeName);
for (const auto item : value) {
for (const auto& item : value) {
trackField(nullptr, item, elementName);
}
nodeStack_.pop();
Expand Down
Loading