1008473 - (SemispaceGC) Implement a two-generation nursery

Status: RESOLVED WONTFIX

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

This is along the same lines as bug 902174, but will be simpler to implement in a short timeframe.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: 0 - make Value::toGCThing() return a Cell*

Comment 2

[Jon Coppeard (:jonco)]

Attachment: 1 - Misc refactoring and removal of unused methods

Comment 3

[Jon Coppeard (:jonco)]

Attachment: 2 - Add NurserySpace class

Comment 4

[Jon Coppeard (:jonco)]

Attachment: 3 - Add a second space to the nursery

Comment 5

[Jon Coppeard (:jonco)]

Attachment: 4 - Allow preservation of store buffer entries

Comment 6

[Jon Coppeard (:jonco)]

Attachment: 5 - Implement semispace GC

Comment 7

[Terrence Cole [:terrence]]

Comment on attachment 8431710 [details] [diff] [review]
0 - make Value::toGCThing() return a Cell*

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

\o/

Comment 8

[Terrence Cole [:terrence]]

Comment on attachment 8431711 [details] [diff] [review]
1 - Misc refactoring and removal of unused methods

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

Thanks!

Comment 9

[Terrence Cole [:terrence]]

Comment on attachment 8431712 [details] [diff] [review]
2 - Add NurserySpace class

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

Great abstraction!

::: js/src/gc/Nursery.cpp
@@ +144,5 @@
> +    JS_ASSERT(isEnabled());
> +    JS_ASSERT(!runtime_->isHeapBusy());
> +    JS_ASSERT(position_ >= allocStart_);
> +
> +    if (position_ + size > chunkEnd_) {

Just pushed bug 1017852, so you'll probably want to apply that here.

Comment 10

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac7043963695
https://hg.mozilla.org/integration/mozilla-inbound/rev/bff5585f1b9b

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ac7043963695
https://hg.mozilla.org/mozilla-central/rev/bff5585f1b9b

Comment 12

[Olav Nymoen [:olav]]

Couldn't build on OSX without also adding

js::gc::Cell   *cell

to line 305 (little endian something something)

Comment 13

[Jon Coppeard (:jonco)]

Attachment: 1.5 - Fix build error

Yes, I broke little-endian 32 bit architectures.

Comment 14

[Terrence Cole [:terrence]]

Comment on attachment 8433372 [details] [diff] [review]
1.5 - Fix build error

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

r=me

Comment 15

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4bb0fa033d15

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4bb0fa033d15

Comment 17

[Jon Coppeard (:jonco)]

Attachment: 2 - Add NurserySpace class

Comment 18

[Jon Coppeard (:jonco)]

Attachment: 3 - Add a second space to the nursery add-second-space

Comment 19

[Jon Coppeard (:jonco)]

Attachment: 4 - Allow preservation of store buffer entriesmaybe-keep-storebuffer-entries

Comment 20

[Jon Coppeard (:jonco)]

Attachment: 5 - Update key after marking for callback postbarriers

Comment 21

[Jon Coppeard (:jonco)]

Attachment: 6 - Reinstate generic hashtable barrier

This reverts that change that removed the generic hashtable postbarrier function.  This gets rid of the casting away of hashtable barriers before doing the rekeying, as we now need this create new store buffer entries where appropriate, i.e. when moving a value that contains nursery pointers.  This was not possible before because all objects were evacuated from the nursery on minor GC.

Comment 22

[Jon Coppeard (:jonco)]

Attachment: 7 - Fix ArrayBufferObject marking and sweeping

We now need to allow postbarriers to add store buffer entries during minor GC.

Comment 23

[Jon Coppeard (:jonco)]

Attachment: 8 - Implement semispace GC

Comment 24

[Jon Coppeard (:jonco)]

This now passes all tests and try is green.

Current octane benchmark results on linux with 16MB nursery, average of 10 runs, semispace results on the right:

         Richards:       22540.3 (10  0.6%)       22447.2 (10  0.7%)  -0.4%
        DeltaBlue:       35849.1 (10  0.6%)       33636.8 (10  4.0%)  -6.2%
           Crypto:       26765.1 (10  0.4%)       26802.5 (10  0.2%)   0.1%
         RayTrace:       84477.4 (10  1.1%)       82338.8 (10  7.5%)  -2.5%
      EarleyBoyer:       33213.7 (10  0.5%)       29295.6 (10  1.3%) -11.8%
           RegExp:        3234.2 (10  1.3%)        3219.3 (10  0.5%)  -0.5%
            Splay:       17577.2 (10  2.4%)       17490.3 (10  1.8%)  -0.5%
     SplayLatency:       11985.4 (10  0.7%)       12016.4 (10  1.6%)   0.3%
     NavierStokes:       32826.2 (10  0.2%)       32834.3 (10  0.2%)   0.0%
            PdfJS:       19285.6 (10  1.6%)       20391.6 (10  1.1%)   5.7%
         Mandreel:       31807.1 (10  2.3%)       32055.6 (10  2.1%)   0.8%
  MandreelLatency:       41505.5 (10  4.2%)       42623.4 (10  3.5%)   2.7%
          Gameboy:       58851.7 (10  0.4%)       58754.2 (10  0.3%)  -0.2%
         CodeLoad:       19922.7 (10  0.7%)       19829.5 (10  0.8%)  -0.5%
            Box2D:       37498.3 (10  1.7%)       36924.9 (10  0.6%)  -1.5%
             zlib:       85531.4 (10  0.5%)       85714.3 (10  0.6%)   0.2%
       Typescript:       28189.7 (10  3.0%)       27735.3 (10  1.3%)  -1.6%
Score (version 9):       27854.1 (10  0.8%)       27572.4 (10  0.7%)  -1.0%

And with current 16M nursery comparted to 32MB semispace nursery:

         Richards:       22540.3 (10  0.6%)       22510.7 (10  0.7%)  -0.1%
        DeltaBlue:       35849.1 (10  0.6%)       33771.9 (10  4.8%)  -5.8%
           Crypto:       26765.1 (10  0.4%)       26803.8 (10  0.2%)   0.1%
         RayTrace:       84477.4 (10  1.1%)       84595.8 (10  1.7%)   0.1%
      EarleyBoyer:       33213.7 (10  0.5%)       32809.7 (10  1.1%)  -1.2%
           RegExp:        3234.2 (10  1.3%)        3094.6 (10  0.5%)  -4.3%
            Splay:       17577.2 (10  2.4%)       17689.6 (10  1.4%)   0.6%
     SplayLatency:       11985.4 (10  0.7%)       12364.1 (10  0.8%)   3.2%
     NavierStokes:       32826.2 (10  0.2%)       32809.7 (10  0.2%)  -0.1%
            PdfJS:       19285.6 (10  1.6%)       20226.0 (10  0.9%)   4.9%
         Mandreel:       31807.1 (10  2.3%)       32438.0 (10  0.4%)   2.0%
  MandreelLatency:       41505.5 (10  4.2%)       43503.2 (10  0.7%)   4.8%
          Gameboy:       58851.7 (10  0.4%)       58983.3 (10  0.3%)   0.2%
         CodeLoad:       19922.7 (10  0.7%)       19927.0 (10  0.6%)   0.0%
            Box2D:       37498.3 (10  1.7%)       36237.3 (10  1.4%)  -3.4%
             zlib:       85531.4 (10  0.5%)       85366.3 (10  0.7%)  -0.2%
       Typescript:       28189.7 (10  3.0%)       27250.3 (10  2.6%)  -3.3%
Score (version 9):       27854.1 (10  0.8%)       27803.5 (10  0.4%)  -0.2%

Comment 25

[Jon Coppeard (:jonco)]

I wasn't able to show that this gained us any performance over what we have already.

Comment 26

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.