Behind objects, arenas containing strings and shapes account for the most unused heap space. Shapes are relatively straightforward to compact since they are purely internal to the JS engine.

The main issue with moving shapes is that you can't access any properties on an object whose shape has been moved until the shape pointer has been updated. When we go to update an object we can make sure the its shape is updated before we access any of its properties. This is implemented by calling the trace hook last in JSObject::TraceChildren. However if its trace hook accesses associated objects we still need to make sure their shape pointer is updated (as well as potentially forwarding the object pointer itself). This happens by making MaybeForwarded do this automatically for objects. I changed uses of this for update only in to use IsForwarded/Forwarded in Shape::fixupDictionaryShapeAfterMovingGC. Another wrinkle is that it is better to relocate shapes after objects so that when we call an object's moved hook its shape won't have been updated yet and property access by the hook will still work. Try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3faa7fff5475

Test results from my main profile of about 50 tabs. Unused heap occupied by shapes decreases from ~15MB to 0.3MB for a 293MB heap after compacting. Before: 374.66 MB (100.0%) -- js-main-runtime-gc-heap-committed ├──292.99 MB (78.20%) -- used │ ├──279.39 MB (74.57%) -- gc-things │ │ ├──140.83 MB (37.59%) ── objects │ │ ├───62.69 MB (16.73%) ── shapes │ │ ├───20.59 MB (05.50%) ── scripts │ │ ├───18.12 MB (04.84%) ── lazy-scripts │ │ ├───17.53 MB (04.68%) ── strings │ │ ├───16.08 MB (04.29%) ── object-groups │ │ └────3.54 MB (00.95%) ++ (3 tiny) │ ├────7.55 MB (02.01%) ── chunk-admin │ └────6.06 MB (01.62%) ── arena-admin └───81.66 MB (21.80%) -- unused ├──80.66 MB (21.53%) -- gc-things │ ├──33.45 MB (08.93%) ── objects │ ├──17.99 MB (04.80%) ── strings │ ├──14.96 MB (03.99%) ── shapes │ ├───7.22 MB (01.93%) ── scripts │ ├───3.85 MB (01.03%) ── object-groups │ └───3.19 MB (00.85%) ++ (4 tiny) └───1.00 MB (00.27%) ++ (2 tiny) After: 328.70 MB (100.0%) -- js-main-runtime-gc-heap-committed ├──293.18 MB (89.19%) -- used │ ├──280.49 MB (85.33%) -- gc-things │ │ ├──141.37 MB (43.01%) ── objects │ │ ├───62.77 MB (19.10%) ── shapes │ │ ├───20.63 MB (06.28%) ── scripts │ │ ├───18.11 MB (05.51%) ── lazy-scripts │ │ ├───18.01 MB (05.48%) ── strings │ │ ├───16.10 MB (04.90%) ── object-groups │ │ ├────3.45 MB (01.05%) ── base-shapes │ │ └────0.04 MB (00.01%) ++ (2 tiny) │ ├────7.33 MB (02.23%) ── chunk-admin │ └────5.36 MB (01.63%) ── arena-admin └───35.52 MB (10.81%) -- unused ├──34.52 MB (10.50%) -- gc-things │ ├──17.69 MB (05.38%) ── strings │ ├───7.20 MB (02.19%) ── scripts │ ├───5.78 MB (01.76%) -- (6 tiny) │ │ ├──2.26 MB (00.69%) ── objects │ │ ├──1.88 MB (00.57%) ── lazy-scripts │ │ ├──1.17 MB (00.36%) ── base-shapes │ │ ├──0.29 MB (00.09%) ── shapes │ │ ├──0.17 MB (00.05%) ── jitcode │ │ └──0.00 MB (00.00%) ── symbols │ └───3.86 MB (01.17%) ── object-groups └───1.00 MB (00.30%) ++ (2 tiny)

(In reply to Jon Coppeard (:jonco) from comment #2) > Test results from my main profile of about 50 tabs. Unused heap occupied by > shapes decreases from ~15MB to 0.3MB for a 293MB heap after compacting. <3

It looks like unused objects also dropped from 33 to 2 MB. Is that expected/meaningful?

(In reply to Nicholas Nethercote [:njn] from comment #4) > It looks like unused objects also dropped from 33 to 2 MB. Is that > expected/meaningful? Yes this is expected, it is from the object compaction that we already have.

Comment on attachment 8732258 [details] [diff] [review] compact-shapes Review of attachment 8732258 [details] [diff] [review]: ----------------------------------------------------------------- That was way simpler than I thought it would be! ::: js/src/jsgc.h @@ +1084,5 @@ > } > }; > > +// Functions for checking and updating GC thing pointers that might be have been > +// moved by compacting GC. Overloads are also provided that work with Values. "that might be have been moved" Single space between sentences. @@ +1093,5 @@ > +// Forwarded - return a pointer to the new location of a GC thing given a > +// pointer to old location. > +// > +// MaybeForwarded - used before dereferencing a pointer that may refer to a moved > +// GC thing without updating it. For JSObjects this will also And here. @@ +1160,5 @@ > +inline void > +MakeAccessibleAfterMovingGC(JSObject* obj) { > + if (obj->isNative()) > + obj->as<NativeObject>().updateShapeAfterMovingGC(); > +} This is less ugly than I thought it would be. ::: js/src/jsobj.cpp @@ +3826,5 @@ > } while (false); > } > + > + if (clasp->trace) > + clasp->trace(trc, this); Please make a note as to why this is in a different position than the equivalent code in processMarkStackTop. Or move the one in processMarkStackTop too. I can see this discrepancy being very confusing for future readers. ::: js/src/jspropertytree.cpp @@ +255,5 @@ > if (listpPointsIntoShape) { > // listp points to the parent field of the next shape. > Shape* next = reinterpret_cast<Shape*>(uintptr_t(listp) - offsetof(Shape, parent)); > + if (gc::IsForwarded(next)) > + listp = &gc::Forwarded(next)->parent; Is this only because MaybeForwarded got slower?

(In reply to Terrence Cole [:terrence] from comment #6) > ::: js/src/jspropertytree.cpp > @@ +255,5 @@ > > if (listpPointsIntoShape) { > > // listp points to the parent field of the next shape. > > Shape* next = reinterpret_cast<Shape*>(uintptr_t(listp) - offsetof(Shape, parent)); > > + if (gc::IsForwarded(next)) > > + listp = &gc::Forwarded(next)->parent; > > Is this only because MaybeForwarded got slower? It's to differentiate the use case of IsForwarded/Forwarded for updating vs. MaybeForwarded for touching associated objects.

https://hg.mozilla.org/mozilla-central/rev/e52c7446d8b4 https://hg.mozilla.org/mozilla-central/rev/414abedbd2aa

We saw a pretty clear win on AWSY as well when this landed: > -4.69 MB (100.0%) -- explicit > ├───6.33 MB (-135.00%) ── heap-unclassified > ├──-4.33 MB (92.48%) ++ window-objects > ├──-2.72 MB (58.04%) -- heap-overhead > │ ├──-3.45 MB (73.70%) ── bin-unused > │ ├───0.72 MB (-15.42%) ── page-cache > │ └───0.01 MB (-0.25%) ── bookkeeping > ├──-3.07 MB (65.47%) ++ js-non-window > > -7.41 MB (100.0%) -- js-main-runtime > ├──-5.19 MB (69.99%) ++ zones > ├──-1.33 MB (17.96%) ── runtime > └──-0.89 MB (12.05%) ++ compartments > > -5.37 MB (100.0%) -- js-main-runtime-gc-heap-committed > ├──-4.76 MB (88.63%) -- unused/gc-things > │ ├──-4.84 MB (90.09%) ── shapes > │ └───0.08 MB (-1.46%) ++ (6 tiny) > └──-0.61 MB (11.37%) ++ used In this case explicit and heap-overhead went down as well which is great (previously CGC wins on the js-heap side seemed mostly to shift to heap-overhead).

(In reply to Eric Rahm [:erahm] from comment #11) Great news!