From 2648c8de3033959f1255e8cb8094bbf2ec591b5c Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Mon, 22 May 2017 14:59:10 -0700 Subject: [PATCH] deps: backport 6d38f89d from upstream V8 Original commit message: [turbofan] Boost performance of Array.prototype.shift by 4x. For small arrays, it's way faster to just move the elements instead of doing the fairly complex and heavy-weight left-trimming. Crankshaft has had this optimization for small arrays already; this CL more or less ports this functionality to TurboFan, which yields a 4x speed-up when using shift on small arrays (with up to 16 elements). This should recover some of the regressions reported in the Node.js issues https://github.com/nodejs/node/issues/12657 and discovered for the syncthrough module using https://github.com/mcollina/syncthrough/blob/master/benchmarks/basic.js as benchmark. R=jarin@chromium.org BUG=v8:6376 Review-Url: https://codereview.chromium.org/2874453002 Cr-Commit-Position: refs/heads/master@{#45216} PR-URL: https://github.com/nodejs/node/pull/13162 Reviewed-By: Franziska Hinkelmann Reviewed-By: James M Snell --- deps/v8/include/v8-version.h | 2 +- deps/v8/src/compiler/js-builtin-reducer.cc | 176 +++++++++++++++++++++ deps/v8/src/compiler/js-builtin-reducer.h | 1 + deps/v8/src/crankshaft/hydrogen.cc | 2 +- deps/v8/src/objects.h | 3 + deps/v8/test/mjsunit/array-shift5.js | 50 ++++++ 6 files changed, 232 insertions(+), 2 deletions(-) create mode 100644 deps/v8/test/mjsunit/array-shift5.js diff --git a/deps/v8/include/v8-version.h b/deps/v8/include/v8-version.h index ac423bf90ec201..f0866a2d9b3264 100644 --- a/deps/v8/include/v8-version.h +++ b/deps/v8/include/v8-version.h @@ -11,7 +11,7 @@ #define V8_MAJOR_VERSION 5 #define V8_MINOR_VERSION 8 #define V8_BUILD_NUMBER 283 -#define V8_PATCH_LEVEL 39 +#define V8_PATCH_LEVEL 40 // Use 1 for candidates and 0 otherwise. // (Boolean macro values are not supported by all preprocessors.) diff --git a/deps/v8/src/compiler/js-builtin-reducer.cc b/deps/v8/src/compiler/js-builtin-reducer.cc index 24eb5cea8ec68b..087015b6b1f865 100644 --- a/deps/v8/src/compiler/js-builtin-reducer.cc +++ b/deps/v8/src/compiler/js-builtin-reducer.cc @@ -5,6 +5,7 @@ #include "src/compiler/js-builtin-reducer.h" #include "src/base/bits.h" +#include "src/builtins/builtins-utils.h" #include "src/code-factory.h" #include "src/compilation-dependencies.h" #include "src/compiler/access-builder.h" @@ -908,6 +909,179 @@ Reduction JSBuiltinReducer::ReduceArrayPush(Node* node) { return NoChange(); } +// ES6 section 22.1.3.22 Array.prototype.shift ( ) +Reduction JSBuiltinReducer::ReduceArrayShift(Node* node) { + Node* target = NodeProperties::GetValueInput(node, 0); + Node* receiver = NodeProperties::GetValueInput(node, 1); + Node* context = NodeProperties::GetContextInput(node); + Node* frame_state = NodeProperties::GetFrameStateInput(node); + Node* effect = NodeProperties::GetEffectInput(node); + Node* control = NodeProperties::GetControlInput(node); + + // TODO(turbofan): Extend this to also handle fast holey double elements + // once we got the hole NaN mess sorted out in TurboFan/V8. + Handle receiver_map; + if (GetMapWitness(node).ToHandle(&receiver_map) && + CanInlineArrayResizeOperation(receiver_map) && + receiver_map->elements_kind() != FAST_HOLEY_DOUBLE_ELEMENTS) { + // Install code dependencies on the {receiver} prototype maps and the + // global array protector cell. + dependencies()->AssumePropertyCell(factory()->array_protector()); + dependencies()->AssumePrototypeMapsStable(receiver_map); + + // Load length of the {receiver}. + Node* length = effect = graph()->NewNode( + simplified()->LoadField( + AccessBuilder::ForJSArrayLength(receiver_map->elements_kind())), + receiver, effect, control); + + // Return undefined if {receiver} has no elements. + Node* check0 = graph()->NewNode(simplified()->NumberEqual(), length, + jsgraph()->ZeroConstant()); + Node* branch0 = + graph()->NewNode(common()->Branch(BranchHint::kFalse), check0, control); + + Node* if_true0 = graph()->NewNode(common()->IfTrue(), branch0); + Node* etrue0 = effect; + Node* vtrue0 = jsgraph()->UndefinedConstant(); + + Node* if_false0 = graph()->NewNode(common()->IfFalse(), branch0); + Node* efalse0 = effect; + Node* vfalse0; + { + // Check if we should take the fast-path. + Node* check1 = + graph()->NewNode(simplified()->NumberLessThanOrEqual(), length, + jsgraph()->Constant(JSArray::kMaxCopyElements)); + Node* branch1 = graph()->NewNode(common()->Branch(BranchHint::kTrue), + check1, if_false0); + + Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1); + Node* etrue1 = efalse0; + Node* vtrue1; + { + Node* elements = etrue1 = graph()->NewNode( + simplified()->LoadField(AccessBuilder::ForJSObjectElements()), + receiver, etrue1, if_true1); + + // Load the first element here, which we return below. + vtrue1 = etrue1 = graph()->NewNode( + simplified()->LoadElement(AccessBuilder::ForFixedArrayElement( + receiver_map->elements_kind())), + elements, jsgraph()->ZeroConstant(), etrue1, if_true1); + + // Ensure that we aren't shifting a copy-on-write backing store. + if (IsFastSmiOrObjectElementsKind(receiver_map->elements_kind())) { + elements = etrue1 = + graph()->NewNode(simplified()->EnsureWritableFastElements(), + receiver, elements, etrue1, if_true1); + } + + // Shift the remaining {elements} by one towards the start. + Node* loop = graph()->NewNode(common()->Loop(2), if_true1, if_true1); + Node* eloop = + graph()->NewNode(common()->EffectPhi(2), etrue1, etrue1, loop); + Node* index = graph()->NewNode( + common()->Phi(MachineRepresentation::kTagged, 2), + jsgraph()->OneConstant(), + jsgraph()->Constant(JSArray::kMaxCopyElements - 1), loop); + + { + Node* check2 = + graph()->NewNode(simplified()->NumberLessThan(), index, length); + Node* branch2 = graph()->NewNode(common()->Branch(), check2, loop); + + if_true1 = graph()->NewNode(common()->IfFalse(), branch2); + etrue1 = eloop; + + Node* control = graph()->NewNode(common()->IfTrue(), branch2); + Node* effect = etrue1; + + ElementAccess const access = AccessBuilder::ForFixedArrayElement( + receiver_map->elements_kind()); + Node* value = effect = + graph()->NewNode(simplified()->LoadElement(access), elements, + index, effect, control); + effect = graph()->NewNode( + simplified()->StoreElement(access), elements, + graph()->NewNode(simplified()->NumberSubtract(), index, + jsgraph()->OneConstant()), + value, effect, control); + + loop->ReplaceInput(1, control); + eloop->ReplaceInput(1, effect); + index->ReplaceInput(1, + graph()->NewNode(simplified()->NumberAdd(), index, + jsgraph()->OneConstant())); + } + + // Compute the new {length}. + length = graph()->NewNode(simplified()->NumberSubtract(), length, + jsgraph()->OneConstant()); + + // Store the new {length} to the {receiver}. + etrue1 = graph()->NewNode( + simplified()->StoreField( + AccessBuilder::ForJSArrayLength(receiver_map->elements_kind())), + receiver, length, etrue1, if_true1); + + // Store a hole to the element we just removed from the {receiver}. + etrue1 = graph()->NewNode( + simplified()->StoreElement(AccessBuilder::ForFixedArrayElement( + GetHoleyElementsKind(receiver_map->elements_kind()))), + elements, length, jsgraph()->TheHoleConstant(), etrue1, if_true1); + } + + Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1); + Node* efalse1 = efalse0; + Node* vfalse1; + { + // Call the generic C++ implementation. + const int builtin_index = Builtins::kArrayShift; + CallDescriptor const* const desc = Linkage::GetCEntryStubCallDescriptor( + graph()->zone(), 1, BuiltinArguments::kNumExtraArgsWithReceiver, + Builtins::name(builtin_index), node->op()->properties(), + CallDescriptor::kNeedsFrameState); + Node* stub_code = jsgraph()->CEntryStubConstant(1, kDontSaveFPRegs, + kArgvOnStack, true); + Address builtin_entry = Builtins::CppEntryOf(builtin_index); + Node* entry = jsgraph()->ExternalConstant( + ExternalReference(builtin_entry, isolate())); + Node* argc = + jsgraph()->Constant(BuiltinArguments::kNumExtraArgsWithReceiver); + if_false1 = efalse1 = vfalse1 = + graph()->NewNode(common()->Call(desc), stub_code, receiver, argc, + target, jsgraph()->UndefinedConstant(), entry, + argc, context, frame_state, efalse1, if_false1); + } + + if_false0 = graph()->NewNode(common()->Merge(2), if_true1, if_false1); + efalse0 = + graph()->NewNode(common()->EffectPhi(2), etrue1, efalse1, if_false0); + vfalse0 = + graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2), + vtrue1, vfalse1, if_false0); + } + + control = graph()->NewNode(common()->Merge(2), if_true0, if_false0); + effect = graph()->NewNode(common()->EffectPhi(2), etrue0, efalse0, control); + Node* value = + graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2), + vtrue0, vfalse0, control); + + // Convert the hole to undefined. Do this last, so that we can optimize + // conversion operator via some smart strength reduction in many cases. + if (IsFastHoleyElementsKind(receiver_map->elements_kind())) { + value = + graph()->NewNode(simplified()->ConvertTaggedHoleToUndefined(), value); + } + + ReplaceWithValue(node, value, effect, control); + return Replace(value); + } + return NoChange(); +} + namespace { bool HasInstanceTypeWitness(Node* receiver, Node* effect, @@ -1995,6 +2169,8 @@ Reduction JSBuiltinReducer::Reduce(Node* node) { return ReduceArrayPop(node); case kArrayPush: return ReduceArrayPush(node); + case kArrayShift: + return ReduceArrayShift(node); case kDateNow: return ReduceDateNow(node); case kDateGetTime: diff --git a/deps/v8/src/compiler/js-builtin-reducer.h b/deps/v8/src/compiler/js-builtin-reducer.h index 6ff06e3bf3874a..a69469707080da 100644 --- a/deps/v8/src/compiler/js-builtin-reducer.h +++ b/deps/v8/src/compiler/js-builtin-reducer.h @@ -57,6 +57,7 @@ class V8_EXPORT_PRIVATE JSBuiltinReducer final IterationKind kind); Reduction ReduceArrayPop(Node* node); Reduction ReduceArrayPush(Node* node); + Reduction ReduceArrayShift(Node* node); Reduction ReduceDateNow(Node* node); Reduction ReduceDateGetTime(Node* node); Reduction ReduceGlobalIsFinite(Node* node); diff --git a/deps/v8/src/crankshaft/hydrogen.cc b/deps/v8/src/crankshaft/hydrogen.cc index d55bb37c398337..af3207e85929b6 100644 --- a/deps/v8/src/crankshaft/hydrogen.cc +++ b/deps/v8/src/crankshaft/hydrogen.cc @@ -8818,7 +8818,7 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall( Handle::null(), true); // Threshold for fast inlined Array.shift(). - HConstant* inline_threshold = Add(static_cast(16)); + HConstant* inline_threshold = Add(JSArray::kMaxCopyElements); Drop(args_count_no_receiver); HValue* result; diff --git a/deps/v8/src/objects.h b/deps/v8/src/objects.h index 0e2063c703c2c3..a11f22239e640e 100644 --- a/deps/v8/src/objects.h +++ b/deps/v8/src/objects.h @@ -10930,6 +10930,9 @@ class JSArray: public JSObject { static const int kLengthOffset = JSObject::kHeaderSize; static const int kSize = kLengthOffset + kPointerSize; + // Max. number of elements being copied in Array builtins. + static const int kMaxCopyElements = 16; + static const int kInitialMaxFastElementArray = (kMaxRegularHeapObjectSize - FixedArray::kHeaderSize - kSize - AllocationMemento::kSize) / diff --git a/deps/v8/test/mjsunit/array-shift5.js b/deps/v8/test/mjsunit/array-shift5.js new file mode 100644 index 00000000000000..c8fb90d830fba9 --- /dev/null +++ b/deps/v8/test/mjsunit/array-shift5.js @@ -0,0 +1,50 @@ +// Copyright 2017 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +(function() { + function doShift(a) { return a.shift(); } + + function test() { + var a = [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16]; + assertEquals(0, doShift(a)); + assertEquals([1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16], a); + } + + test(); + test(); + %OptimizeFunctionOnNextCall(doShift); + test(); +})(); + +(function() { + function doShift(a) { return a.shift(); } + + function test() { + var a = [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16.1]; + assertEquals(0, doShift(a)); + assertEquals([1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16.1], a); + } + + test(); + test(); + %OptimizeFunctionOnNextCall(doShift); + test(); +})(); + +(function() { + function doShift(a) { return a.shift(); } + + function test() { + var a = [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,"16"]; + assertEquals(0, doShift(a)); + assertEquals([1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,"16"], a); + } + + test(); + test(); + %OptimizeFunctionOnNextCall(doShift); + test(); +})();