755581 - Some extra post-CPG memory reporters for JS

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

This will cover things like JSCompartment objects, and cross-compartment wrappers.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: 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 [inactive]]

Attachment: 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 [inactive]]

Attachment: 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 [inactive]]

Attachment: 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 [inactive]]

Attachment: 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]]

Comment on attachment 624265 [details] [diff] [review]
Patch 3: Clean up JSRuntime::sizeOfIncludingThis()

Looks better

Comment 7

[Nicholas Nethercote [inactive]]

Five reviews in under an hour... thanks!  \o/

Comment 8

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d7a4dc88acf
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b1d60b3bc57
https://hg.mozilla.org/integration/mozilla-inbound/rev/2eba6266c527
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf84a728f4ed
https://hg.mozilla.org/integration/mozilla-inbound/rev/747124aac893

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3d7a4dc88acf
https://hg.mozilla.org/mozilla-central/rev/2b1d60b3bc57
https://hg.mozilla.org/mozilla-central/rev/2eba6266c527
https://hg.mozilla.org/mozilla-central/rev/bf84a728f4ed
https://hg.mozilla.org/mozilla-central/rev/747124aac893

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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 [inactive]]

> > +    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 (he/him; ⌚ UTC+1/+2)]

Thanks!

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