New issue
Advanced search Search tips
Starred by 3 users
Status: Fixed
Owner:
Closed: Sep 7
Cc:
Components:
HW: All
OS: All
Priority: 1
Type: Bug
ES5



Sign in to add a comment
Significant regression in for-in performance (with I+TF)
Project Member Reported by bmeu...@chromium.org, Aug 14 Back to list
As highlighted in https://twitter.com/lukeed05/status/896815029239947264 and previously described in the long blog post from nearForm on the I+TF launch (https://www.nearform.com/blog/node-js-is-getting-a-new-v8-with-turbofan/), the performance of for-in dropped significantly in some cases. Specifically TurboFan doesn't optimize away Object.prototype.hasOwnProperty calls and doesn't optimize keyed accesses to the enumerated properties anymore (Crankshaft used to do both of these).

Proper micro-benchmark from  https://gist.github.com/bmeurer/6ad8e8ffb36e2a0e70a68819071d7e7f to illustrate the core issue:

===================================================================================
if (typeof console === 'undefined') console = {log:print};

function forIn(o) {
  var result = 0;
  for (var i in o) {
    result += 1;
  }
  return result;
}

function forInHasOwnProperty(o) {
  var result = 0;
  for (var i in o) {
    if (o.hasOwnProperty(i)) {
      result += 1;
    }
  }
  return result;
}

function forInHasOwnPropertySafe(o) {
  var result = 0;
  for (var i in o) {
    if (Object.prototype.hasOwnProperty.call(o, i)) {
      result += 1;
    }
  }
  return result;
}

function forInSum(o) {
  var result = 0;
  for (var i in o) {
    result += o[i];
  }
  return result;
}

function forInSumSafe(o) {
  var result = 0;
  for (var i in o) {
    if (Object.prototype.hasOwnProperty.call(o, i)) {
      result += o[i];
    }
  }
  return result;
}

var TESTS = [
  forIn,
  forInHasOwnProperty,
  forInHasOwnPropertySafe,
  forInSum,
  forInSumSafe
];
var o = {w:0, x:1, y:2, z:3};

var n = 1e8;

function test(fn) {
  var result;
  for (var i = 0; i < n; ++i) result = fn(o);
  return result;
}

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

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

Numbers regress up to 4.5x from V8 5.8 (latest CS based version) to V8 ToT:

===================================================================================
$ ./d8-5.8.283.38 bench-forin.js
forIn: 1641 ms.
forInHasOwnProperty: 1719 ms.
forInHasOwnPropertySafe: 1802 ms.
forInSum: 2226 ms.
forInSumSafe: 2409 ms.
$ out/Release/d8 bench-forin.js
forIn: 1713 ms.
forInHasOwnProperty: 5417 ms.
forInHasOwnPropertySafe: 5324 ms.
forInSum: 7556 ms.
forInSumSafe: 11067 ms.
===================================================================================

Specifically the forInSumSafe benchmark probably matches the common case (there's even an eslint rule at http://eslint.org/docs/rules/guard-for-in enforcing the guarding with O.p.hasOwnProperty).

Interestingly the traditional benchmarks like Speedometer/React don't seem to be affected by this a lot, although they make use of for-in a lot (see Firefox investigation at https://bugzilla.mozilla.org/show_bug.cgi?id=1356315).
 
Labels: ES5 Hotlist-TurboFan Hotlist-NodeJS TurboFan
Labels: -Priority-1 Priority-2
Cc: cbruni@chromium.org
I have a pending CL (https://chromium-review.googlesource.com/636243) that improves these numbers significantly (above the crankshaft score):

forIn: 1608 ms.
forInHasOwnProperty: 1663 ms.
forInHasOwnPropertySafe: 1716 ms.
forInSum: 2042 ms.
forInSumSafe: 2110 ms.

The CL also improves the score on the string-fasta benchmark by 12%, which is expected. Unfortunately the CL currently suffers from the same deoptimization loop problem that plagued Crankshaft (missing indices cache, which was one of the reason why TurboFan does so much better on Speedometer/React and probably React applications in general). I'll work on that first.
Labels: -Priority-2 Priority-1
Project Member Comment 6 by bugdroid1@chromium.org, Aug 28
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/06753c64eba67975e3f705dc0c4e05a68cc3b82a

commit 06753c64eba67975e3f705dc0c4e05a68cc3b82a
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Mon Aug 28 06:00:47 2017

[turbofan] Optimize O.p.hasOwnProperty inside for-in.

Optimize the common pattern

  for (var i in o) {
    if (Object.prototype.hasOwnProperty.call(o, i)) {
      // do something
    }
  }

which is part of the guard-for-in style in ESLint (see the documentation
at https://eslint.org/docs/rules/guard-for-in for details). This pattern
also shows up in React and Ember applications quite a lot (and is tested
by the appropriate Speedometer benchmarks, although not dominating those
benchmarks, since they spent a lot of time in non-TurboFan'ed code).

This improves the forInHasOwnProperty and forInHasOwnPropertySafe micro-
benchmarks in v8:6702, which look like this

  function forInHasOwnProperty(o) {
    var result = 0;
    for (var i in o) {
      if (o.hasOwnProperty(i)) {
        result += 1;
      }
    }
    return result;
  }

  function forInHasOwnPropertySafe(o) {
    var result = 0;
    for (var i in o) {
      if (Object.prototype.hasOwnProperty.call(o, i)) {
        result += 1;
      }
    }
    return result;
  }

by around 4x and allows for additional optimizations in the future, by
also elimiating the megamorphic load when accessing the enumerated
properties.

This changes the interpreter ForInNext bytecode to collect more precise
feedback about the for-in state, which now consists of three individual
states: UNINITIALIZED, MEGAMORPHIC and GENERIC. The MEGAMORPHIC state
means that the ForInNext has only seen objects with a usable enum cache
thus far, whereas GENERIC means that we have seen some slow-mode for..in
objects as well.

R=jarin@chromium.org

Bug:  v8:6702 
Change-Id: Ibcd75ea9b58c3b4f9219f11bc37eb04a2b985604
Reviewed-on: https://chromium-review.googlesource.com/636964
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47632}
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/bootstrapper.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/builtins/builtins-definitions.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/builtins/builtins-object-gen.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/access-builder.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/access-builder.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/bytecode-graph-builder.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/bytecode-graph-builder.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/js-call-reducer.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/js-type-hint-lowering.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/js-type-hint-lowering.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/js-typed-lowering.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/opcodes.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/simplified-lowering.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/simplified-operator.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/simplified-operator.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/typer.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/compiler/verifier.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/debug/debug-evaluate.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/deoptimize-reason.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/feedback-vector-inl.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/feedback-vector.cc
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/feedback-vector.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/heap-symbols.h
[modify] https://crrev.com/06753c64eba67975e3f705dc0c4e05a68cc3b82a/src/interpreter/interpreter-generator.cc

Project Member Comment 7 by bugdroid1@chromium.org, Aug 30
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/562663d545bc1838ce1712d49fa389358985d706

commit 562663d545bc1838ce1712d49fa389358985d706
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Wed Aug 30 05:29:16 2017

[cleanup] Unify enum cache handling.

Introduce a proper empty_descriptor_array, which has the proper layout
(length is 2 and the two fields are set properly). Also add a special
EnumCache class and a matching empty_enum_cache. The contract now is
that we only need to check the EnumLength on the map to know whether we
are allowed to use the enum cache. This greatly simplifies the handling
of the enum cache (and also the descriptor arrays), especially for the
future work on optimizing keyed access via the enum cache indices.

Bug:  v8:6702 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I5ef517a3041163cd65ef003f691139ea52233e83
Reviewed-on: https://chromium-review.googlesource.com/641030
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47697}
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/api.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/builtins/builtins-forin-gen.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/builtins/builtins-object-gen.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/compiler/access-builder.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/compiler/access-builder.h
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/compiler/js-type-hint-lowering.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/factory.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/factory.h
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/heap/heap.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/heap/heap.h
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/heap/mark-compact.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/heap/object-stats.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/keys.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/objects-debug.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/objects-inl.h
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/objects.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/objects.h
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/objects/descriptor-array.h
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/runtime/runtime-forin.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/src/value-serializer.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/test/cctest/heap/test-alloc.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/test/cctest/test-api.cc
[modify] https://crrev.com/562663d545bc1838ce1712d49fa389358985d706/tools/v8heapconst.py

Project Member Comment 8 by bugdroid1@chromium.org, Aug 30
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/8efc5f0425cf9c97ff4a0ac0f285f36422a36bd8

commit 8efc5f0425cf9c97ff4a0ac0f285f36422a36bd8
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Wed Aug 30 05:56:44 2017

[fix] Properly use CHECK instead of DCHECK.

Cleanup for 562663d545bc1838ce1712d49fa389358985d706.

Bug:  v8:6702 
Change-Id: I7fbacbe6e4b52dc56d810cab3123b497329be3ca
Tbr: jarin@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/641874
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47698}
[modify] https://crrev.com/8efc5f0425cf9c97ff4a0ac0f285f36422a36bd8/src/objects-debug.cc

Project Member Comment 9 by bugdroid1@chromium.org, Sep 1
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/f1ec44e2f525434b7af60450aa7678d12b519aee

commit f1ec44e2f525434b7af60450aa7678d12b519aee
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Fri Sep 01 11:27:37 2017

[turbofan] Optimize fast enum cache driven for..in.

This CL adds support to optimize for..in in fast enum-cache mode to the
same degree that it was optimized in Crankshaft, without adding the same
deoptimization loop that Crankshaft had with missing enum cache indices.
That means code like

  for (var k in o) {
    var v = o[k];
    // ...
  }

and code like

  for (var k in o) {
    if (Object.prototype.hasOwnProperty.call(o, k)) {
      var v = o[k];
      // ...
    }
  }

which follows the https://eslint.org/docs/rules/guard-for-in linter
rule, can now utilize the enum cache indices if o has only fast
properties on the receiver, which speeds up the access o[k]
significantly and reduces the pollution of the global megamorphic
stub cache.

For example the micro-benchmark in the tracking  bug v8:6702  now runs
faster than ever before:

 forIn: 1516 ms.
 forInHasOwnProperty: 1674 ms.
 forInHasOwnPropertySafe: 1595 ms.
 forInSum: 2051 ms.
 forInSumSafe: 2215 ms.

Compared to numbers from V8 5.8 which is the last version running with
Crankshaft

 forIn: 1641 ms.
 forInHasOwnProperty: 1719 ms.
 forInHasOwnPropertySafe: 1802 ms.
 forInSum: 2226 ms.
 forInSumSafe: 2409 ms.

and V8 6.0 which is the current stable version with TurboFan:

 forIn: 1713 ms.
 forInHasOwnProperty: 5417 ms.
 forInHasOwnPropertySafe: 5324 ms.
 forInSum: 7556 ms.
 forInSumSafe: 11067 ms.

It also improves the throughput on the string-fasta benchmark by
around 7-10%, and there seems to be a ~5% improvement on the
Speedometer/React benchmark locally.

For this to work, the ForInPrepare bytecode was split into
ForInEnumerate and ForInPrepare, which is very similar to how it was
handled in Fullcodegen initially. In TurboFan we introduce a new
operator LoadFieldByIndex that does the dynamic property load.

This also removes the CheckMapValue operator again in favor of
just using LoadField, ReferenceEqual and CheckIf, which work
automatically with the EscapeAnalysis and the
BranchConditionElimination.

Bug:  v8:6702 
Change-Id: I91235413eea478ba77ace7bd14bb2f62e155dd9a
Reviewed-on: https://chromium-review.googlesource.com/645949
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47768}
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/BUILD.gn
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/ast/ast.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/builtins/builtins-definitions.h
[delete] https://crrev.com/6d249930b37b985d534e9cfcbf0b854701d98935/src/builtins/builtins-forin-gen.cc
[delete] https://crrev.com/6d249930b37b985d534e9cfcbf0b854701d98935/src/builtins/builtins-forin-gen.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/builtins/builtins-internal-gen.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/code-stub-assembler.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/code-stub-assembler.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/access-builder.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/access-builder.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/bytecode-graph-builder.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/bytecode-graph-builder.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/common-operator.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-generic-lowering.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-native-context-specialization.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-operator.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-operator.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-type-hint-lowering.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-type-hint-lowering.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-typed-lowering.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/js-typed-lowering.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/opcodes.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/operator-properties.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/simplified-lowering.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/simplified-operator.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/simplified-operator.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/typer.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/compiler/verifier.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/debug/debug-evaluate.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/feedback-vector-inl.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/feedback-vector.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/feedback-vector.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/globals.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/interpreter/bytecode-array-builder.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/interpreter/bytecode-array-builder.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/interpreter/bytecode-generator.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/interpreter/bytecodes.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/interpreter/interpreter-generator.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/objects-printer.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/runtime/runtime-forin.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/runtime/runtime.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/runtime/runtime.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/type-hints.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/type-hints.h
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/src/v8.gyp
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/test/cctest/interpreter/bytecode_expectations/ForIn.golden
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/test/cctest/interpreter/bytecode_expectations/WideRegisters.golden
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/test/cctest/test-feedback-vector.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/test/unittests/interpreter/bytecode-array-builder-unittest.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/test/unittests/interpreter/bytecode-array-iterator-unittest.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/test/unittests/interpreter/bytecode-array-random-iterator-unittest.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/test/unittests/interpreter/bytecode-array-writer-unittest.cc
[modify] https://crrev.com/f1ec44e2f525434b7af60450aa7678d12b519aee/test/unittests/interpreter/bytecode-decoder-unittest.cc

Project Member Comment 11 by bugdroid1@chromium.org, Sep 7
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/866782e0e068e2a2656b68857dc899435d7cbb45

commit 866782e0e068e2a2656b68857dc899435d7cbb45
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Thu Sep 07 10:43:52 2017

[turbofan] Don't introduce unnecessary map checks.

Introduce NodeProperties::NoObservableSideEffectBetween to check if
there's any observable side effect between two nodes in the effect
chain. Use this to guard the insertion of potentially redundant map
checks in the lowering of Object.prototype.hasOwnProperty and keyed
accesses within a for..in loop. This gives another boost on the for..in
performance front.

Bug:  v8:6702 
Change-Id: I68133f14ad388a1a7422714319c9b323d5cf8bc4
Reviewed-on: https://chromium-review.googlesource.com/654640
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47869}
[modify] https://crrev.com/866782e0e068e2a2656b68857dc899435d7cbb45/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/866782e0e068e2a2656b68857dc899435d7cbb45/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/866782e0e068e2a2656b68857dc899435d7cbb45/src/compiler/js-native-context-specialization.h
[modify] https://crrev.com/866782e0e068e2a2656b68857dc899435d7cbb45/src/compiler/node-properties.cc
[modify] https://crrev.com/866782e0e068e2a2656b68857dc899435d7cbb45/src/compiler/node-properties.h

Status: Fixed
There's a blog post at http://benediktmeurer.de/2017/09/07/restoring-for-in-peak-performance with some details on the story, the fixes and the end result. I consider this fixed, although there's more we could do to further boost for..in in the future.
Sign in to add a comment