Last Comment Bug 619558 - (GenerationalGC) [meta] Implement generational garbage collection
(GenerationalGC)
: [meta] Implement generational garbage collection
Status: RESOLVED FIXED
[games:p2] [js:p1:fx31][talos_regress...
: feature, leave-open, perf
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal with 50 votes (vote)
: mozilla31
Assigned To: Terrence Cole [:terrence]
: Andrei Vaida, QA [:avaida] – please ni? me
Mentors:
: 701448 (view as bug list)
Depends on: 619561 641027 658676 673454 707049 714647 750733 752093 ExactRooting 764876 782646 831506 832489 838810 851181 851342 863521 863524 ggcfuzz 877907 882482 883234 883466 883498 885955 886575 893486 906091 916829 919536 921178 923179 925397 925817 925863 941779 956434 967720 984684 985562 986147 987160 988053 988821 991845 994450 995442 1009675 1015618 1020751 1068399 1127246 1149526
Blocks: GC 619392 650063 gecko-games k9o-perf 765980 874174 874580 928894 465923 590379 605426 625615 664613 764220 768745 256meg 933313 990085
  Show dependency treegraph
 
Reported: 2010-12-15 16:22 PST by Paul Biggar
Modified: 2016-04-20 02:51 PDT (History)
113 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
disabled
fixed
32+


Attachments
v0: A trivial do-nothing, nursery-stealing Generational GC (21.48 KB, patch)
2012-03-05 13:11 PST, Terrence Cole [:terrence]
no flags Details | Diff | Splinter Review
enable_ggc_by_default_on_desktop-v0.diff (4.23 KB, patch)
2014-02-18 14:39 PST, Terrence Cole [:terrence]
sphink: review+
Details | Diff | Splinter Review
enable_ggc_by_default_on_desktop-v1.diff (4.23 KB, patch)
2014-02-18 16:19 PST, Terrence Cole [:terrence]
terrence: review+
terrence: superreview+
Details | Diff | Splinter Review
noggc-startup (41.45 KB, application/x-gzip)
2014-03-27 05:01 PDT, Nicholas Nethercote [:njn]
no flags Details
ggc-startup (46.76 KB, application/x-gzip)
2014-03-27 05:02 PDT, Nicholas Nethercote [:njn]
no flags Details
noggc-mem50-full (355.53 KB, application/x-gzip)
2014-03-27 05:02 PDT, Nicholas Nethercote [:njn]
no flags Details
ggc-mem50-full (362.86 KB, application/x-gzip)
2014-03-27 05:03 PDT, Nicholas Nethercote [:njn]
no flags Details
noggc-mem50-after (38.20 KB, application/x-gzip)
2014-03-27 05:03 PDT, Nicholas Nethercote [:njn]
no flags Details
ggc-mem50-after.json.gz (38.00 KB, application/x-gzip)
2014-03-27 05:04 PDT, Nicholas Nethercote [:njn]
no flags Details
Results of latest landing (190.24 KB, text/html)
2014-03-28 01:17 PDT, Steve Fink [:sfink] [:s:]
no flags Details

Description Paul Biggar 2010-12-15 16:22:21 PST
Bill's notes: https://wiki.mozilla.org/JavaScript:GenerationalGC
Comment 1 Terrence Cole [:terrence] 2011-11-14 16:00:33 PST
*** Bug 701448 has been marked as a duplicate of this bug. ***
Comment 2 Terrence Cole [:terrence] 2012-03-05 13:11:41 PST
Created attachment 603033 [details] [diff] [review]
v0: A trivial do-nothing, nursery-stealing Generational GC

A random test collector I whipped up this weekend to play with potential performance tradeoffs and to see if tying a new allocator into JaegerMonkey is as easy as it looks.
Comment 3 Paul Wright 2012-03-05 13:19:21 PST
Will there be any differences in how the Generational GC works with Ion versus Jager?  If so, would it make more sense to prototype GGC with Ion?
Comment 4 David Mandelin [:dmandelin] 2012-05-01 11:55:07 PDT
Too big for the k9o initial time frame.
Comment 5 Justin Lebar (not reading bugmail) 2012-10-02 16:45:29 PDT
*** Bug 668809 has been marked as a duplicate of this bug. ***
Comment 6 Terrence Cole [:terrence] 2014-02-18 14:39:52 PST
Created attachment 8377864 [details] [diff] [review]
enable_ggc_by_default_on_desktop-v0.diff

This is identical to the patch Ted reviewed for exact rooting and has extensive testing on Maple, so shouldn't need too skeptical a review.
Comment 7 Steve Fink [:sfink] [:s:] 2014-02-18 14:41:24 PST
Comment on attachment 8377864 [details] [diff] [review]
enable_ggc_by_default_on_desktop-v0.diff

Review of attachment 8377864 [details] [diff] [review]:
-----------------------------------------------------------------

I am happy to ride on the coattails of ted's review.
Comment 8 Dave Garrett 2014-02-18 15:11:17 PST
Comment on attachment 8377864 [details] [diff] [review]
enable_ggc_by_default_on_desktop-v0.diff

>+JSGC_GENERATIONAL=1
>+MOZ_ARG_DISABLE_BOOL(gcgenerational,
>+[  --enable-gcgenerational Disable generational GC],
>+    JSGC_GENERATIONAL= ,
>+    JSGC_GENERATIONAL=1 )
> if test -n "$JSGC_GENERATIONAL"; then
>     AC_DEFINE(JSGC_GENERATIONAL)
> fi

Correct me if I'm wrong, but shouldn't that be "--disable-gcgenerational" there?
Comment 9 Terrence Cole [:terrence] 2014-02-18 16:19:11 PST
Created attachment 8377934 [details] [diff] [review]
enable_ggc_by_default_on_desktop-v1.diff

(In reply to Dave Garrett from comment #8)
> Correct me if I'm wrong, but shouldn't that be "--disable-gcgenerational"
> there?

Great catch! It does indeed look like I've missed a string.
Comment 11 Jan de Mooij [:jandem] 2014-03-26 19:31:54 PDT
(In reply to Jon Coppeard (:jonco) from comment #10)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/52f43e3f552f

\o/ \o/
Comment 12 Paul Wright 2014-03-26 19:32:58 PDT
Awesome!
Comment 14 Terrence Cole [:terrence] 2014-03-26 19:54:52 PDT
Comment on attachment 8377934 [details] [diff] [review]
enable_ggc_by_default_on_desktop-v1.diff

sr+ from Naveed, over-the-shoulder.
Comment 15 Nicholas Nethercote [:njn] 2014-03-26 20:33:26 PDT
Let's watch AWSY closely.
Comment 16 Carsten Book [:Tomcat] - PTO-back Sept 4th 2014-03-27 02:15:47 PDT
sorry had to backout this changeset, seems its causing frequent test failures on OSX 10.8 like https://tbpl.mozilla.org/php/getParsedLog.php?id=36790332&tree=Mozilla-Inbound
Comment 17 Nicholas Nethercote [:njn] 2014-03-27 04:59:50 PDT
The AWSY results will be available later today. In the meantime, I did some local benchmarking using http://gregor-wagner.com/tmp/mem50. I dumped memory reports at three points:
- immediately after start-up
- after running the benchmark (with all 50 tabs open)
- after closing all the tabs and hitting "minimize memory usage" three times

I tested against revisions b0dbbb5d770a (just before landing) and 52f43e3f552f (just after landing). This is on a Mac.

The summary is that 'resident' memory usage is up by roughly 8%.
- At start-up, it is up by 14 MiB, from 179 to 193
- With 50 tabs open, it is up by 117 MiB, from 1270 to 1387
- After closing the tabs, it is up by 35 MiB, from 409 to 444

I will attach the relevant memory report files shortly.

This is disappointing. I'd been hoping that by filtering out many of the short-lived objects, the tenured heap would grow more slowly and that this would actually reduce memory usage.

At start-up:
- 7 MiB is from nurseries; 4 MiB in the main runtime, and 2 and 1 in the two workers
- 4 MiB is additional heap-unclassified, which is surprising
- 6? MiB is just more GC stuff being held onto, which is surprising

With 50 tabs open:
- 17.5 MiB is additional heap-overhead, which is surprising. Are we doing more heap allocations, maybe due to store buffers?
- 16 MiB is additional heap-unclassified

With 50 tabs closed:
- 15 MiB is additional heap-overhead

So the clear signals are coming from nurseries, heap-overhead, and heap-unclassified. And then there's just more general JS stuff being held onto as well.
Comment 18 Nicholas Nethercote [:njn] 2014-03-27 05:01:39 PDT
Created attachment 8397781 [details]
noggc-startup
Comment 19 Nicholas Nethercote [:njn] 2014-03-27 05:02:07 PDT
Created attachment 8397782 [details]
ggc-startup
Comment 20 Nicholas Nethercote [:njn] 2014-03-27 05:02:51 PDT
Created attachment 8397783 [details]
noggc-mem50-full
Comment 21 Nicholas Nethercote [:njn] 2014-03-27 05:03:18 PDT
Created attachment 8397785 [details]
ggc-mem50-full
Comment 22 Nicholas Nethercote [:njn] 2014-03-27 05:03:52 PDT
Created attachment 8397786 [details]
noggc-mem50-after
Comment 23 Nicholas Nethercote [:njn] 2014-03-27 05:04:20 PDT
Created attachment 8397787 [details]
ggc-mem50-after.json.gz
Comment 24 Nicholas Nethercote [:njn] 2014-03-27 05:35:16 PDT
It's worth noting that a typical worker (e.g. osfile_async_worker.js, SessionWorker.js) is currently 2--3 MiB, so an extra 1 MiB of nursery is a big increase, and goes against bug 864927's goals.
Comment 25 Nicholas Nethercote [:njn] 2014-03-27 05:44:24 PDT
Thinking some more...

- The nursery space isn't surprising. For workers, yesterday we discussed the idea of possibly decommitting part of it and having some kind of guard page, in order to effectively reduce the chunk size.

- The increased tenured heap size is surprising. It seems like lots of GC things are being held onto for longer?

- The heap-overhead increase is surprising. I don't understand exactly what that measures, but it's some sort of malloc heap fragmentation measure. Does GGC cause more malloc heap churn?

- The heap-unclassified increase is also surprising. I thought we had coverage for all the new structures. Are there some we are missing?
Comment 26 Nicholas Nethercote [:njn] 2014-03-27 06:02:03 PDT
> - The heap-unclassified increase is also surprising. I thought we had
> coverage for all the new structures. Are there some we are missing?

mccr8 just worked this out -- we're not measuring all the malloc'd stuff hanging off the GC things in the nursery. And it sounds like this isn't even possible, because the nursery isn't designed to be iteratable. Luke suggested that forcing a nursery collection before running the memory reporters would avoid this problem, though it could result in misleading measurements.
Comment 27 Jan de Mooij [:jandem] 2014-03-27 06:32:49 PDT
(In reply to Nicholas Nethercote [:njn] from comment #26)
> mccr8 just worked this out -- we're not measuring all the malloc'd stuff
> hanging off the GC things in the nursery. And it sounds like this isn't even
> possible, because the nursery isn't designed to be iteratable.

The nursery does have a list of external malloc'ed slots (because it has to free them eventually), so it seems you could measure this somehow.
Comment 28 Nicholas Nethercote [:njn] 2014-03-27 09:01:11 PDT
> The nursery does have a list of external malloc'ed slots (because it has to
> free them eventually), so it seems you could measure this somehow.

Ah yes. This is Nursery::hugeSlots, which stores malloc'd slots. But there are other malloc'd things that can hang of objects: elements, and things in the private slot (e.g. for PropertyIteratorObject and RegExpStaticsObject), and things in fixed slots (e.g. for ArgumentsObject), and reserved slots (e.g. for AsmJSModuleObject). How are these tracked?

Also, how is the number of committed chunks in the Nursery controlled? When does it increase and decrease?
Comment 29 Terrence Cole [:terrence] 2014-03-27 12:43:00 PDT
(In reply to Nicholas Nethercote [:njn] from comment #28)
> > The nursery does have a list of external malloc'ed slots (because it has to
> > free them eventually), so it seems you could measure this somehow.
> 
> Ah yes. This is Nursery::hugeSlots, which stores malloc'd slots. But there
> are other malloc'd things that can hang of objects: elements, and things in
> the private slot (e.g. for PropertyIteratorObject and RegExpStaticsObject),
> and things in fixed slots (e.g. for ArgumentsObject), and reserved slots
> (e.g. for AsmJSModuleObject). How are these tracked?

|hugeSlots| stores both slots and elements. The nursery only stores background finalizable objects. The background finalizer only knows how to free slots and elements, so we would be leaking regardless if that were the case.

> Also, how is the number of committed chunks in the Nursery controlled? When
> does it increase and decrease?

It is based on the promotion rate. Search for growAllocableSpace and shrinkAllocableSpace in Nursery.cpp.
Comment 30 Steve Fink [:sfink] [:s:] 2014-03-27 13:20:20 PDT
(In reply to Terrence Cole [:terrence] from comment #29)
> (In reply to Nicholas Nethercote [:njn] from comment #28)
> > > The nursery does have a list of external malloc'ed slots (because it has to
> > > free them eventually), so it seems you could measure this somehow.
> > 
> > Ah yes. This is Nursery::hugeSlots, which stores malloc'd slots. But there
> > are other malloc'd things that can hang of objects: elements, and things in
> > the private slot (e.g. for PropertyIteratorObject and RegExpStaticsObject),
> > and things in fixed slots (e.g. for ArgumentsObject), and reserved slots
> > (e.g. for AsmJSModuleObject). How are these tracked?
> 
> |hugeSlots| stores both slots and elements. The nursery only stores
> background finalizable objects. The background finalizer only knows how to
> free slots and elements, so we would be leaking regardless if that were the
> case.

njn, the classes with malloced data in their private slots (eg PropertyIteratorObject) will have finalizers, and we do not nursery allocate anything with a finalizer. (You can be background finalizable with or without a custom finalizer hook.)

The examples you list all have custom finalizers: PropertyIteratorObject, RegExpStaticsObject, ArgumentsObject, and AsmJSModuleObject.
Comment 31 Nicholas Nethercote [:njn] 2014-03-27 15:44:42 PDT
So the AWSY results are in and they actually look good -- I was expecting an obvious jump but there wasn't one. All the changes were within the noise, except perhaps for the 'explicit' start-up measurements, which were up by about 6 MiB (those measurements don't vary much, so this is beyond the noise).

Hopefully this is accurate, and somehow my measurements from this morning are bogus in some way...
Comment 32 Steve Fink [:sfink] [:s:] 2014-03-27 20:31:50 PDT
re-landing: https://hg.mozilla.org/integration/mozilla-inbound/rev/786057e01ef3
Comment 33 Steve Fink [:sfink] [:s:] 2014-03-28 01:17:42 PDT
Created attachment 8398386 [details]
Results of latest landing

No further comment.
Comment 34 Nicholas Nethercote [:njn] 2014-03-28 07:38:08 PDT
> So the AWSY results are in and they actually look good

Subsequent measurements were similar, which is good.

I did some more measurements by hand. Startup 'resident' is still about 5% worse on my Mac, but I also tried opening every article on the front page of TechCrunch and I got about a 30 MiB (2.5%) reduction.
Comment 35 Hannes Verschore [:h4writer] 2014-03-28 10:25:31 PDT
This definitely needs to get into the release notes. So nominating.
Comment 36 Boris Zbarsky [:bz] 2014-03-28 10:41:22 PDT
Either this or bug 988821 caused 5-7% dromaeo-dom regressions across the board.  Was that expected?

In particular, dom_attr got consistently slower....
Comment 37 Hannes Verschore [:h4writer] 2014-03-28 10:58:32 PDT
(In reply to Boris Zbarsky [:bz] from comment #36)
> Either this or bug 988821 caused 5-7% dromaeo-dom regressions across the
> board.  Was that expected?
> 
> In particular, dom_attr got consistently slower....

Is this on a mac? Since there is one bug (bug 987047) still open that made ggc a regression on awfy instead of an improvement on mac. So if this is mac-only, I would bet it is that problem.
Comment 38 Terrence Cole [:terrence] 2014-03-28 15:35:04 PDT
I will benchmark as soon as I get home from the work-week.
Comment 39 Wes Kocher (:KWierso) 2014-03-28 16:21:14 PDT
https://hg.mozilla.org/mozilla-central/rev/786057e01ef3
Comment 40 Joel Maher ( :jmaher) 2014-03-31 08:32:49 PDT
As mentioned in comment 36, this shows a regression on talos (linux, linux64, osx 10.6, osx 10.8, win7, win8) for dromaeo_dom:
http://graphs.mozilla.org/graph.html#tests=[[73,131,31],[73,131,25],[73,131,35],[73,131,33],[73,63,24],[73,63,21]]&sel=1395660120282,1396264920282&displayrange=7&datatype=running

on datazilla you can see on 10.6 traverse.html is regressed:
https://datazilla.mozilla.org/?start=1395659415&stop=1396264215&product=Firefox&repository=Mozilla-Inbound&os=mac&os_version=OS%20X%2010.6.8&test=dromaeo_dom&graph_search=786057e01ef3&error_bars=false&project=talos

and for win7, traverse.html is regressed as well:
https://datazilla.mozilla.org/?start=1395659415&stop=1396264215&product=Firefox&repository=Mozilla-Inbound-Non-PGO&os=win&os_version=6.1.7601&test=dromaeo_dom&graph_search=786057e01ef3&error_bars=false&project=talos


In addition to dromaeo(dom) regression, kraken has regressed:
http://graphs.mozilla.org/graph.html#tests=[[232,63,24],[232,63,21],[232,131,25],[232,131,33],[232,131,35],[232,131,31]]&sel=1395668004184,1396272804184&displayrange=7&datatype=running

on all platforms except for linux 32 where it improved!

here is a view of datazilla for win7 kraken:
https://datazilla.mozilla.org/?start=1395659415&stop=1396264215&product=Firefox&repository=Mozilla-Inbound-Non-PGO&os=win&os_version=6.1.7601&test=kraken&graph_search=786057e01ef3&error_bars=false&project=talos

we regressed on:
json-parse-financial
audio-oscillator
audio-fft
audio-beat-detection

but we improved on stanford-cryto-aes though! 


Now for even more regressions, ts paint:
http://graphs.mozilla.org/graph.html#tests=[[83,131,33],[83,131,25],[83,131,35]]&sel=1395929540852.4817,1396010084966.6313,0,1080&displayrange=7&datatype=running

This is showing regressions on win7, win8, linux 32, linux64


the last regression I see from here is the tpaint (opening windows) regression:
http://graphs.mozilla.org/graph.html#tests=[[82,131,25],[82,131,37],[82,131,31],[82,131,33],[82,63,21]]&sel=none&displayrange=7&datatype=running

this is seen on winxp, win7, win7 and linxu32 and osx 10.6 


I have done a lot of retriggers on tests before and after this landing:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&fromchange=7cad4bd4bfe5&tochange=d9e6a6c40a57&jobname=mozilla-inbound%20talos%20other
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&fromchange=7cad4bd4bfe5&tochange=d9e6a6c40a57&jobname=mozilla-inbound%20talos%20dromaeojs

If there are no plans to back this out or fix this, I would like to see a post outlining that we are fine regressing these 4 performance tests on the majority of our platforms.
Comment 41 Terrence Cole [:terrence] 2014-03-31 10:32:57 PDT
(In reply to Joel Maher (:jmaher) from comment #40)
> 
> If there are no plans to back this out or fix this, I would like to see a
> post outlining that we are fine regressing these 4 performance tests on the
> majority of our platforms.

We expected performance to vary up and down across the board. We still have 4 weeks to address the cases where the variance is more down than up, so lets hold off on the backout for a bit.
Comment 42 Joel Maher ( :jmaher) 2014-03-31 10:57:04 PDT
I think I had outlined all the regressions in my previous comment, make a little work and that should be minimized.
Comment 43 Joel Maher ( :jmaher) 2014-04-01 12:46:23 PDT
one other regression is the TART test, I see it on windows 8 (PGO):
http://graphs.mozilla.org/graph.html#tests=[[293,63,31]]&sel=1393767907190,1396359907190&displayrange=30&datatype=running

I had done some retriggers and I can see the change prior to this with a 4.4-4.6 range on tart and this change has 4.5-5.0:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=WINNT%206.2%20mozilla-inbound%20pgo%20talos%20svgr&fromchange=092a737e9451&tochange=a4c9a284e014

this took a while to get an alert as we don't have less frequent pgo builds.
Comment 44 Terrence Cole [:terrence] 2014-04-02 08:34:04 PDT
With Bug 990336 landed, we've recovered our kraken performance and then some. Verified on AWFY and talos.
Comment 45 Terrence Cole [:terrence] 2014-04-02 08:34:56 PDT
Whoops, didn't mean to un-needinfo myself; still more to be done for dromaeo.
Comment 46 Sylvestre Ledru [:sylvestre] 2014-04-09 07:13:24 PDT
Added to the release note for 31. If that changes (ie ship in 32), please reset the relnotes flag to "?"
Comment 47 Viktor Stanchev [:vikstrous] 2014-04-09 14:36:20 PDT
see bug 994329
Comment 48 Joel Maher ( :jmaher) 2014-04-14 07:52:14 PDT
we still have a few other regressions and <2 weeks to go. I am glad to see the kraken regression fixed, is it realistic to get tspaint, tart, and dromaeo fixed as well?
Comment 49 Nicholas Nethercote [:njn] 2014-05-01 06:35:30 PDT
I've removed the MemShrink:P1 tag because this is basically done and it turned out to not have much effect on memory usage. I've added a MemShrink tag to bug 650161 (compacting GC) which should have a bigger effect.
Comment 50 Sylvestre Ledru [:sylvestre] 2014-07-13 09:49:51 PDT
Resetting the relnotes flag to make sure we have it in the 32 beta release notes.
Comment 51 Ryan VanderMeulen [:RyanVM] 2014-07-16 08:03:54 PDT
This still landed on trunk during the Fx31 cycle, even though it was later reverted on mozilla-beta.
Comment 52 Sylvestre Ledru [:sylvestre] 2014-07-17 07:35:57 PDT
Moved to the 32 beta release notes
Comment 53 Joel Maher ( :jmaher) 2014-10-01 07:22:57 PDT
I believe this is released now, we appear to have not addressed all the talos performance regressions from this, specifically looking back in the last 6 months, windows 7 tpaint is worse:
http://graphs.mozilla.org/graph.html#tests=%5B%5B82,131,25%5D,%5B82,53,25%5D%5D&sel=1380637141812,1412173141812&displayrange=365&datatype=running

Are we done with ggc?  are there next steps?
Comment 54 Terrence Cole [:terrence] 2014-10-02 10:24:10 PDT
No, we still plan to address this; it isn't worth holding up GGC over it, however.

Note You need to log in before you can comment on or make changes to this bug.