Some extra post-CPG memory reporters for JS

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(5 attachments)

(Assignee)

Description

5 years ago
This will cover things like JSCompartment objects, and cross-compartment wrappers.
(Assignee)

Updated

5 years ago
Blocks: 752865
(Assignee)

Comment 1

5 years ago
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.
Attachment #624260 - Flags: review?(luke)
(Assignee)

Comment 2

5 years ago
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".
Attachment #624261 - Flags: review?(luke)

Updated

5 years ago
Attachment #624260 - Flags: review?(luke) → review+

Updated

5 years ago
Attachment #624261 - Flags: review?(luke) → review+
(Assignee)

Comment 3

5 years ago
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().
Attachment #624265 - Flags: review?(luke)
(Assignee)

Comment 4

5 years ago
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.
Attachment #624267 - Flags: review?(luke)
(Assignee)

Comment 5

5 years ago
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.
Attachment #624269 - Flags: review?(luke)

Comment 6

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

Looks better
Attachment #624265 - Flags: review?(luke) → review+

Updated

5 years ago
Attachment #624267 - Flags: review?(luke) → review+

Updated

5 years ago
Attachment #624269 - Flags: review?(luke) → review+
(Assignee)

Comment 7

5 years ago
Five reviews in under an hour... thanks!  \o/
(Assignee)

Comment 8

5 years ago
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
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
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
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::...
(Assignee)

Comment 11

5 years ago
> > +    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
Thanks!

https://hg.mozilla.org/mozilla-central/rev/dcd6365a8255
You need to log in before you can comment on or make changes to this bug.