594972 - JM: weird performance numbers for constructor/apply

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Sorry for the vague title.. Consider this snippet (reduced from v8-raytrace):
---------
function Test() {
    this.initialize.apply(this, arguments);
    //this.initialize(arguments[0], arguments[1], this);
}
Test.prototype.initialize = function(x, y)  {
    this.x = x;
    this.y = y;
}

function f() {
    var t0 = new Date;
    var x;
    
    for(var i=0; i<400000; i++) {
        t = new Test(i);
    }
    print(new Date - t0);
}
f(1, 2);
---------
1. This code is about 10x slower in JM than in V8 (part of that is object creation though)

2. Commenting the first line and uncommenting the second line in Test makes V8 and TM slower. JM gets more than 2x faster...

3. Commenting both lines (empty constructor) makes JM slower... TM and V8 get faster (as you would expect)

Maybe there is a perfectly good reason, but just wanted to let you know.

Comment 1

[Jan de Mooij [:jandem]]

I dug a bit further. Problem 3 (empty constructor slower than constructor-with-some-statement like "var x") seems to be caused by these lines in MonoIC.cpp:
----------
if (!script->ncode && !script->isEmpty()) {
  if (mjit::TryCompile(cx, script, fun, scopeChain) == Compile_Error)
    THROWV(NULL);
}
----------
For an empty constructor, script.isEmpty() is true, so the slow path is taken. "var x" makes the script non-empty and it's compiled.

What would be the right fix here?

Comment 2

[Jan de Mooij [:jandem]]

Btw, the same holds for functions. Calling "function f() {}" is almost 2x slower than calling "function() { var x; }" 

I don't know if calling empty functions is common.. But it makes things like measuring call overhead much less reliable.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to comment #2)

We treat the empty script as a gross hack and deoptimize it. That's not the right call - we should remove it, or figure out how to special case it correctly. People microbenchmark empty function calls.

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

It's not just microbenchmarks, people use empty functions for "base classes" or placeholder event receivers in frameworks. Do not disrespect the degenerate case.

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Filed bug 595391.

Comment 6

[Jan de Mooij [:jandem]]

OK, so the other issue is that |this.initialize.apply(this, arguments)| is much slower than |this.initialize(arguments[0], arguments[1], this)|

I measured performance of the benchmark in comment 0 for the following statements (I subtracted with times for "var x" to account for object allocation differences etc.)

JM:
23 ms: this.initialize(0, 1);
38 ms: this.initialize(arguments[0], arguments[1], this);
123 ms: this.initialize.apply(this, null);
140 ms: return arguments;
275 ms: this.initialize.apply(this, arguments);

V8:
6 ms: this.initialize(0, 1);
58 ms: this.initialize(arguments[0], arguments[1], this);
22 ms: this.initialize.apply(this, null);
7 ms: return arguments;
20 ms: this.initialize.apply(this, arguments);

So apply clearly has some overhead (123-38=85 ms). But most of the slowdown comes from JSOP_ARGUMENTS (275-123=152 ms) 

v8-raytrace calls |this.initialize.apply(this, arguments)| 400.000 times, like the benchmark. So bringing that 152 ms. overhead down to ~20 and apply-overhead from 85 to ~20 could win ~200 ms on v8-raytrace for me. At least 150 ms. on awfy.

Will luke's argv work help here, or should we file a bug for JSOP_ARGUMENTS performance?

Comment 7

[Jan de Mooij [:jandem]]

All bugs blocking this one are fixed now, and I can't reproduce the issues in comment 0 anymore.