Last Comment Bug 755581 - Some extra post-CPG memory reporters for JS
: Some extra post-CPG memory reporters for JS
Status: RESOLVED FIXED
[MemShrink:P2]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Nicholas Nethercote [:njn]
:
:
Mentors:
Depends on:
Blocks: 752865
  Show dependency treegraph
 
Reported: 2012-05-15 17:00 PDT by Nicholas Nethercote [:njn]
Modified: 2012-05-18 13:27 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch 1: change "mjit/data" reporter to "mjit-data" (1.16 KB, patch)
2012-05-15 17:51 PDT, Nicholas Nethercote [:njn]
luke: review+
Details | Diff | Splinter Review
Patch 2: change "runtime/normal" reporter to "runtime/dtoa" (5.56 KB, patch)
2012-05-15 17:52 PDT, Nicholas Nethercote [:njn]
luke: review+
Details | Diff | Splinter Review
Patch 3: Clean up JSRuntime::sizeOfIncludingThis() (14.11 KB, patch)
2012-05-15 18:17 PDT, Nicholas Nethercote [:njn]
luke: review+
Details | Diff | Splinter Review
Patch 4: add "math-cache", "script-filenames" and "compartment-objects" reporters (6.17 KB, patch)
2012-05-15 18:24 PDT, Nicholas Nethercote [:njn]
luke: review+
Details | Diff | Splinter Review
Patch 5: add "cross-compartment-wrappers" reporter (2.92 KB, patch)
2012-05-15 18:28 PDT, Nicholas Nethercote [:njn]
luke: review+
Details | Diff | Splinter Review

Description Nicholas Nethercote [:njn] 2012-05-15 17:00:57 PDT
This will cover things like JSCompartment objects, and cross-compartment wrappers.
Comment 1 Nicholas Nethercote [:njn] 2012-05-15 17:51:00 PDT
Created attachment 624260 [details] [diff] [review]
Patch 1: change "mjit/data" reporter to "mjit-data"

This reduces some unnecessary tree nesting in about:memory.  This:

│  │   ├─────193,200 B (00.05%) -- mjit
│  │   │     └──193,200 B (00.05%) ── data [12]

Becomes this:

│  │   ├─────193,200 B (00.05%) -- mjit-data [12]

The nested version made sense when mjit-code was stored in the compartment,
but it's now in the runtime.
Comment 2 Nicholas Nethercote [:njn] 2012-05-15 17:52:43 PDT
Created attachment 624261 [details] [diff] [review]
Patch 2: change "runtime/normal" reporter to "runtime/dtoa"

The "runtime/normal" entry in about:memory only measures DtoaState, so let's
change it to "runtime/dtoa".
Comment 3 Nicholas Nethercote [:njn] 2012-05-15 18:17:40 PDT
Created attachment 624265 [details] [diff] [review]
Patch 3: Clean up JSRuntime::sizeOfIncludingThis()

JSRuntime::sizeOfIncludingThis() is getting unwieldy.  This patch puts all
those parameters into a struct.  It also moves the measurement of a couple
of JSRuntime-related sizes into JSRuntime::sizeOfIncludingThis().
Comment 4 Nicholas Nethercote [:njn] 2012-05-15 18:24:19 PDT
Created attachment 624267 [details] [diff] [review]
Patch 4: add "math-cache", "script-filenames" and "compartment-objects" reporters

This patch adds reporters for the mathCache, ScriptFilenameTable, and 
JSCompartment objects.  The latter is the biggest, 200KB+ at start-up on 64-bit.
Comment 5 Nicholas Nethercote [:njn] 2012-05-15 18:28:18 PDT
Created attachment 624269 [details] [diff] [review]
Patch 5: add "cross-compartment-wrappers" reporter

This is usually tiny (e.g. 1KB per compartment), but it's a good reporter to have because it might blow out on occasion.
Comment 6 Luke Wagner [:luke] 2012-05-15 18:41:37 PDT
Comment on attachment 624265 [details] [diff] [review]
Patch 3: Clean up JSRuntime::sizeOfIncludingThis()

Looks better
Comment 7 Nicholas Nethercote [:njn] 2012-05-15 19:03:17 PDT
Five reviews in under an hour... thanks!  \o/
Comment 10 :Ms2ger (⌚ UTC+1/+2) 2012-05-17 04:00:16 PDT
Comment on attachment 624265 [details] [diff] [review]
Patch 3: Clean up JSRuntime::sizeOfIncludingThis()

Review of attachment 624265 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/public/MemoryMetrics.h
@@ +152,5 @@
>        , currCompartmentStats(NULL)
>        , mallocSizeOf(mallocSizeOf)
>      {}
>  
> +    js::RuntimeSizes runtime;

Should be JS::...
Comment 11 Nicholas Nethercote [:njn] 2012-05-17 17:36:21 PDT
> > +    js::RuntimeSizes runtime;
> 
> Should be JS::...

Nice catch.  I was wondering how that even compiled, when Luke told me we have this:

  namespace js { using namespace JS }

I changed it.  Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/dcd6365a8255
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2012-05-18 13:27:05 PDT
Thanks!

https://hg.mozilla.org/mozilla-central/rev/dcd6365a8255

Note You need to log in before you can comment on or make changes to this bug.