Closed
Bug 671352
Opened 13 years ago
Closed 13 years ago
Split chrome into multiple compartments for better accounting of JS memory used by chrome code (and add-ons)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 650353
People
(Reporter: ehsan.akhgari, Unassigned)
References
()
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.
Comment 1•13 years ago
|
||
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?
Comment 2•13 years ago
|
||
(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...
Comment 3•13 years ago
|
||
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
Comment 4•13 years ago
|
||
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?
Comment 5•13 years ago
|
||
(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.
Reporter | ||
Comment 6•13 years ago
|
||
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?
Comment 7•13 years ago
|
||
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.
Comment 9•13 years ago
|
||
(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?
Comment 10•13 years ago
|
||
(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.
Comment 11•13 years ago
|
||
(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.
Comment 12•13 years ago
|
||
(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.
Reporter | ||
Comment 13•13 years ago
|
||
(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
Reporter | ||
Comment 14•13 years ago
|
||
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?
Comment 15•13 years ago
|
||
You can't. That's what bug 673331 is open for.
Reporter | ||
Comment 16•13 years ago
|
||
In that case, I'm not sure if there is anything for me to do here...
Assignee: ehsan → nobody
Updated•13 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 17•13 years ago
|
||
Compartment-per-global will fix this more comprehensively.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•