816439 - Slower prototype inheritance because of variable indirection

Status: RESOLVED INACTIVE

(Core :: JavaScript Engine, defect)

Description

[André Cruz]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.11 (KHTML, like Gecko) Chrome/23.0.1271.91 Safari/537.11

Steps to reproduce:

I have two functions to create prototypal inheritance: 

    function inheritPrototypeSlow(A, B) {
        var EmptyFunc = function () {};
        EmptyFunc.prototype = B.prototype;
        var temp = new EmptyFunc();
        A.prototype = temp;
        A.prototype.constructor = A;
    }
    function inheritPrototypeFast(A, B) {
        var EmptyFunc = function () {};
        EmptyFunc.prototype = B.prototype;
        A.prototype = new EmptyFunc();
        A.prototype.constructor = A;
    }

The first one is slower because of the indirection added by the extra var temp.
Here's the jsperf: http://jsperf.com/strange-behavior-prototype-inherit

This happens at least in versions greater than 17.


Actual results:

First one is much slower because of var temp..


Expected results:

Be equal in terms of performance.

Comment 1

[André Cruz]

Typo, it happens in FF >= 17

Comment 2

[André Cruz]

Is worth mentioning that I'm not measuring the actual inheritPrototypeSlow() and inheritPrototypeFast() performance but I'm measuring the initializing of classes using both functions. Please see the jsperf for the full code.

Comment 3

[André Cruz]

Some additional details.
If I change the code to:

  function inheritPrototypeFast2(A, B) {
        var EmptyFunc = function () {};
        EmptyFunc.prototype = B.prototype;
        var temp = A.prototype = new EmptyFunc();
        A.prototype.constructor = A;
    }

Then the code is as fast as inheritPrototypeFast.

It sounds to me that firefox is making some optimizations based in typical code? Is this related to Kraken?

Comment 4

[Loic]

Regression range:

m-c
good=2012-07-17
bad=2012-07-18
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c918ff2f0994&tochange=ae22909cef5a

m-i
good=2012-07-16
bad=2012-07-17
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=082542b01af8&tochange=0030a8043e4a

Comment 5

[Boris Zbarsky [:bzbarsky]]

This has the look of TI failing somehow, at first glance.

In that range, bug 773731 is interesting.

Loic, did that change slow down the "slow" version or speed up the "fast" one?

Comment 6

[Boris Zbarsky [:bzbarsky]]

That said, on Mac in Firefox 17 I see the "slow" and "fast" versions in th jsperf run at about the same speed in a 64-bit build.  In a 32-bit build, "slow" is the same as in the 64-bit build but "fast" is 50% faster (!).  So something _really_ weird is going on here.

Comment 7

[Loic]

(In reply to Boris Zbarsky (:bz) from comment #5)
> Loic, did that change slow down the "slow" version or speed up the "fast"
> one?

It speeds up the fast version, I think.

Example of test:

good=2012-07-17
slow: 10,971,846 ops/sec
fast: 10,817,095 ops/sec

bad=2012-07-18
slow: 10,345,576 ops/sec (68% slower)
fast: 32,879,815 ops/sec

Comment 8

[Boris Zbarsky [:bzbarsky]]

And in a 32-bit build, Firefox 16 runs both testcases at the "slow" speed.

So it looks like bug 773731 just sped up the "fast" version but not the "slow" one.  Which is not surprising for a pattern-matching-based optimization.

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

David - is this regression something we should track for forward fix or backout of bug 773721?

Comment 10

[Boris Zbarsky [:bzbarsky]]

There's absolutely no reason to back out bug 773721.  All it did was make one of the testcases in this bug faster without speeding up the other one.  This bug is more or less about speeding up the other one, as far as I can tell.  Plus the lack of speedup on 64-bit, I guess.

Now that we understand that, I don't think this needs tracking.

Comment 11

[André Cruz]

Any ETA on this?

Comment 12

[Kannan Vijayan [:djvj]]

Speeding this up in both cases is a matter of getting TI to be more general about recognizing these sites and assigning new type-objects appropriately.

I'm not sure how difficult that would be, or if it's even realistically doable.  Pulling bhackett in for comment.

Comment 13

[Brian Hackett [Laid off!]]

This could be fixed.  Currently, we decide at the point of an object's allocation whether to give it a unique type, which enables the optimizations seen in the faster version (this is done by sniffing the bytecode to look for a following .prototype assignment).  If this decision were relaxed, so that an object's type could change *after* it has been created, then we could just give it a unique type at the point is is assigned to some .prototype.  Doing that requires that type sets which contain the object be updated to reflect the object's new type, which is only really possible to do if the object has not escaped to the heap.  Determining *that* property requires escape analysis, but we already do variants of escape analysis using SSA information for analyzing arguments usage and scripts called with 'new'.  If the object is never assigned to a heap location at any time within the constructor function or within its caller (other than the assignment to .prototype) then the object's type can be changed to a unique type at the point of the .prototype assignment.

Comment 14

[André Cruz]

I hope that this doesn't take much to solve.

I'm the creator of dejavu, an OOP library for javascript.
This along with the issue https://bugzilla.mozilla.org/show_bug.cgi?id=610296 are causing it to slowdown in Firefox.
It has the higher performance in every other browser except Firefox. Please see: http://jsperf.com/oop-benchmark/79

I created this issue because I traced the problem and caught this up.

The issue linked above about closures is what is making Firefox slower overall. Almost every script/library is wrapped in closures (jquery, jquery plugins, code written in AMD modules, etc..). Just by writing code in a closure turns it 30% slower!! It's amazing that the issue has been there for more than 2 years and is yet not solved. Is the same going to happen to this issue? That's a very frustrating. Often people find issues in Firefox and, because of the review/fix process being so slow, they simply don't report it.

Comment 15

[Boris Zbarsky [:bzbarsky]]

André, this bug can't be causing a slowdown compared to before this bug appeared, because the "slow" case is the same before and after, right?  It's just that the "fast" case got faster...

As for time to resolve issues, it really depends on the issue.  If something is simple to fix it will obviously take less time to fix; if it requires rearchitecting a lot of things (like bug 610296) it will take longer.  Please do keep reporting bugs, and encourage others to.  Some will be fixed quickly, some will not, but that's just a matter of limited resources, just like for every browser.  I understand that doesn't reduce the frustration for you, unfortunately.

Comment 16

[André Cruz]

Boris, the issue that is causing the overall slowdown is not this one but bug 610296. While this issue should be fixed, it's easy to circuvent. I've never stopped reporting bugs, but I know of people that simply give up having faith in fixing process.

Sorry If I was too aggressive, but wanted to express my feelings.

Comment 17

[Kannan Vijayan [:djvj]]

(In reply to André Cruz from comment #14)
> I hope that this doesn't take much to solve.
> 
> I'm the creator of dejavu, an OOP library for javascript.
> This along with the issue
> https://bugzilla.mozilla.org/show_bug.cgi?id=610296 are causing it to
> slowdown in Firefox.
> It has the higher performance in every other browser except Firefox. Please
> see: http://jsperf.com/oop-benchmark/79
> 
> I created this issue because I traced the problem and caught this up.
> 
> The issue linked above about closures is what is making Firefox slower
> overall. Almost every script/library is wrapped in closures (jquery, jquery
> plugins, code written in AMD modules, etc..). Just by writing code in a
> closure turns it 30% slower!! It's amazing that the issue has been there for
> more than 2 years and is yet not solved. Is the same going to happen to this
> issue? That's a very frustrating. Often people find issues in Firefox and,
> because of the review/fix process being so slow, they simply don't report it.

Please don't be discouraged.  The performance difference you are seeing is due to optimizing a common, easy to recognize pattern.

It's unfortunate that this particular optimization leads to a performance cliff when the easily recognized pattern is not present.  While Brian's suggestion is certainly possible to implement, it's also rather complex and time consuming, and would still present a performance cliff (e.g. as soon as the JS code defeats any rudimentary escape analysis we implement - let's say you make a non-inlinable call to generate "new EmptyFunc()" instead of doing it directly within the "inheritPrototype" function).

A more resilient approach to improving performance in this case (and other similar cases), is to improve our ability to monitor shapes at use-sites and use that information in generating JIT code (as opposed to creatively assigning hidden types to objects at creation time).  That approach would improve performance across a wide variety of cases, and would work regardless of the complexity in the code that creates new javascript constructors.

That is being worked on, but is more general in scope.  Thank you for posting the bug, and please continue to post bugs as you encounter them.