[webkit-reviews] review granted: [Bug 45134] Added statistics sampling and reporting for JavaScriptCore's RegisterFile and ExecutableAllocator classes : [Attachment 66982] Same patch as above with style error resolved (Revised patch w/ changes in Mutex use and function renaming)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 9 09:42:55 PDT 2010


Alexey Proskuryakov <ap at webkit.org> has granted John Therrell
<jtherrell at apple.com>'s request for review:
Bug 45134: Added statistics sampling and reporting for JavaScriptCore's
RegisterFile and ExecutableAllocator classes
https://bugs.webkit.org/show_bug.cgi?id=45134

Attachment 66982: Same patch as above with style error resolved (Revised patch
w/ changes in Mutex use and function renaming)
https://bugs.webkit.org/attachment.cgi?id=66982&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> +	   void addToCommittedByteCount(size_t byteCount);

The argument name is not needed here, it doesn't add any information. I'm not
sure if this is going to build with all compilers - you pass intptr_t to this
function, so there is a suspicious signed to unsigned conversion. It's also
confusing that this function takes a size_t, but is frequently called with
negative values. I think that the argument can be a long.

> +static FixedVMPoolAllocator* allocator = 0; 

There is a stray space at the end of the line.

Looks good to me.


More information about the webkit-reviews mailing list