[webkit-reviews] review granted: [Bug 45528] Move JavaScriptCore statistics gathering into JavaScriptCore : [Attachment 67166] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Stephanie Lewis
<slewis at apple.com>'s request for review:
Bug 45528: Move JavaScriptCore statistics gathering into JavaScriptCore
https://bugs.webkit.org/show_bug.cgi?id=45528

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

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
Oops!

> +#include "ExecutableAllocator.h"
> +#include "JSGlobalData.h"
> +#include "RegisterFile.h"
> +
> +

Extra blank line here.

> +namespace JSC {
> +
> +    MemoryStatistics memoryStatistics(JSGlobalData* commonGlobalData) 

This shouldn't be indented.

I think that it's nice to put heap and stack/JIT statistics under a common
umbrella. But I don't think that it's good to report both global and per-heap
statistics via one call. Can it be split in two?

> +namespace JSC {
> +
> +    struct MemoryStatistics {

This shouldn't be indented.

> +    MemoryStatistics mStats =
memoryStatistics(JSDOMWindow::commonJSGlobalData());

We prefer to not abbreviate words - and "m" is a prefix for member variables!

r=me, since we need to fix the build ASAP, but I think that this can be
improved a bit later.