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

doc: fix the example implementation of MemoryRetainer #26262

Closed
wants to merge 1 commit into from
Closed

doc: fix the example implementation of MemoryRetainer #26262

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
18 changes: 15 additions & 3 deletions src/memory_tracker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ class NodeBIO;
* // Node name and size comes from the MemoryInfoName and SelfSize of
* // AnotherRetainerClass
* tracker->TrackField("another_retainer", another_retainer_);
*
* // Add non_pointer_retainer as a separate node into the graph
* // and track its memory information recursively.
* // Note that we need to make sure its size is not accounted in
* // ExampleRetainer::SelfSize().
* tracker->TrackField("non_pointer_retainer", &non_pointer_retainer_);
*
* // Specify node name and size explicitly
* tracker->TrackFieldWithSize("internal_member",
* internal_member_.size(),
Expand All @@ -60,9 +67,12 @@ class NodeBIO;
* return "ExampleRetainer";
* }
*
* // Or use SET_SELF_SIZE(ExampleRetainer)
* // Classes that only want to return its sizeof() value can use the
* // SET_SELF_SIZE(Class) macro instead.
* size_t SelfSize() const override {
* return sizeof(ExampleRetainer);
* // We need to exclude the size of non_pointer_retainer so that
* // we can track it separately in ExampleRetainer::MemoryInfo().
* return sizeof(ExampleRetainer) - sizeof(NonPointerRetainerClass);
Copy link
Member

Choose a reason for hiding this comment

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

How about sizeof(*this) and sizeof(non_pointer_retainer)?

Copy link
Member

Choose a reason for hiding this comment

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

Ping @joyeecheung

Copy link
Member Author

@joyeecheung joyeecheung Mar 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@TimothyGu For the example itself there probably is no difference - whatever that makes the calculation easier to understand for the writer is good enough.

One thing to note: if non_pointer_retainer is of certain types, the equation may be more complicated than a - sizeof(non_pointer_retainer) if the self size of that node that needs to be split out of the parent is not exactly sizeof(non_pointer_retainer). So, keeping sizeof(NonPointerRetainerClass) makes that obvious, as in actual code the member name may be as vague as obj or val.

* }
*
* // Note: no need to implement these two methods when implementing
Expand All @@ -71,8 +81,10 @@ class NodeBIO;
* v8::Local<v8::Object> WrappedObject() const override {
* return node::PersistentToLocal::Default(wrapped_);
* }
*
* private:
* AnotherRetainerClass another_retainer_;
* AnotherRetainerClass* another_retainer_;
* NonPointerRetainerClass non_pointer_retainer;
* InternalClass internal_member_;
* std::vector<uv_async_t> vector_;
* node::Persistent<Object> target_;
Expand Down