Bug 197344 - Web Inspector: "Retained Size" does not make sense
Summary: Web Inspector: "Retained Size" does not make sense
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: Safari 12
Hardware: Mac macOS 10.13
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-27 05:36 PDT by philipp.kuederli
Modified: 2019-07-10 09:09 PDT (History)
4 users (show)

See Also:


Attachments
Retained Size Example (80.34 KB, image/png)
2019-04-27 05:36 PDT, philipp.kuederli
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description philipp.kuederli 2019-04-27 05:36:47 PDT
Created attachment 368400 [details]
Retained Size Example

According to the "Retained Size" (in German "Grösse beibehalten") my size is 781.92 GB (see attachment). This is more than my memory and my hard disk together. 

When I hover the "Retained Size" column header it says that it is the size of the current object plus all objects which depend on this object. But it cannot be (at least in my eyes) that I even could have so much memory.
Comment 1 Joseph Pecoraro 2019-04-30 15:49:53 PDT
The top level description is adding up the sum of all the direct children's retained size. It is not particularly valuable, but this isn't necessarily wrong. This can happen if a change of objects each holds onto the next.

Here is how it can happen. Lets say you have an object graph which contains objects like so:

  window
    |
    A (1MB)
    |
    --- B (1MB)
        |
        --- C (1MB)

Which would be like:

   window.A = {};
   window.A.B = {};
   window.A.B.C = {};
   growObject(window.A);
   growObject(window.B);
   growObject(window.C);
   function growObject(o) {
      o.__foo = "abcdefghijklmn...."; // Something large to make the object large
   }

Now the UI would effectively show something like:

    Retained  Self  Instances
    --------  ----  -----------
         6MB         ▼ Object
         3MB   1MB      Object (@1) - "window.A"
         2MB   1MB      Object (@2) - "window.A.B"
         1MB   1MB      Object (@3) - "window.A.B.C"

In this case:
• Individually the self sizes accurately reflect that A/B/C are each 1MB
• Individually the retained sizes accurately reflect that C retains 1MB, B retains 2MB, and A retains 3MB
• The sum of all the retained size (the top line) reflects (3 + 2 + 1) = 6MB even though there is only 3MB worth of objects on the page.

Maybe we could eliminate or change the meaning of retained size at the top level... but it does end up being useful. For example, in the case where a Map holds onto a few objects with a large retained size, the sum of retained size bubbles "Map" up to the top instead of the individual Objects. The "retained size" is duplicated across all objects, but it is just a weight that shows the heaviest objects.

I suspect this is behaving correctly.
Comment 2 Joseph Pecoraro 2019-04-30 15:52:00 PDT
To aid in investigation, you can export a heap snapshot. That would produce a JSON file you could attach to this bug that would allow this case to be investigated. Alternatively pointing to a web page that we can reproduce the issue would make it actionable.
Comment 3 philipp.kuederli 2019-05-01 02:22:22 PDT
Thanks for the hints. I use a custom implemented React-similar way of state handling for a WebGL application where I found out thanks to the "retained size" that there is indeed a chain of last states (current -> last -> last -> last ...) caused by closures. Unfortunately I cannot extract a simple example right now and there is no public link available yet. 

So on the one hand this tool helps to find potential non-obvious memory leaks caused for example by closures. However on the other hand since this accumulated sum showed in my case 781.92 GB I almost ignored it because I thought its just a Safari developer tools bug. 

From https://webkit.org/blog/6425/memory-debugging-with-web-inspector/ I found 
"The retained size is the size of the object plus the size of all of the nodes it dominates (the objects that this particular object solely keeps alive). An easy way to think about the retained size is if the object were to be deleted right now, the retained size would be the amount of memory that would be reclaimed."

According to this explanation it would mean it really should show a concrete amount of memory allocated at the moment. The accumulated sum however at the top level in Safari developer tools shows some "virtual" calculation which does not represent real memory like you already mentioned too. 

The question now is if the top level would only show effective 3 MB from your example instead of 6 MB would I have found the memory leak too? I guess yes however after a longer runtime (here also I would need to make an example with concrete calculation, but I don't know if this is needed). 

To summarize at the moment it seems to make no sense to investigate more since there seems to be no bugs neither in the safari tools nor in my application (after removing the leak). However I would really think about changing the measurement to have an effective memory footprint instead of a accumulated sum to prevent confusion. I had a very bad feeling about what should I trust now? Or I guess at least there should somewhere be an explanation how it is calculated. When a developer starts to profile it means he does not understand 100% anymore what is going on in his code so he has to be sure that at least the profiling tools he understands perfectly otherwise he has even more unknowns.
Comment 4 Joseph Pecoraro 2019-05-01 12:01:46 PDT
> Thanks for the hints. I use a custom implemented React-similar way of state
> handling for a WebGL application where I found out thanks to the "retained
> size" that there is indeed a chain of last states (current -> last -> last
> -> last ...) caused by closures. Unfortunately I cannot extract a simple
> example right now and there is no public link available yet. 

Understood. Building a long chain is all it takes though. You can increase the value of "N" here to easily get over a Terrabyte (N = 1500 for example) in the current implementation. For now this test matches the A.B.C example I listed above:

<script>
setTimeout(() => {
    console.takeHeapSnapshot("before");

    function largeObject() {
        return {__mem: "x".repeat(1024*1024)};
    }

    window.chain = largeObject();
    let last = window.chain;
    let N = 2;
    for (let i = 0; i < N; ++i) {
        last.X = largeObject();
        last = last.X;
    }

    console.takeHeapSnapshot("after");
});
</script>


> So on the one hand this tool helps to find potential non-obvious memory
> leaks caused for example by closures. However on the other hand since this
> accumulated sum showed in my case 781.92 GB I almost ignored it because I
> thought its just a Safari developer tools bug. 
> 
> From https://webkit.org/blog/6425/memory-debugging-with-web-inspector/ I
> found 
> "The retained size is the size of the object plus the size of all of the
> nodes it dominates (the objects that this particular object solely keeps
> alive). An easy way to think about the retained size is if the object were
> to be deleted right now, the retained size would be the amount of memory
> that would be reclaimed."
> 
> According to this explanation it would mean it really should show a concrete
> amount of memory allocated at the moment. The accumulated sum however at the
> top level in Safari developer tools shows some "virtual" calculation which
> does not represent real memory like you already mentioned too.

Yes. And it is concerning that seeing a large number deter'd you temporarily, so we should address that by putting a better number at the top level.

> The question now is if the top level would only show effective 3 MB from
> your example instead of 6 MB would I have found the memory leak too? I guess
> yes however after a longer runtime (here also I would need to make an
> example with concrete calculation, but I don't know if this is needed). 

No matter what we change the top level category number to, I don't think we'd change what happens when you expand the category. So each object would list its individual retained size (and in a chain you'd continue to see large values but nothing as meaningless sum).

> To summarize at the moment it seems to make no sense to investigate more
> since there seems to be no bugs neither in the safari tools nor in my
> application (after removing the leak). However I would really think about
> changing the measurement to have an effective memory footprint instead of a
> accumulated sum to prevent confusion. I had a very bad feeling about what
> should I trust now? Or I guess at least there should somewhere be an
> explanation how it is calculated. When a developer starts to profile it
> means he does not understand 100% anymore what is going on in his code so he
> has to be sure that at least the profiling tools he understands perfectly
> otherwise he has even more unknowns.

We should just put a better number at the top level. Devin had an idea to use some kind of computed value that is the retained size of that class without double counting. I'm not sure precisely how to count that but if we can manage that it would be a better UI. In the example above you'd end up with roughly `(N+1)MB` at the top level as you'd expect.
Comment 5 Radar WebKit Bug Importer 2019-07-10 09:09:18 PDT
<rdar://problem/52894776>