Skip to content

Commit

Permalink
[turbofan] Boost performance of Array.prototype.shift by 4x.
Browse files Browse the repository at this point in the history
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

  nodejs/node#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}
  • Loading branch information
bmeurer authored and Commit bot committed May 10, 2017
1 parent 619dfed commit 6d38f89
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 1 deletion.
11 changes: 11 additions & 0 deletions 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 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 @@ -1021,6 +1022,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 @@ -2141,6 +2315,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 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 src/crankshaft/hydrogen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8583,7 +8583,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 src/objects.h
Original file line number Diff line number Diff line change
Expand Up @@ -9599,6 +9599,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 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 6d38f89

Please sign in to comment.