[webkit-reviews] review granted: [Bug 86568] Add instrumentation to Vector to track used capacity, and dump useful info : [Attachment 142141] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 86568: Add instrumentation to Vector to track used capacity, and dump
useful info
https://bugs.webkit.org/show_bug.cgi?id=86568

Attachment 142141: Patch
https://bugs.webkit.org/attachment.cgi?id=142141&action=review

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=142141&action=review


The whole patch should probably have been landed long ago; it probably doesn't
impact non-Mac platforms at all. We're there particular cases where these
statistics were helpful in debugging a problem that you can mention?

R=me.

> Source/WTF/wtf/Vector.cpp:56
> +    pthread_once(&initializeOnceKey, initializeOnce);

Should we have a FIXME here for other platforms?

> Source/WTF/wtf/Vector.cpp:125
> +    VectorHash::iterator it = vectorData.find(vector);

It might be clearer to handle the fail/log case as an early return here.

> Source/WTF/wtf/Vector.cpp:142
> +    long long m_totalSizeBytes; 

Would it be better to use something like int64_t? Actually, does it make sense
to allow these to be signed? Maybe uint64_t would be better?