Skip to content

Commit 2648c8d

Browse files
ofrobotsjasnell
authored andcommitted
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 #12657 and discovered for the syncthrough module using https://github.com/mcollina/syncthrough/blob/master/benchmarks/basic.js as benchmark. [email protected] BUG=v8:6376 Review-Url: https://codereview.chromium.org/2874453002 Cr-Commit-Position: refs/heads/master@{#45216} PR-URL: #13162 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 5ad4170 commit 2648c8d

File tree

6 files changed

+232
-2
lines changed

6 files changed

+232
-2
lines changed

deps/v8/include/v8-version.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#define V8_MAJOR_VERSION 5
1212
#define V8_MINOR_VERSION 8
1313
#define V8_BUILD_NUMBER 283
14-
#define V8_PATCH_LEVEL 39
14+
#define V8_PATCH_LEVEL 40
1515

1616
// Use 1 for candidates and 0 otherwise.
1717
// (Boolean macro values are not supported by all preprocessors.)

deps/v8/src/compiler/js-builtin-reducer.cc

+176
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "src/compiler/js-builtin-reducer.h"
66

77
#include "src/base/bits.h"
8+
#include "src/builtins/builtins-utils.h"
89
#include "src/code-factory.h"
910
#include "src/compilation-dependencies.h"
1011
#include "src/compiler/access-builder.h"
@@ -908,6 +909,179 @@ Reduction JSBuiltinReducer::ReduceArrayPush(Node* node) {
908909
return NoChange();
909910
}
910911

912+
// ES6 section 22.1.3.22 Array.prototype.shift ( )
913+
Reduction JSBuiltinReducer::ReduceArrayShift(Node* node) {
914+
Node* target = NodeProperties::GetValueInput(node, 0);
915+
Node* receiver = NodeProperties::GetValueInput(node, 1);
916+
Node* context = NodeProperties::GetContextInput(node);
917+
Node* frame_state = NodeProperties::GetFrameStateInput(node);
918+
Node* effect = NodeProperties::GetEffectInput(node);
919+
Node* control = NodeProperties::GetControlInput(node);
920+
921+
// TODO(turbofan): Extend this to also handle fast holey double elements
922+
// once we got the hole NaN mess sorted out in TurboFan/V8.
923+
Handle<Map> receiver_map;
924+
if (GetMapWitness(node).ToHandle(&receiver_map) &&
925+
CanInlineArrayResizeOperation(receiver_map) &&
926+
receiver_map->elements_kind() != FAST_HOLEY_DOUBLE_ELEMENTS) {
927+
// Install code dependencies on the {receiver} prototype maps and the
928+
// global array protector cell.
929+
dependencies()->AssumePropertyCell(factory()->array_protector());
930+
dependencies()->AssumePrototypeMapsStable(receiver_map);
931+
932+
// Load length of the {receiver}.
933+
Node* length = effect = graph()->NewNode(
934+
simplified()->LoadField(
935+
AccessBuilder::ForJSArrayLength(receiver_map->elements_kind())),
936+
receiver, effect, control);
937+
938+
// Return undefined if {receiver} has no elements.
939+
Node* check0 = graph()->NewNode(simplified()->NumberEqual(), length,
940+
jsgraph()->ZeroConstant());
941+
Node* branch0 =
942+
graph()->NewNode(common()->Branch(BranchHint::kFalse), check0, control);
943+
944+
Node* if_true0 = graph()->NewNode(common()->IfTrue(), branch0);
945+
Node* etrue0 = effect;
946+
Node* vtrue0 = jsgraph()->UndefinedConstant();
947+
948+
Node* if_false0 = graph()->NewNode(common()->IfFalse(), branch0);
949+
Node* efalse0 = effect;
950+
Node* vfalse0;
951+
{
952+
// Check if we should take the fast-path.
953+
Node* check1 =
954+
graph()->NewNode(simplified()->NumberLessThanOrEqual(), length,
955+
jsgraph()->Constant(JSArray::kMaxCopyElements));
956+
Node* branch1 = graph()->NewNode(common()->Branch(BranchHint::kTrue),
957+
check1, if_false0);
958+
959+
Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1);
960+
Node* etrue1 = efalse0;
961+
Node* vtrue1;
962+
{
963+
Node* elements = etrue1 = graph()->NewNode(
964+
simplified()->LoadField(AccessBuilder::ForJSObjectElements()),
965+
receiver, etrue1, if_true1);
966+
967+
// Load the first element here, which we return below.
968+
vtrue1 = etrue1 = graph()->NewNode(
969+
simplified()->LoadElement(AccessBuilder::ForFixedArrayElement(
970+
receiver_map->elements_kind())),
971+
elements, jsgraph()->ZeroConstant(), etrue1, if_true1);
972+
973+
// Ensure that we aren't shifting a copy-on-write backing store.
974+
if (IsFastSmiOrObjectElementsKind(receiver_map->elements_kind())) {
975+
elements = etrue1 =
976+
graph()->NewNode(simplified()->EnsureWritableFastElements(),
977+
receiver, elements, etrue1, if_true1);
978+
}
979+
980+
// Shift the remaining {elements} by one towards the start.
981+
Node* loop = graph()->NewNode(common()->Loop(2), if_true1, if_true1);
982+
Node* eloop =
983+
graph()->NewNode(common()->EffectPhi(2), etrue1, etrue1, loop);
984+
Node* index = graph()->NewNode(
985+
common()->Phi(MachineRepresentation::kTagged, 2),
986+
jsgraph()->OneConstant(),
987+
jsgraph()->Constant(JSArray::kMaxCopyElements - 1), loop);
988+
989+
{
990+
Node* check2 =
991+
graph()->NewNode(simplified()->NumberLessThan(), index, length);
992+
Node* branch2 = graph()->NewNode(common()->Branch(), check2, loop);
993+
994+
if_true1 = graph()->NewNode(common()->IfFalse(), branch2);
995+
etrue1 = eloop;
996+
997+
Node* control = graph()->NewNode(common()->IfTrue(), branch2);
998+
Node* effect = etrue1;
999+
1000+
ElementAccess const access = AccessBuilder::ForFixedArrayElement(
1001+
receiver_map->elements_kind());
1002+
Node* value = effect =
1003+
graph()->NewNode(simplified()->LoadElement(access), elements,
1004+
index, effect, control);
1005+
effect = graph()->NewNode(
1006+
simplified()->StoreElement(access), elements,
1007+
graph()->NewNode(simplified()->NumberSubtract(), index,
1008+
jsgraph()->OneConstant()),
1009+
value, effect, control);
1010+
1011+
loop->ReplaceInput(1, control);
1012+
eloop->ReplaceInput(1, effect);
1013+
index->ReplaceInput(1,
1014+
graph()->NewNode(simplified()->NumberAdd(), index,
1015+
jsgraph()->OneConstant()));
1016+
}
1017+
1018+
// Compute the new {length}.
1019+
length = graph()->NewNode(simplified()->NumberSubtract(), length,
1020+
jsgraph()->OneConstant());
1021+
1022+
// Store the new {length} to the {receiver}.
1023+
etrue1 = graph()->NewNode(
1024+
simplified()->StoreField(
1025+
AccessBuilder::ForJSArrayLength(receiver_map->elements_kind())),
1026+
receiver, length, etrue1, if_true1);
1027+
1028+
// Store a hole to the element we just removed from the {receiver}.
1029+
etrue1 = graph()->NewNode(
1030+
simplified()->StoreElement(AccessBuilder::ForFixedArrayElement(
1031+
GetHoleyElementsKind(receiver_map->elements_kind()))),
1032+
elements, length, jsgraph()->TheHoleConstant(), etrue1, if_true1);
1033+
}
1034+
1035+
Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1);
1036+
Node* efalse1 = efalse0;
1037+
Node* vfalse1;
1038+
{
1039+
// Call the generic C++ implementation.
1040+
const int builtin_index = Builtins::kArrayShift;
1041+
CallDescriptor const* const desc = Linkage::GetCEntryStubCallDescriptor(
1042+
graph()->zone(), 1, BuiltinArguments::kNumExtraArgsWithReceiver,
1043+
Builtins::name(builtin_index), node->op()->properties(),
1044+
CallDescriptor::kNeedsFrameState);
1045+
Node* stub_code = jsgraph()->CEntryStubConstant(1, kDontSaveFPRegs,
1046+
kArgvOnStack, true);
1047+
Address builtin_entry = Builtins::CppEntryOf(builtin_index);
1048+
Node* entry = jsgraph()->ExternalConstant(
1049+
ExternalReference(builtin_entry, isolate()));
1050+
Node* argc =
1051+
jsgraph()->Constant(BuiltinArguments::kNumExtraArgsWithReceiver);
1052+
if_false1 = efalse1 = vfalse1 =
1053+
graph()->NewNode(common()->Call(desc), stub_code, receiver, argc,
1054+
target, jsgraph()->UndefinedConstant(), entry,
1055+
argc, context, frame_state, efalse1, if_false1);
1056+
}
1057+
1058+
if_false0 = graph()->NewNode(common()->Merge(2), if_true1, if_false1);
1059+
efalse0 =
1060+
graph()->NewNode(common()->EffectPhi(2), etrue1, efalse1, if_false0);
1061+
vfalse0 =
1062+
graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
1063+
vtrue1, vfalse1, if_false0);
1064+
}
1065+
1066+
control = graph()->NewNode(common()->Merge(2), if_true0, if_false0);
1067+
effect = graph()->NewNode(common()->EffectPhi(2), etrue0, efalse0, control);
1068+
Node* value =
1069+
graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
1070+
vtrue0, vfalse0, control);
1071+
1072+
// Convert the hole to undefined. Do this last, so that we can optimize
1073+
// conversion operator via some smart strength reduction in many cases.
1074+
if (IsFastHoleyElementsKind(receiver_map->elements_kind())) {
1075+
value =
1076+
graph()->NewNode(simplified()->ConvertTaggedHoleToUndefined(), value);
1077+
}
1078+
1079+
ReplaceWithValue(node, value, effect, control);
1080+
return Replace(value);
1081+
}
1082+
return NoChange();
1083+
}
1084+
9111085
namespace {
9121086

9131087
bool HasInstanceTypeWitness(Node* receiver, Node* effect,
@@ -1995,6 +2169,8 @@ Reduction JSBuiltinReducer::Reduce(Node* node) {
19952169
return ReduceArrayPop(node);
19962170
case kArrayPush:
19972171
return ReduceArrayPush(node);
2172+
case kArrayShift:
2173+
return ReduceArrayShift(node);
19982174
case kDateNow:
19992175
return ReduceDateNow(node);
20002176
case kDateGetTime:

deps/v8/src/compiler/js-builtin-reducer.h

+1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ class V8_EXPORT_PRIVATE JSBuiltinReducer final
5757
IterationKind kind);
5858
Reduction ReduceArrayPop(Node* node);
5959
Reduction ReduceArrayPush(Node* node);
60+
Reduction ReduceArrayShift(Node* node);
6061
Reduction ReduceDateNow(Node* node);
6162
Reduction ReduceDateGetTime(Node* node);
6263
Reduction ReduceGlobalIsFinite(Node* node);

deps/v8/src/crankshaft/hydrogen.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -8818,7 +8818,7 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
88188818
Handle<JSObject>::null(), true);
88198819

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

88238823
Drop(args_count_no_receiver);
88248824
HValue* result;

deps/v8/src/objects.h

+3
Original file line numberDiff line numberDiff line change
@@ -10930,6 +10930,9 @@ class JSArray: public JSObject {
1093010930
static const int kLengthOffset = JSObject::kHeaderSize;
1093110931
static const int kSize = kLengthOffset + kPointerSize;
1093210932

10933+
// Max. number of elements being copied in Array builtins.
10934+
static const int kMaxCopyElements = 16;
10935+
1093310936
static const int kInitialMaxFastElementArray =
1093410937
(kMaxRegularHeapObjectSize - FixedArray::kHeaderSize - kSize -
1093510938
AllocationMemento::kSize) /

deps/v8/test/mjsunit/array-shift5.js

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2017 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Flags: --allow-natives-syntax
6+
7+
(function() {
8+
function doShift(a) { return a.shift(); }
9+
10+
function test() {
11+
var a = [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16];
12+
assertEquals(0, doShift(a));
13+
assertEquals([1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16], a);
14+
}
15+
16+
test();
17+
test();
18+
%OptimizeFunctionOnNextCall(doShift);
19+
test();
20+
})();
21+
22+
(function() {
23+
function doShift(a) { return a.shift(); }
24+
25+
function test() {
26+
var a = [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16.1];
27+
assertEquals(0, doShift(a));
28+
assertEquals([1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16.1], a);
29+
}
30+
31+
test();
32+
test();
33+
%OptimizeFunctionOnNextCall(doShift);
34+
test();
35+
})();
36+
37+
(function() {
38+
function doShift(a) { return a.shift(); }
39+
40+
function test() {
41+
var a = [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,"16"];
42+
assertEquals(0, doShift(a));
43+
assertEquals([1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,"16"], a);
44+
}
45+
46+
test();
47+
test();
48+
%OptimizeFunctionOnNextCall(doShift);
49+
test();
50+
})();

0 commit comments

Comments
 (0)