Getters/setters are currently stored in the shape tree, in `AccessorShape`s. The reasons for doing it this way were mostly historical, as far as I know: we already supported `JSGetterOp` and `JSSetterOp` getters/setters and that code was reused when implementing ES5 getter/setter objects. There are a number of problems with this: * Branchy shape trees when lambdas are used as getters/setters can result in performance cliffs. See the micro-benchmark below for an example of this. * It complicates the shape code because data properties and accessor properties are so different. * Certain changes to the object layout we're considering are not possible. The plan here is as follows: 1. Remove `JSGetterOp/JSSetterOp` (bug 1404885). Today these are only used for `array.length` and `ArgumentsObject`, and I think we can handle these special cases with a special property attribute instead. 2. Add a `GetterSetterPair` GC thing, a tuple holding just the getter/setter objects. Use this in `AccessorShape`. 3. Store `GetterSetterPair` in an object slot as `PrivateGCThingValue`. Remove AccessorShape. This will require an extra CacheIR guard for getter/setter calls. This is likely acceptable compared to the call overhead, and this could be optimized away in the future if needed (since most getters/setters are on prototype objects, we could use fuses to invalidate code when redefining an accessor property). --- Here's a micro-benchmark where a getter property makes accessing a data property much slower (28 ms => 204 ms). ```js function init() { var objs = []; for (var i = 0; i < 128; i++) { objs.push({get foo() {}, x: 1}); } return objs; } function test(objs) { var res = 0; for (var i = 0; i < 10_000_000; i++) { res += objs[i % 128].x; } return res; } var objs = init(); var t = dateNow(); var res = test(objs); print(dateNow() - t); print(res); ```
Bug 1700052 Comment 0 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Getters/setters are currently stored in the shape tree, in `AccessorShape`s. The reasons for doing it this way were mostly historical, as far as I know: we already supported `JSGetterOp` and `JSSetterOp` getters/setters and that code was reused when implementing ES5 getter/setter objects. There are a number of problems with this: * Branchy shape trees when lambdas are used as getters/setters can result in performance cliffs and wasting memory. See the micro-benchmark below for an example of this. * It complicates the shape code because data properties and accessor properties are so different. * Certain changes to the object layout we're considering are not possible. The plan here is as follows: 1. Remove `JSGetterOp/JSSetterOp` (bug 1404885). Today these are only used for `array.length` and `ArgumentsObject`, and I think we can handle these special cases with a special property attribute instead. 2. Add a `GetterSetterPair` GC thing, a tuple holding just the getter/setter objects. Use this in `AccessorShape`. 3. Store `GetterSetterPair` in an object slot as `PrivateGCThingValue`. Remove AccessorShape. This will require an extra CacheIR guard for getter/setter calls. This is likely acceptable compared to the call overhead, and this could be optimized away in the future if needed (since most getters/setters are on prototype objects, we could use fuses to invalidate code when redefining an accessor property). --- Here's a micro-benchmark where a getter property makes accessing a data property much slower (28 ms => 204 ms). ```js function init() { var objs = []; for (var i = 0; i < 128; i++) { objs.push({get foo() {}, x: 1}); } return objs; } function test(objs) { var res = 0; for (var i = 0; i < 10_000_000; i++) { res += objs[i % 128].x; } return res; } var objs = init(); var t = dateNow(); var res = test(objs); print(dateNow() - t); print(res); ```