doc: fix the example implementation of MemoryRetainer - #26262
Conversation
Sorry, something went wrong.
We need to be careful not to include the size of non-pointer fields in the parent's self size if we want to track them separately as a different node. Refs: https://github.com/nodejs/node/pull/26161/files#r259170771
|
@joyeecheung sadly an error occured when I tried to trigger a build :( |
Sorry, something went wrong.
|
CI: https://ci.nodejs.org/job/node-test-pull-request/20962/ CI is locked |
Sorry, something went wrong.
|
@addaleax #26161 probably does not eliminate the need for careful calculations, for example it does not cover the all types specialized by TrackField so for example when there is a uv_async_t member the user wants to track separately they still need to take care of this. They also need to be careful to subtract size of the fields they track with TrackFieldWithSize(), which may be more complicated than just subtracting sizeof(FieldType) depending on how much memory they want to shift into the child node. Maybe it would help if we add a enum/boolean to the TrackField implementations notifying the tracker to subtract sizeof(FieldType) automatically but I am not sure how complicated the calculation would become when we need to call it recursively for STL containers and other composite types. |
Sorry, something went wrong.
| * 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); |
There was a problem hiding this comment.
How about sizeof(*this) and sizeof(non_pointer_retainer)?
Sorry, something went wrong.
There was a problem hiding this comment.
Ping @joyeecheung
Sorry, something went wrong.
There was a problem hiding this comment.
@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.
Sorry, something went wrong.
Sorry, something went wrong.
|
Landed in f3d6207 |
Sorry, something went wrong.
We need to be careful not to include the size of non-pointer fields in the parent's self size if we want to track them separately as a different node. Refs: https://github.com/nodejs/node/pull/26161/files#r259170771 PR-URL: #26262 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
We need to be careful not to include the size of non-pointer fields in the parent's self size if we want to track them separately as a different node. Refs: https://github.com/nodejs/node/pull/26161/files#r259170771 PR-URL: nodejs#26262 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
We need to be careful not to include the size of non-pointer fields in the parent's self size if we want to track them separately as a different node. Refs: https://github.com/nodejs/node/pull/26161/files#r259170771 PR-URL: #26262 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
We need to be careful not to include the size of non-pointer fields in the parent's self size if we want to track them separately as a different node. Refs: https://github.com/nodejs/node/pull/26161/files#r259170771 PR-URL: #26262 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
We need to be careful not to include the size of non-pointer
fields in the parent's self size if we want to track them separately
as a different node.
Refs: https://github.com/nodejs/node/pull/26161/files#r259170771
Checklist