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)

x86
macOS
defect
Not set
normal

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.
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: 13 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.