bz just noticed that for CSS2Properties.prototype we have 0.03 MB of BaseShapes and 0.02 MB of shapes. Right now BaseShape stores the getter/setter info, so each accessor property gets its own base shape. BaseShape has a lot of other fields, most of them redundant for objects with many accessors: clasp, parent, metadata, compartment, &c. It might be worth adding a new class, AccessorShape, that inherits from Shape and stores the getter/setter bits. Then we no longer need separate BaseShapes for those, their shapes just become a bit fatter. I just instrumented a browser and about 60% of BaseShapes are for accessor properties. This scheme could end up being less efficient if there are many shapes with the same getter/setter/clasp/compartment, as we will no longer share the getter/setter part and have slightly fatter shapes. I'll try to get some data on this.

I have some WIP patches for this. Below are some measurements on 32-bit OS X. For the notorious CSS2PropertiesPrototype object mentioned in comment 0: Before: ─33,752 B (00.02%) -- class(CSS2PropertiesPrototype) ├──33,672 B (00.02%) -- shapes │ ├──29,544 B (00.02%) -- gc-heap │ │ ├──18,480 B (00.01%) ── base │ │ └──11,064 B (00.01%) ── tree │ └───4,128 B (00.00%) ── malloc-heap/tree-tables After: ─18,976 B (00.01%) -- class(CSS2PropertiesPrototype) ├──18,896 B (00.01%) -- shapes │ ├──14,768 B (00.01%) -- gc-heap │ │ ├──14,672 B (00.01%) ── tree │ │ └──────96 B (00.00%) ── base │ └───4,128 B (00.00%) ── malloc-heap/tree-tables The number of BaseShapes goes from 462 -> 3. Although the shapes become a bit bigger, the gc-heap size is more than halved. Some browser numbers (after opening gmail.com, a Facebook page and bing.com/maps in different tabs and waiting 15 seconds): Before: shapes gc-heap: 10.02 MB, 10.25 MB, 10.25 MB tree : 5.59 MB, 5.67 MB, 5.66 MB base : 2.86 MB, 2.96 MB, 2.97 MB dict : 1.58 MB, 1.62 MB, 1.62 MB After: shapes gc-heap: 8.87 MB, 8.88 MB, 8.80 MB tree : 5.88 MB, 5.92 MB, 5.85 MB base : 1.30 MB, 1.29 MB, 1.28 MB dict : 1.69 MB, 1.68 MB, 1.67 MB compartment-tables before: 2.49 MB, 2.53 MB, 2.51 MB compartment-tables after : 1.82 MB, 1.83 MB, 1.74 MB unused-gc-things before: 13.22 MB, 12.93 MB, 12.88 MB unused-gc-things after : 11.90 MB, 13.12 MB, 12.26 MB So we eliminated most BaseShapes and the compartment tables are also a lot smaller.

This patch works as described in comment 0. See comment 1 for the results. Initially I tried moving the getter/setter methods from Shape to AccessorShape, but this got pretty awkward and verbose. The current patch keeps them as Shape methods, so that the Shape/AccessorShape split is an implementation detail and consumers don't have to know about this.

Comment on attachment 8500531 [details] [diff] [review] Patch Hrm, Try pointed out an issue with this patch (that I should have realized before): StackShape::hash() now includes the getter + setter. If one of those is an object allocated in the nursery and is moved on GC, we need to rekey all KidsHash tables that contain this shape. Unfortunately I don't really have a good fix for this. If we don't include the getter/setter in the hash, there will be a lot more collisions I think. Will investigate more tomorrow.

This version fixes the Generational GC issue in comment 3, by using a custom post barrier that updates the shape's hash when the getter/setter moves. Try is green now. The patch also makes some changes for Compacting GC (now passes jit-tests with CGC as well).

Comment on attachment 8501596 [details] [diff] [review] Patch Review of attachment 8501596 [details] [diff] [review]: ----------------------------------------------------------------- Nice! ::: js/src/vm/Shape.cpp @@ +625,5 @@ > uint32_t index; > bool indexed = js_IdIsIndex(id, &index); > > Rooted<UnownedBaseShape*> nbase(cx); > + if (last->matchesGetterSetter(getter, setter) && !indexed) { Since the BaseShape doesn't include getter/setter information at all now, I don't think the matchesGetterSetter part of this test is necessary anymore. ::: js/src/vm/Shape.h @@ +771,5 @@ > * hint for type inference. > */ > OVERWRITTEN = 0x04, > > + ACCESSOR_SHAPE = 0x08, Needs a comment.

For posterity, on AWSY, "JS: After TP5 [+30s]" (Miscellaneous measurements graph) dropped from 99 MB to 92-93 MB. Also some wins on the other graphs, but they are more noisy.

> For posterity, on AWSY, "JS: After TP5 [+30s]" (Miscellaneous measurements > graph) dropped from 99 MB to 92-93 MB. Excellent. It's not easy to get a clear downward signal like that on AWSY :)

Could this change be responsible for build bustage reported in Bug 1081523?

(In reply to Stefan Sitter from comment #10) > Could this change be responsible for build bustage reported in Bug 1081523? Yes, --disable-gcgenerational bustage, sorry. Posted a patch in that bug.