New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
HW: All
NextAction: ----
OS: All
Priority: 2
Type: FeatureRequest



Sign in to add a comment

Improve performance of Object.is

Project Member Reported by bmeu...@chromium.org, Oct 4 2017

Issue description

The Object.is builtin that was introduced with ES2015 (https://tc39.github.io/ecma262/#sec-object.is) acts as an entry point to the abstract operation SameValue (https://tc39.github.io/ecma262/#sec-samevalue), which provides a strict equality check that also properly distinguishes 0 and -0, and identifies NaNs. It provides an easy way to check arbitrary values for -0, i.e. as used by Node in formatNumber (https://github.com/nodejs/node/blob/306391c/lib/util.js#L619).

Unfortunately Object.is is currently implemented as C++ builtin, despite having all the logic for a fast CSA based version in place already. Also TurboFan doesn't know anything about Object.is, not even for the fairly simple cases where one side is known to be -0 or NaN (the interesting cases), or where both inputs refer to the same SSA value, which can be constant-folded to true easily.

The simple benchmark below highlights these points and compares Object.is to strict equal:

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

function testNumberIsMinusZero(o) {
  let result = 0;
  o = +o;
  for (let i = 0; i < N; ++i) {
    if (Object.is(o, -0)) result++;
  }
  return result;
}

function testObjectIsMinusZero(o) {
  let result = 0;
  for (let i = 0; i < N; ++i) {
    if (Object.is(o, -0)) result++;
  }
  return result;
}

function testObjectIsNaN(o) {
  let result = 0;
  for (let i = 0; i < N; ++i) {
    if (Object.is(o, NaN)) result++;
  }
  return result;
}

function testObjectIsSame(o) {
  let result = 0;
  for (let i = 0; i < N; ++i) {
    if (Object.is(o, o)) result++;
  }
  return result;
}

function testStrictEqualSame(o) {
  let result = 0;
  for (let i = 0; i < N; ++i) {
    if (o === o) result++;
  }
  return result;
}

const N = 1e7;
const OBJS = [-0, 0, undefined, NaN, null, '', {}, []];
const TESTS = [
    testNumberIsMinusZero,
    testObjectIsMinusZero,
    testObjectIsNaN,
    testObjectIsSame,
    testStrictEqualSame
];

function test(fn) {
  var result;
  for (var o of OBJS) result = fn(o);
  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 one V8 6.1 and ToT shows that there's a lot of potential for optimization here:

=====================================================================
$ ./d8-6.1.534.15 bench-objectis.js
testNumberIsMinusZero: 991 ms.
testObjectIsMinusZero: 853 ms.
testObjectIsNaN: 914 ms.
testObjectIsSame: 828 ms.
testStrictEqualSame: 109 ms.
$ ./d8-master bench-objectis.js
testNumberIsMinusZero: 1000 ms.
testObjectIsMinusZero: 929 ms.
testObjectIsNaN: 954 ms.
testObjectIsSame: 793 ms.
testStrictEqualSame: 104 ms.
=====================================================================

 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d4da17c6e386819426d7929fa0b6287a4f62f002

commit d4da17c6e386819426d7929fa0b6287a4f62f002
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Wed Oct 04 06:58:36 2017

[es2015] Optimize Object.is baseline and interesting cases.

The Object.is builtin provides an entry point to the abstract operation
SameValue, which properly distinguishes -0 and 0, and also identifies
NaNs. Most of the time you don't need these, but rather just regular
strict equality, but when you do, Object.is(o, -0) is the most readable
way to check for minus zero.

This is for example used in Node.js by formatNumber to properly print -0
for negative zero. However since the builtin thus far implemented as C++
builtin and TurboFan didn't know anything about it, Node.js considering
to go with a more performant, less readable version (which also makes
assumptions about the input value) in

  https://github.com/nodejs/node/pull/15726

until the performance of Object.is will be on par (so hopefully we can
go back to Object.is in Node 9).

This CL ports the baseline implementation of Object.is to CSA, which
is pretty straight-forward since SameValue is already available in
CodeStubAssembler, and inlines a few interesting cases into TurboFan,
i.e. comparing same SSA node, and checking for -0 and NaN explicitly.

On the micro-benchmarks we go from

  testNumberIsMinusZero: 1000 ms.
  testObjectIsMinusZero: 929 ms.
  testObjectIsNaN: 954 ms.
  testObjectIsSame: 793 ms.
  testStrictEqualSame: 104 ms.

to

  testNumberIsMinusZero: 89 ms.
  testObjectIsMinusZero: 88 ms.
  testObjectIsNaN: 88 ms.
  testObjectIsSame: 86 ms.
  testStrictEqualSame: 105 ms.

which is a nice 10x to 11x improvement and brings Object.is on par with
strict equality for most cases.

Drive-by-fix: Also refactor and optimize the SameValue check in the
CodeStubAssembler to avoid code bloat (by not inlining StrictEqual
into every user of SameValue, and also avoiding useless checks).

Bug:  v8:6882 
Change-Id: Ibffd8c36511f219fcce0d89ed4e1073f5d6c6344
Reviewed-on: https://chromium-review.googlesource.com/700254
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48275}
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/builtins/builtins-definitions.h
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/builtins/builtins-object-gen.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/builtins/builtins-object.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/builtins/builtins-promise-gen.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/builtins/builtins-proxy-gen.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/builtins/builtins-regexp-gen.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/code-stub-assembler.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/code-stub-assembler.h
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/graph-assembler.h
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/js-builtin-reducer.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/js-builtin-reducer.h
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/opcodes.h
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/simplified-lowering.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/simplified-operator.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/simplified-operator.h
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/typer.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/compiler/verifier.cc
[modify] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/src/objects.h
[add] https://crrev.com/d4da17c6e386819426d7929fa0b6287a4f62f002/test/mjsunit/compiler/object-is.js

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c77dfda0ac13080f8b8987930277b20d158acd45

commit c77dfda0ac13080f8b8987930277b20d158acd45
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Fri Oct 06 07:56:31 2017

[turbofan] Properly check call arity for Object.is(o,o).

Bug:  chromium:771971 ,  v8:6882 
Change-Id: Id1a602306bc89a5f96e180f70d6f713015d2dbb6
Reviewed-on: https://chromium-review.googlesource.com/702834
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48329}
[modify] https://crrev.com/c77dfda0ac13080f8b8987930277b20d158acd45/src/compiler/js-builtin-reducer.cc
[add] https://crrev.com/c77dfda0ac13080f8b8987930277b20d158acd45/test/mjsunit/regress/regress-crbug-771971.js

Comment 4 by adamk@chromium.org, Oct 27 2017

Cc: adamk@chromium.org
 Issue 7007  has been merged into this issue.

Sign in to add a comment