Closed Bug 882736 Opened 6 years ago Closed 6 years ago

IM: Add support for JSOP_INITPROP_GETTER and JSOP_INITPROP_SETTER

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: till, Assigned: jandem)

References

Details

(Whiteboard: [js:t])

Attachments

(1 file)

Running the following script takes about 85ms for me:

instance = {
	field : 1,
	// get prop() this.field,
	method : function() this.field
};
//Object.defineProperty(instance, "prop", {get : function() this.field});
var t1 = Date.now();

for (i = 0; i < 50000000; i++)
	instance.field;

print(Date.now() - t1);

As soon as I remove the comment from the third line, adding a getter to the object but not using it in any way, that increases to about 1170ms. That's a slowdown of ~12.8x.

Removing the comment from line 6, otoh, doesn't affect performance at all.

Maybe we're forcing objects into dictionary mode when parsing the literal shorthand?
The 'get prop' will inhibit Ion compilation, and when that is commented out Ion should be able to loop hoist the instance.field access so it is running an empty loop body.
(In reply to Brian Hackett (:bhackett) from comment #1)
> The 'get prop' will inhibit Ion compilation

Mmh, but why does the literal shorthand way of defining a getter do that, while defineProperty doesn't? Shouldn't the result be the same as far as the jit is concerned?

> and when that is commented out
> Ion should be able to loop hoist the instance.field access so it is running
> an empty loop body.

You're right. I wrongly reduced a more complex case to this. Here's a version that forces the loop body to be interpreted:

var instance = {field : 1,
	get prop() this.field
};
var t1 = Date.now();
for (var i = 0, result = 0; i < 50000000; i++)
	result += instance.field;
print(Date.now() - t1, result);

Turns out the comparison is even worse, now that we have to do more stuff, while still not running the jit: 1690ms vs 98ms with the second line commented out.
(In reply to Till Schneidereit [:till] from comment #2)
> Mmh, but why does the literal shorthand way of defining a getter do that,
> while defineProperty doesn't? Shouldn't the result be the same as far as the
> jit is concerned?

The shorthand uses an opcode, JSOP_INITPROP_GETTER or somesuch, that isn't handled by IonBuilder, causing Ion compilation to abort.
Summary: Defining getters using the object literal shorthand slows down property access dramatically → IM: Add support for JSOP_INITPROP_GETTER
Whiteboard: [js:t]
Might just as well do both at the same time, I suppose.
Summary: IM: Add support for JSOP_INITPROP_GETTER → IM: Add support for JSOP_INITPROP_GETTER and JSOP_INITPROP_SETTER
Blocks: 885526
Attached patch PatchSplinter Review
Also changes InitGetterSetterOperation to take the value as HandleObject instead of HandleValue.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #782662 - Flags: review?(bhackett1024)
Attachment #782662 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/25e81fe30063
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.