[Webkit-unassigned] [Bug 66849] The GC does not have a facility for profiling the kinds of objects that occupy the heap

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=66849


Geoffrey Garen <ggaren at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #104980|review?                     |review-
               Flag|                            |




--- Comment #2 from Geoffrey Garen <ggaren at apple.com>  2011-08-24 12:15:04 PST ---
(From update of attachment 104980)
View in context: https://bugs.webkit.org/attachment.cgi?id=104980&action=review

As you say, needs more build foo, plus perf testing. Review comments below.

> Source/JavaScriptCore/heap/VTableSpectrum.cpp:56
> +    HashMap<void*, uintptr_t>::iterator iter = m_map.find(vTablePointer);
> +    if (iter == m_map.end())
> +        m_map.add(vTablePointer, 1);
> +    else
> +        iter->second++;

Our idiom for this kind of operation is:

pair<HashMap<void*, uintptr_t>::iterator, bool> result = m_map.add(vTablePointer, 1);
if (!result.second) // pre-existing item in the table
    ++result.first->second;

This avoids a double hash lookup in the case of the first entry in the table. (This isn't performance-critical code, but it's good to use the right idioms everywhere.)

> Source/JavaScriptCore/heap/VTableSpectrum.cpp:66
> +    uintptr_t count;

uintptr_t is only needed if you're going to cast between unsigned and a pointer type. I think this should just be unsigned long, since that's how count is used, and then you can remove the cast from printf.

> Source/JavaScriptCore/runtime/JSCell.h:218
> +        visitor.noticeAnthracite(this);
>          visitor.append(&m_structure);

Slightly better to put this operation inside SlotVisitor::visitChildren. That way, when we optimize to skip calling visitChildren on some cells, this will still work. Also, you can then remove the noticeAnthracite API altogether, which is nice, since I looked up "anthracite" and got "a hard, compact variety of mineral coal that has a high luster". :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.