2001145 - Optimize spread calls with additional arguments better

Status: NEW

(Core :: JavaScript Engine, task, P2)

Description

[Jan de Mooij [:jandem]]

The Air subtest in JetStream 3 spends a bunch of time in this class method:

    visitArg(index, func, ...args)
    {
        let replacement = func(this._args[index], ...args);
        if (replacement)
            this._args[index] = replacement;
    }

We allocate the rest array for visitArg and then we allocate a second array for the func spread call that has the extra argument. We also have iterator protocol overhead for this.

We're already able to optimize calls with a single spread argument such as f(...rest) using JSOp::OptimizeSpreadCall, and in that case we can avoid both array allocations in Ion. However we're very slow when there's an additional argument.

Below is a micro-benchmark for this. It's about 5x faster if I change the g(1, ...rest) call to g(...rest).

function g(x, y) {
  return x | y;
}
function f(a, ...rest) {
  return g(1, ...rest);
}
function test(...rest) {
  var t = Date.now();
  var res = 0;
  for (var i = 0; i < 10_000_000; i++) {
    res |= f(i, i, 1);
  }
  print(Date.now() - t);
  return res;
}
test();

Comment 1

[Iain Ireland [:iain]]

I've thought for a while that it would be good if we modified JSOp::SpreadCall (and its variants) to have the same argc operand as JSOp::Call, representing the number of additional arguments before a final spread. (Cases like f(...arr, x) or f(...arr1, ...arr2) would still require allocating a temporary array; I think they're significantly less common.)

Then we would "just" have to push that change through all the code that handles spread calls.

Comment 2

[Jan de Mooij [:jandem]]

Yes that's a good idea. I'll mark this js-perf-next because this could be quite impactful.

js-perf-next: Make our optimization for f(...rest) with JSOp::OptimizeSpreadCall also work for f(a, ...rest), f(a, b, ...rest), etc. This requires changing the bytecode emitter (see the JSOp::OptimizeSpreadCall path) and also our JSOp::SpreadCall bytecode ops (see comment 1). I'd start with just the bytecode emitter and the C++ Interpreter, then run tests and the micro-benchmark with --no-blinterp. If that all works and is a speedup, add Baseline Interpreter/JIT support, and then Ion support.