Bug 104630 - Memory instrumentation: make sure each edge is reported only once
Summary: Memory instrumentation: make sure each edge is reported only once
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-12-10 21:51 PST by Yury Semikhatsky
Modified: 2012-12-11 00:34 PST (History)
11 users (show)

See Also:


Attachments
Patch (16.80 KB, patch)
2012-12-10 21:56 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (16.74 KB, patch)
2012-12-10 22:41 PST, Yury Semikhatsky
pfeldman: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2012-12-10 21:51:58 PST
In case of class B : public A we may come to object B first by B* and later by A* in which case all outgoing links will be reported twice.
Comment 1 Yury Semikhatsky 2012-12-10 21:56:55 PST
Created attachment 178713 [details]
Patch
Comment 2 Yury Semikhatsky 2012-12-10 21:57:40 PST
(In reply to comment #1)
> Created an attachment (id=178713) [details]
> Patch

Exported symbol lists probably need to be updated to make linker happy.
Comment 3 Ilya Tikhonovsky 2012-12-10 22:18:03 PST
Comment on attachment 178713 [details]
Patch

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

> Source/WTF/wtf/MemoryInstrumentation.cpp:91
> +    callReportMemoryUsage(&memoryObjectInfo);
> +
> +    const void* realAddress = memoryObjectInfo.reportedPointer();

realAddress could be a return value for callReportMemoryUsage

> Source/WTF/wtf/MemoryInstrumentation.cpp:115
> +    if (!m_memoryObjectInfo->objectSize()) {
> +        const void* instrumentedPointer = m_memoryObjectInfo->reportedPointer();
> +        if (instrumentedPointer != objectAddress && instrumentedPointer && m_memoryInstrumentation->visited(objectAddress))
> +            m_memoryObjectInfo->setAlreadyVisited();
> +    }

I'd move this logic into MemoryObjectInfo::reportObjectInfo method
Comment 4 Build Bot 2012-12-10 22:30:49 PST
Comment on attachment 178713 [details]
Patch

Attachment 178713 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15257492
Comment 5 Yury Semikhatsky 2012-12-10 22:34:07 PST
(In reply to comment #3)
> (From update of attachment 178713 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178713&action=review
> 
> > Source/WTF/wtf/MemoryInstrumentation.cpp:91
> > +    callReportMemoryUsage(&memoryObjectInfo);
> > +
> > +    const void* realAddress = memoryObjectInfo.reportedPointer();
> 
> realAddress could be a return value for callReportMemoryUsage
> 
The current approach seems more clear. Returning real address from callReportMemoryUsage would require access to memoryObjectInfo.reportedPointer() and MemoryObjectInfo definition is not available there. Alternative approach would be to return real address from reportMemoryUsage free function but that would touch too many files and I don't think it is justified.


> > Source/WTF/wtf/MemoryInstrumentation.cpp:115
> > +    if (!m_memoryObjectInfo->objectSize()) {
> > +        const void* instrumentedPointer = m_memoryObjectInfo->reportedPointer();
> > +        if (instrumentedPointer != objectAddress && instrumentedPointer && m_memoryInstrumentation->visited(objectAddress))
> > +            m_memoryObjectInfo->setAlreadyVisited();
> > +    }
> 
> I'd move this logic into MemoryObjectInfo::reportObjectInfo method

Done.
Comment 6 Yury Semikhatsky 2012-12-10 22:41:20 PST
Created attachment 178722 [details]
Patch
Comment 7 Build Bot 2012-12-10 23:12:05 PST
Comment on attachment 178722 [details]
Patch

Attachment 178722 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15272122
Comment 8 Yury Semikhatsky 2012-12-11 00:34:02 PST
Committed r137261: <http://trac.webkit.org/changeset/137261>