New issue
Advanced search Search tips
Starred by 1 user
Status: Fixed
Owner:
Closed: Oct 23
Cc:
Components:
HW: All
OS: All
Priority: 1
Type: Bug
ES5

Blocking:
issue 6936



Sign in to add a comment
Instanceof slow-path hit a lot in uglify-es benchmark
Project Member Reported by bmeu...@chromium.org, Oct 20 Back to list
Looking at the profile for the uglify-es test in the web-tooling-benchmark, we see that we spend a significant amount of time in the instanceof slow-paths, especially from optimized code. See the attached picture.
 
wtb-uglify-es.js
3.0 MB View Download
wtb-uglify-es-instanceof.png
117 KB View Download
In the particular case of the benchmark it seems that the context specialization doesn't kick in because the instanceof is inside a function that has many closures. Simplified example:

===========================================
const A = class A {}

const makeFoo = () => x => x instanceof A;
(makeFoo())(1);
const foo = makeFoo();

console.log(foo(1));
console.log(foo(2));
%OptimizeFunctionOnNextCall(foo);
console.log(foo(3));
===========================================

Remove the (makeFoo())(1) and the instanceof is fully inlined into foo afterwards. So to address this odd performance cliff, we need to collect feedback about the right-hand side of instanceof, an INSTANCEOF_IC, which now fairly straight-forward with only Ignition and TurboFan around.
There are indeed many cases of instanceof that weren't covered by the fast-path so far. The following micro-benchmark illustrates that:

=================< bench-instanceof.js >===============================
if (typeof console === 'undefined') console = {log:print};

const N = 1e7;
const TESTS = [];

class A {};
const a = new A;

(function() {
  const Aconst = A;
  function makeInstanceofMultiClosure() {
    return function instanceofMultiClosure(a, ignored) {
      return a instanceof A;
    }
  }
  makeInstanceofMultiClosure();  // throw away closure
  var instanceofMultiClosure = makeInstanceofMultiClosure();

  function instanceofSingleClosure_Const(a, ignored) {
    return a instanceof Aconst;
  }

  function instanceofSingleClosure_Class(a, ignored) {
    return a instanceof A;
  }

  function instanceofParameter(a, A) {
    return a instanceof A;
  }

  TESTS.push(
      instanceofSingleClosure_Const,
      instanceofSingleClosure_Class,
      instanceofMultiClosure,
      instanceofParameter);
})();

function test(fn) {
  var result;
  for (var i = 0; i < N; i += 1) result = fn(a, A);
  return result;
}

for (var j = 0; j < TESTS.length; ++j) {
  test(TESTS[j]);
}

for (var j = 0; j < TESTS.length; ++j) {
  var startTime = Date.now();
  test(TESTS[j]);
  console.log(TESTS[j].name + ':', (Date.now() - startTime), 'ms.');
}
=======================================================================

Running this on V8 ToT the results are mixed:

=======================================================================
instanceofSingleClosure_Const: 69 ms.
instanceofSingleClosure_Class: 246 ms.
instanceofMultiClosure: 246 ms.
instanceofParameter: 246 ms.
=======================================================================

You get 3.6x slow-down if you miss the fast-path, and it's easy to miss, and probably happens quite often. So we should widen the fast-path here, and have a dedicated InstanceOfIC to do that.

Project Member Comment 3 by bugdroid1@chromium.org, Oct 23
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/bcee140617fe90e8c354394216225615b9558113

commit bcee140617fe90e8c354394216225615b9558113
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Mon Oct 23 10:15:36 2017

[turbofan] Introduce InstanceOfIC to collect rhs feedback.

This adds a new InstanceOfIC where the TestInstanceOf bytecode collects
constant feedback about the right-hand side of instanceof operators,
including both JSFunction and JSBoundFunction instances. TurboFan then
uses the feedback to optimize instanceof in places where the right-hand
side is not a known constant (known to TurboFan).

This addresses the odd performance cliff that we see with instanceof in
functions with multiple closures. It was discovered as one of the main
bottlenecks on the uglify-es test in the web-tooling-benchmark. The
uglify-es test (run in separation) is ~18% faster with this change.

On the micro-benchmark in the tracking bug we go from

  instanceofSingleClosure_Const: 69 ms.
  instanceofSingleClosure_Class: 246 ms.
  instanceofMultiClosure: 246 ms.
  instanceofParameter: 246 ms.

to

  instanceofSingleClosure_Const: 70 ms.
  instanceofSingleClosure_Class: 75 ms.
  instanceofMultiClosure: 76 ms.
  instanceofParameter: 73 ms.

boosting performance by roughly 3.6x and thus effectively removing the
performance cliff around instanceof.

Bug: v8:6936,  v8:6971 
Change-Id: Ib88dbb9eaef9cafa4a0e260fbbde73427a54046e
Reviewed-on: https://chromium-review.googlesource.com/730686
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48820}
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/compiler/bytecode-graph-builder.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/compiler/js-operator.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/compiler/js-operator.h
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/compiler/js-type-hint-lowering.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/compiler/property-access-builder.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/compiler/property-access-builder.h
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/feedback-vector-inl.h
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/feedback-vector.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/feedback-vector.h
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/interpreter/bytecode-array-builder.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/interpreter/bytecode-generator.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/interpreter/bytecodes.h
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/interpreter/interpreter-assembler.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/interpreter/interpreter-assembler.h
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/interpreter/interpreter-generator.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/src/objects-printer.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/test/cctest/interpreter/bytecode_expectations/IfConditions.golden
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/test/cctest/interpreter/test-interpreter.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/test/mjsunit/compiler/instanceof.js
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/test/unittests/compiler/typer-unittest.cc
[modify] https://crrev.com/bcee140617fe90e8c354394216225615b9558113/test/unittests/interpreter/bytecode-array-builder-unittest.cc

Status: Fixed
Sign in to add a comment