1168864 - Use mayResolve hook for addprop stubs

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jan de Mooij [:jandem]]

Attachment: Patch

Websites/frameworks often add new properties to functions. Currently we can't inline the addprop operation because JSFunction has a resolve hook.

This patch makes us check for the mayResolve hook I added in bug 1155946, so that we can inline property adds on functions and other objects with resolve hooks.

With this patch + the patch in bug 935932, the micro-benchmark below improves from 1190 ms to 30 ms.

function f() {
    for (var i=0; i<1000000; i++) {
	var res = function() {};
	res.x = 2;
	res.y = 3;
	res.z = 4;

    }
    return res;
}
var t = new Date;
f();
print(new Date - t);

Comment 1

[Boris Zbarsky [:bzbarsky]]

In CheckHasNoSuchProperty, shouldn't that be using curObj instead of obj?

Comment 2

[Brian Hackett [Laid off!]]

Comment on attachment 8611229 [details] [diff] [review]
Patch

Review of attachment 8611229 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with a fix for the bug bz found.

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Not doing reviews right now from comment #1)
> In CheckHasNoSuchProperty, shouldn't that be using curObj instead of obj?

Good catch, thanks!

Not sure how I missed that.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/db3d40e8b4de

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/db3d40e8b4de

Comment 6

[Hannes Verschore [:h4writer]]

Could this have caused and do we want to act on that?

A regression/improvement was reported on AWFY:
- slave: Mac OS X 10.10 32-bit (Mac Pro, shell)
- mode: Ion

Regression(s)/Improvement(s):
- ss: raytrace: 4% (regression)

Recorded range:
- http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b563fdf07f83&tochange=20458c713464

More details: http://arewefastyet.com/regressions/#/regression/1689307

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Followup patch

I added some checks for addProperty hooks, but we actually don't need to check for that one in CheckHasNoSuchProperty because it's only used for x.missingProp or "x in y".

I thought addProperty hooks were uncommon but the ArrayObject class has one.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> I thought addProperty hooks were uncommon

Fwiw, most DOM things have them. :(

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/306758f7b340

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/306758f7b340