Skip to content

Commit

Permalink
deps: cherry-pick 6d38f89 from upstream V8
Browse files Browse the repository at this point in the history
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

      #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: #13263
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
  • Loading branch information
targos authored and addaleax committed Jul 24, 2017
1 parent 4c4f647 commit 65956e6
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 1 deletion.
11 changes: 11 additions & 0 deletions deps/v8/src/compiler/graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,17 @@ class V8_EXPORT_PRIVATE Graph final : public NON_EXPORTED_BASE(ZoneObject) {
Node* nodes[] = {n1, n2, n3, n4, n5, n6, n7, n8, n9};
return NewNode(op, arraysize(nodes), nodes);
}
Node* NewNode(const Operator* op, Node* n1, Node* n2, Node* n3, Node* n4,
Node* n5, Node* n6, Node* n7, Node* n8, Node* n9, Node* n10) {
Node* nodes[] = {n1, n2, n3, n4, n5, n6, n7, n8, n9, n10};
return NewNode(op, arraysize(nodes), nodes);
}
Node* NewNode(const Operator* op, Node* n1, Node* n2, Node* n3, Node* n4,
Node* n5, Node* n6, Node* n7, Node* n8, Node* n9, Node* n10,
Node* n11) {
Node* nodes[] = {n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11};
return NewNode(op, arraysize(nodes), nodes);
}

// Clone the {node}, and assign a new node id to the copy.
Node* CloneNode(const Node* node);
Expand Down
176 changes: 176 additions & 0 deletions deps/v8/src/compiler/js-builtin-reducer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1005,6 +1006,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<Map> 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,
Expand Down Expand Up @@ -2125,6 +2299,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:
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/compiler/js-builtin-reducer.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class V8_EXPORT_PRIVATE JSBuiltinReducer final
Reduction ReduceArrayIsArray(Node* node);
Reduction ReduceArrayPop(Node* node);
Reduction ReduceArrayPush(Node* node);
Reduction ReduceArrayShift(Node* node);
Reduction ReduceDateNow(Node* node);
Reduction ReduceDateGetTime(Node* node);
Reduction ReduceGlobalIsFinite(Node* node);
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/crankshaft/hydrogen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8821,7 +8821,7 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
Handle<JSObject>::null(), true);

// Threshold for fast inlined Array.shift().
HConstant* inline_threshold = Add<HConstant>(static_cast<int32_t>(16));
HConstant* inline_threshold = Add<HConstant>(JSArray::kMaxCopyElements);

Drop(args_count_no_receiver);
HValue* result;
Expand Down
3 changes: 3 additions & 0 deletions deps/v8/src/objects.h
Original file line number Diff line number Diff line change
Expand Up @@ -9608,6 +9608,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) /
Expand Down
50 changes: 50 additions & 0 deletions deps/v8/test/mjsunit/array-shift5.js
Original file line number Diff line number Diff line change
@@ -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();
})();

0 comments on commit 65956e6

Please sign in to comment.