882736 - IM: Add support for JSOP_INITPROP_GETTER and JSOP_INITPROP_SETTER

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Till Schneidereit [:till]]

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?

Comment 1

[Brian Hackett [Laid off!]]

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.

Comment 2

[Till Schneidereit [:till]]

(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.

Comment 3

[Brian Hackett [Laid off!]]

(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.

Comment 4

[Till Schneidereit [:till]]

Might just as well do both at the same time, I suppose.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch

Also changes InitGetterSetterOperation to take the value as HandleObject instead of HandleValue.

Comment 6

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/25e81fe30063

(OS X/Linux Try: https://tbpl.mozilla.org/?tree=Try&rev=3fa5edc48595)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/25e81fe30063