Split chrome into multiple compartments for better accounting of JS memory used by chrome code (and add-ons)

RESOLVED DUPLICATE of bug 650353

Status

()

defect
RESOLVED DUPLICATE of bug 650353
8 years ago
7 years ago

People

(Reporter: Ehsan, Unassigned)

Tracking

Trunk
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2], )

We can probably have a chrome per XUL document and JS module.  See the discussion in the dev.platform thread in the URL field for the background.
We have to be careful that the number of crossCompartmentWrappers doesn't increase exponentially. Nick, the size of the WrapperTable per compartment would be useful in about:memory.
We have seen this explosion with one compartment per iFrame.

It could also lead to *more* fragmentation since half empty arenas can't be reused by other chrome compartments any more. We would need an allocationGroup API as suggested by Andreas.
We need a lot of measurements here.

I noticed that the new chrome-compartment fragmentation problem is startup related. We create a ton of objects during startup that don't survive the first GC. So we end up with many (half)empty arenas that are never reused because the chrome-compartment doesn't grow that big any more. At least for my regular surf behavior. Do we really need 15 - 20MB temporary objects during startup?
(In reply to comment #1)
> We have to be careful that the number of crossCompartmentWrappers doesn't
> increase exponentially.

I thought you and gal did an experiment where you created compartment-per-global and did not observe that many wrappers?  This was a rather important precondition to bug 650353...
Some numbers for the chrome-compartment startup memory consumption:

GChrome performs one GC after startup:
0.8 -> 0.6 MB, 3 ms.

Our GC right after startup with an empty page looks like:
30MB -> 15MB
With our 1-line mockup patch we didn't see a huge increase in cross-compartment wrappers, certainly not exponential. Gregor, want to post some numbers?
(In reply to comment #4)
> With our 1-line mockup patch we didn't see a huge increase in
> cross-compartment wrappers, certainly not exponential. Gregor, want to post
> some numbers?

Yeah I looked up the numbers again. We saw many more compartments for certain workloads but the number of crossCompartment Wrappers didn't increase exponential (expect for GMail?). These are the numbers from my paper.

Alias Origin Wrappers IFrame Wrappers
280s 1  26   2 85
AMAZ 4  280 16 563
BING 1  80   3 105
DIGG 3  114  3 115
EBAY 1  48   1 50
FBOK 1  249  6 445
FLKR 3  185 23 1094
GDOC 6  552  7 277
GMAP 1  88   2 82
GMIL 2  183  9 5654
GOGL 1  60   2 209
HULU 1  103 10 245
ISHK 6  776 41 1396
TECH 11 2324 154 3094
V8BE 1  35   1 35
YTUB 2  183  7 204

The first 2 numbers show the compartment and wrapper count for the current situation (6 months ago).
The last 2 numbers show compartment and wrapper count if we create a compartment per iframe. Some examples:
Flickr would have 23 compartments instead of 3.
Gmail would have 5600 wrappers instead of 180.
Gregor, this bug is really wanted for memshrink, but according to jst you may not get to work on it in the next couple of months.  Can we get a preliminary version of a patch here soon that is _not_ good enough to land on m-c but developers can at least apply it in their own builds and get some stats until we can get a real fix here?
I don't know the code, so I present this as an armchair opinion, but it seems that number of compartments here is another parameter to be optimized over. Compartments carry some overhead, though contribute to general robustness.  Forgive me if I'm stating the obvious. (Which isn't to say anything in particular about this particular bug, FWIW.)

I'm also curious...would module-level globals in chrome-level JS be system-global? Or would extra effort be required to achieve this? The test harnesses may need to be updated if this is the case (to be audited).
(In reply to comment #7)
> I don't know the code, so I present this as an armchair opinion, but it
> seems that number of compartments here is another parameter to be optimized
> over.

This is not a priori true.  We used to have one compartment (back before we had compartments) and we went through a lot of trouble to have more than one.  All other things being equal, sure, we should have fewer compartments, but all other things are often not equal.

> Compartments carry some overhead, though contribute to general
> robustness.  Forgive me if I'm stating the obvious. (Which isn't to say
> anything in particular about this particular bug, FWIW.)

Right.  Compartments give us some security goodness and let us do smaller GCs, both of which are pretty awesome.  There are probably some other benefits I'm forgetting too.

> I'm also curious...would module-level globals in chrome-level JS be
> system-global? Or would extra effort be required to achieve this? The test
> harnesses may need to be updated if this is the case (to be audited).

I don't know what this means.  If you're asking if things that do not currently share global objects would, the answer is no.
(In reply to comment #8)
> (In reply to comment #7)
> > I don't know the code, so I present this as an armchair opinion, but it
> > seems that number of compartments here is another parameter to be optimized
> > over.
> 
> This is not a priori true.  We used to have one compartment (back before we
> had compartments) and we went through a lot of trouble to have more than
> one.  All other things being equal, sure, we should have fewer compartments,
> but all other things are often not equal.

I'm being hand-wavy here, but in general we're optimizing for "real" UX (versus perceived UX which would ignore e.g. security, etc). So for the same reliability, security, etc., # of compartments is just a number.  And yes, all other things are not equal :)

> > Compartments carry some overhead, though contribute to general
> > robustness.  Forgive me if I'm stating the obvious. (Which isn't to say
> > anything in particular about this particular bug, FWIW.)
> 
> Right.  Compartments give us some security goodness and let us do smaller
> GCs, both of which are pretty awesome.  There are probably some other
> benefits I'm forgetting too.

Instrumentation for one. Robustness for another.

> > I'm also curious...would module-level globals in chrome-level JS be
> > system-global? Or would extra effort be required to achieve this? The test
> > harnesses may need to be updated if this is the case (to be audited).
> 
> I don't know what this means.  If you're asking if things that do not
> currently share global objects would, the answer is no.

Quite the opposite: for JS modules which have global variables but are in the (singular) chrome process, will the singleton nature of global values continue to be true?  Or will they have to communicate to affect this?
(In reply to comment #6)
> Gregor, this bug is really wanted for memshrink, but according to jst you
> may not get to work on it in the next couple of months.  Can we get a
> preliminary version of a patch here soon that is _not_ good enough to land
> on m-c but developers can at least apply it in their own builds and get some
> stats until we can get a real fix here?

Don't count on me with time-critical stuff in the next few months. Somebody else should work on it if this is urgent.
(In reply to comment #10)
> Don't count on me with time-critical stuff in the next few months. Somebody
> else should work on it if this is urgent.
Gregor, I think Ehsan is just referring to the "one line mockup patch" referred to in comment 4, if you have that somewhere.
(In reply to comment #11)
> (In reply to comment #10)
> > Don't count on me with time-critical stuff in the next few months. Somebody
> > else should work on it if this is urgent.
> Gregor, I think Ehsan is just referring to the "one line mockup patch"
> referred to in comment 4, if you have that somewhere.

Oh ok that's easy :) This patch can be found in bug 624980.
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > Don't count on me with time-critical stuff in the next few months. Somebody
> > > else should work on it if this is urgent.
> > Gregor, I think Ehsan is just referring to the "one line mockup patch"
> > referred to in comment 4, if you have that somewhere.
> 
> Oh ok that's easy :) This patch can be found in bug 624980.

I'm gonna try this patch...
Assignee: general → ehsan
So, with this patch, I do get quite a few system compartments, each with a different address.  How can I tell which document/module they belong to?
You can't.  That's what bug 673331 is open for.
In that case, I'm not sure if there is anything for me to do here...
Assignee: ehsan → nobody
Depends on: 673331
Whiteboard: [MemShrink] → [MemShrink:P2]
Compartment-per-global will fix this more comprehensively.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: cpg
You need to log in before you can comment on or make changes to this bug.