Bug 45528 - Move JavaScriptCore statistics gathering into JavaScriptCore
Summary: Move JavaScriptCore statistics gathering into JavaScriptCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Stephanie Lewis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-10 03:24 PDT by Stephanie Lewis
Modified: 2010-10-13 14:13 PDT (History)
3 users (show)

See Also:


Attachments
Patch (13.85 KB, patch)
2010-09-10 03:39 PDT, Stephanie Lewis
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephanie Lewis 2010-09-10 03:24:24 PDT
Refactor JavaScriptCore memory statistics so that WebKit doesn't need to know about the JIT and other implementation details of JavaScriptCore.  Necessary to fix PPC build.
Comment 1 Stephanie Lewis 2010-09-10 03:39:15 PDT
Created attachment 67166 [details]
Patch
Comment 2 WebKit Review Bot 2010-09-10 03:41:37 PDT
Attachment 67166 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/runtime/MemoryStatistics.cpp:36:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
JavaScriptCore/runtime/MemoryStatistics.h:35:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alexey Proskuryakov 2010-09-10 08:53:20 PDT
Comment on attachment 67166 [details]
Patch

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.
Comment 4 Eric Seidel (no email) 2010-10-13 12:27:23 PDT
Attachment 67166 [details] was posted by a committer and has review+, assigning to Stephanie Lewis for commit.
Comment 5 Stephanie Lewis 2010-10-13 14:13:02 PDT
oops, I forgot to close this bug.  Sorry!

Landed in http://trac.webkit.org/projects/webkit/changeset/67265