Bug 99958 - Web Inspector: do not double count memory of objects with multiple ancestors
Summary: Web Inspector: do not double count memory of objects with multiple ancestors
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-21 23:27 PDT by Yury Semikhatsky
Modified: 2012-10-22 05:16 PDT (History)
11 users (show)

See Also:

Attachments
Patch (6.75 KB, patch)
2012-10-22 02:00 PDT, Yury Semikhatsky 		no flags Details | Formatted Diff | Diff
Patch (7.03 KB, patch)
2012-10-22 02:37 PDT, Yury Semikhatsky 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2012-10-21 23:27:24 PDT 
If there is an instrumented ancestor class which is not first in the parents' list and we have a pointer to the ancestor and another one to the descendant the ancestor's memory will be counted twice. To avoid this we should use pointer to the ancestor class when check if an object has already been visited.
Comment 1 Yury Semikhatsky 2012-10-22 02:00:09 PDT 
Created attachment 169854 [details]
Patch
Comment 2 Ilya Tikhonovsky 2012-10-22 02:19:26 PDT 
Comment on attachment 169854 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=169854&action=review

> Source/WTF/wtf/MemoryInstrumentation.h:144
> +        memoryObjectInfo->reportObjectInfo<T>(0, 0, sizeof(T));

we can report actual pointer here and use it latter.

> Source/WTF/wtf/MemoryInstrumentation.h:268
> +    if (pointer) {

unnecessary line

> Source/WTF/wtf/MemoryInstrumentation.h:272
> +    } else
> +        pointer = m_pointer;

ditto
Comment 3 Yury Semikhatsky 2012-10-22 02:37:55 PDT 
Created attachment 169861 [details]
Patch
Comment 4 Ilya Tikhonovsky 2012-10-22 02:39:26 PDT 
Comment on attachment 169861 [details]
Patch

lgtm
Comment 5 Yury Semikhatsky 2012-10-22 02:39:57 PDT 
(In reply to comment #2)
> (From update of attachment 169854 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169854&action=review
> 
> > Source/WTF/wtf/MemoryInstrumentation.h:144
> > +        memoryObjectInfo->reportObjectInfo<T>(0, 0, sizeof(T));
> 
> we can report actual pointer here and use it latter.
> 
Goof point. Done!

> > Source/WTF/wtf/MemoryInstrumentation.h:268
> > +    if (pointer) {
> 
> unnecessary line
> 
Done.

> > Source/WTF/wtf/MemoryInstrumentation.h:272
> > +    } else
> > +        pointer = m_pointer;
> 
> ditto
Done.
Comment 6 WebKit Review Bot 2012-10-22 03:29:12 PDT 
Comment on attachment 169861 [details]
Patch

Clearing flags on attachment: 169861

Committed r132059: <http://trac.webkit.org/changeset/132059>
Comment 7 WebKit Review Bot 2012-10-22 03:29:15 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Chris Dumez 2012-10-22 04:40:47 PDT 
This patch appears to have broken test_wtf API tests:
http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/4844/steps/API%20tests/logs/stdio

[ RUN      ] MemoryInstrumentationTest.instrumentedWithMultipleAncestors
1   0x59156f
2   0x2b2d921bf4c0
3   0x7fff776dcfa0
Comment 9 Yury Semikhatsky 2012-10-22 05:16:15 PDT 
(In reply to comment #8)
> This patch appears to have broken test_wtf API tests:
> http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/4844/steps/API%20tests/logs/stdio
> 
> [ RUN      ] MemoryInstrumentationTest.instrumentedWithMultipleAncestors
> 1   0x59156f
> 2   0x2b2d921bf4c0
> 3   0x7fff776dcfa0

Fix landed in http://trac.webkit.org/changeset/132065