Right now Shapes determine getter/setter identity. This works fine for native accessor functions, but breaks down when code uses lambdas like this: for (...) { var o = {}; Object.defineProperty(o, "foo", {get: function() {return i; }}); foo(o); } Here each object will have a different Shape (because the getter lambda is cloned each iteration). This makes it harder to optimize getter/setter calls and it results in a lot of almost-duplicate shapes. The attached micro-benchmark demonstrates this: Safari: 8 ms Chrome: 20 ms Nightly: 221 ms We could remove getter/setter info from Shape (and remove AccessorShape in the process!) and make getter/setter properties slotful: the slot would store a pointer to a (maybe GC allocated) object that stores the getter/setter functions. Shape attributes could be used to distinguish "no getter", "scripted getter", "native getter", etc. An IC stub for scripted getters, for instance, could guard on the Shape and then it knows the slot stores a scripted getter function, so it could then just call that directly. Another benefit: since bug 1295967 shapes are shared across compartments, but sharing doesn't work when getters/setters are involved because the accessor functions are compartment-specific. Once getters/setters are no longer stored in shapes, we could share a lot more shapes across compartments. The main complexity on the JIT side is Ion inlining of scripted/DOM getters/setters. Maybe we could use TI to avoid an extra guard there.

Brian, what do you think?

Another option is for scripted getters/setters to store the canonical function in the Shape and the actual getter/setter clone in object slots...

There are also some cases like array_length_{getter,setter} where we really don't want to allocate object slots...

(In reply to Jan de Mooij [:jandem] from comment #1) > Brian, what do you think? I think this sounds really good. An alternative design that would address the array length issue would be to keep AccessorShape but only use it for storing raw getter/setter pointers instead of getter/setter objects. This would also allow the getter/setter objects to be stored in slots on the object instead of in a separate heap or GC allocated structure.

(In reply to Brian Hackett (:bhackett) from comment #4) > I think this sounds really good. An alternative design that would address > the array length issue would be to keep AccessorShape but only use it for > storing raw getter/setter pointers instead of getter/setter objects. This > would also allow the getter/setter objects to be stored in slots on the > object instead of in a separate heap or GC allocated structure. Thanks. Note that raw getters/setters could be stored in a PrivateValue... Tom suggested removing array_length_getter/setter and special-casing it; we definitely have some options here. I'm currently working on bug 1389510. That should remove a lot of complexity and perf overhead in this area and is something I wanted to do for a while.

After thinking about this a bit more, the simplest option might be to only change things for scripted accessors: in that case store the canonical JSFunction(s) in the Shape and the actual JSFunction(s) in object slots. Moving everything out of the Shape is appealing, but I don't like the extra guards when calling/inlining native/DOM accessors.