JM: weird performance numbers for constructor/apply

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jandem, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

7 years ago
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.
Blocks: 578133
(Reporter)

Comment 1

7 years ago
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?
(Reporter)

Comment 2

7 years ago
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.
(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.
It's not just microbenchmarks, people use empty functions for "base classes" or placeholder event receivers in frameworks. Do not disrespect the degenerate case.
Filed bug 595391.
(Reporter)

Comment 6

7 years ago
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?
(Reporter)

Updated

7 years ago
Depends on: 595391, 595884
(Reporter)

Updated

7 years ago
Depends on: 595902
(Reporter)

Comment 7

7 years ago
All bugs blocking this one are fixed now, and I can't reproduce the issues in comment 0 anymore.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.