Closed Bug 1128646 Opened 9 years ago Closed 9 years ago

Optimize calls to own property scripted getters/setters

Categories

(Core :: JavaScript Engine: JIT, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39

People

(Reporter: bzbarsky, Assigned: jandem)

References

Details

Attachments

(1 file, 1 obsolete file)

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...
Blocks: 1128157
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.
Depends on: 1129382
Blocks: 626021
This just renames Baseline's GetProp_CallScripted stub to GetProp_CallScriptedPrototype, this nicely matches CallNativePrototype.

No change in behavior.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8565409 - Flags: review?(efaustbmo)
Depends on: 1134142
Depends on: 1134232
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.
Attachment #8565409 - Flags: review?(efaustbmo) → review+
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.
Attachment #8565409 - Attachment is obsolete: true
Attachment #8572598 - Flags: review?(efaustbmo)
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.
Attachment #8572598 - Flags: review?(efaustbmo) → review+
Part 1, own scripted getters:

https://hg.mozilla.org/integration/mozilla-inbound/rev/83fabedf6d3a

Now I just need to do the same for setters.
Keywords: leave-open
Depends on: 1157231
(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).
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: