Apparently, we only optimize scripted getters/setters if they're on the proto chain, not on the receiver. We do handle own native getters/setters...

Ah, interesting. JSIL actually uses this a lot. I wonder if the same is true for V8...

V8 seems to optimize these fine.

The patch in bug 1129382 adds Ion ICs for scripted getters/setters and these will also work for own getters/setters. The Baseline ICs and IonBuilder paths should also support own getters/setters, but it's a good start.

This just renames Baseline's GetProp_CallScripted stub to GetProp_CallScriptedPrototype, this nicely matches CallNativePrototype. No change in behavior.

Comment on attachment 8565409 [details] [diff] [review] Part 1 - Rename GetProp_CallScripted to GetProp_CallScriptedPrototype Review of attachment 8565409 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, this probably should have happened when I did the native side refactor, for consistency. Good to see it happen now.

Actually, I no longer think having separate stubs for own-getter and proto-getter is the way to go. It does save some memory but it adds a lot of extra code and complexity and I don't think it's worth it. This patch merges GetProp_CallNative and GetProp_CallNativePrototype into GetProp_CallNative, and makes GetProp_CallScripted handle own getters. It removes at least 150 lines of code. Also note that we currently only handle setters on the prototype, and we'd need the same inheritance structure there for scripted/native setters. With this approach, handling own setters should be a lot simpler.

Comment on attachment 8572598 [details] [diff] [review] Part 1 - Refactor + handle own scripted getters Review of attachment 8572598 [details] [diff] [review]: ----------------------------------------------------------------- This is a pretty reduction. r=me Thanks for adding the various asserts about key firlds, and cleaning up Native code as well.

Part 1, own scripted getters: https://hg.mozilla.org/integration/mozilla-inbound/rev/83fabedf6d3a Now I just need to do the same for setters.

(In reply to Jan de Mooij [:jandem] from comment #8) > Now I just need to do the same for setters. Doing this in bug 1157231, to make release tracking easier (the patch here landed in 39).