Bug 104630 – Memory instrumentation: make sure each edge is reported only once

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Yury Semikhatsky]

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.

[Yury Semikhatsky]

Created attachment 178713 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #1)
> Created an attachment (id=178713) [details]
> Patch

Exported symbol lists probably need to be updated to make linker happy.

[Ilya Tikhonovsky]

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

[Build Bot]

Comment on attachment 178713 [details]
Patch

Attachment 178713 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15257492

[Yury Semikhatsky]

(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.

[Yury Semikhatsky]

Created attachment 178722 [details]
Patch

[Build Bot]

Comment on attachment 178722 [details]
Patch

Attachment 178722 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15272122

[Yury Semikhatsky]

Committed r137261: <http://trac.webkit.org/changeset/137261>