Commit 6230641b authored by adamk's avatar adamk Committed by Commit bot

Optimize testing for an index's existence in packed Arrays

This patch introduces a new inline runtime function,
%_HasFastPackedElements(), and uses it both in the implementation
of the 'in' operator and in the array builtins to speed
up testing for the existence of an index in an array.

In testing with the microbenchmark on the attached bug,
for example, the runtime goes from 326ms to 66ms.

A reviewer might ask whether the HAS_INDEX macro is worthwhile,
and I tried the same example without it, which pushed the
microbenchmark up to 157ms. So it seems it's worth it to
avoid the function call to IN() if we know we're dealing
with arrays and numbers.

BUG=v8:3701
LOG=n

Review URL: https://codereview.chromium.org/754863002

Cr-Commit-Position: refs/heads/master@{#25665}
parent f62ab8d0
......@@ -298,9 +298,10 @@ function SparseMove(array, start_i, del_count, len, num_additional_args) {
// because the receiver is not an array (so we have no choice) or because we
// know we are not deleting or moving a lot of elements.
function SimpleSlice(array, start_i, del_count, len, deleted_elements) {
var is_array = IS_ARRAY(array);
for (var i = 0; i < del_count; i++) {
var index = start_i + i;
if (index in array) {
if (HAS_INDEX(array, index, is_array)) {
var current = array[index];
// The spec requires [[DefineOwnProperty]] here, %AddElement is close
// enough (in that it ignores the prototype).
......@@ -311,6 +312,7 @@ function SimpleSlice(array, start_i, del_count, len, deleted_elements) {
function SimpleMove(array, start_i, del_count, len, num_additional_args) {
var is_array = IS_ARRAY(array);
if (num_additional_args !== del_count) {
// Move the existing elements after the elements to be deleted
// to the right position in the resulting array.
......@@ -318,7 +320,7 @@ function SimpleMove(array, start_i, del_count, len, num_additional_args) {
for (var i = len - del_count; i > start_i; i--) {
var from_index = i + del_count - 1;
var to_index = i + num_additional_args - 1;
if (from_index in array) {
if (HAS_INDEX(array, from_index, is_array)) {
array[to_index] = array[from_index];
} else {
delete array[to_index];
......@@ -328,7 +330,7 @@ function SimpleMove(array, start_i, del_count, len, num_additional_args) {
for (var i = start_i; i < len - del_count; i++) {
var from_index = i + del_count;
var to_index = i + num_additional_args;
if (from_index in array) {
if (HAS_INDEX(array, from_index, is_array)) {
array[to_index] = array[from_index];
} else {
delete array[to_index];
......@@ -1153,9 +1155,10 @@ function ArrayFilter(f, receiver) {
var result = new $Array();
var accumulator = new InternalArray();
var accumulator_length = 0;
var is_array = IS_ARRAY(array);
var stepping = DEBUG_IS_ACTIVE && %DebugCallbackSupportsStepping(f);
for (var i = 0; i < length; i++) {
if (i in array) {
if (HAS_INDEX(array, i, is_array)) {
var element = array[i];
// Prepare break slots for debugger step in.
if (stepping) %DebugPrepareStepInIfStepping(f);
......@@ -1188,9 +1191,10 @@ function ArrayForEach(f, receiver) {
needs_wrapper = SHOULD_CREATE_WRAPPER(f, receiver);
}
var is_array = IS_ARRAY(array);
var stepping = DEBUG_IS_ACTIVE && %DebugCallbackSupportsStepping(f);
for (var i = 0; i < length; i++) {
if (i in array) {
if (HAS_INDEX(array, i, is_array)) {
var element = array[i];
// Prepare break slots for debugger step in.
if (stepping) %DebugPrepareStepInIfStepping(f);
......@@ -1221,9 +1225,10 @@ function ArraySome(f, receiver) {
needs_wrapper = SHOULD_CREATE_WRAPPER(f, receiver);
}
var is_array = IS_ARRAY(array);
var stepping = DEBUG_IS_ACTIVE && %DebugCallbackSupportsStepping(f);
for (var i = 0; i < length; i++) {
if (i in array) {
if (HAS_INDEX(array, i, is_array)) {
var element = array[i];
// Prepare break slots for debugger step in.
if (stepping) %DebugPrepareStepInIfStepping(f);
......@@ -1253,9 +1258,10 @@ function ArrayEvery(f, receiver) {
needs_wrapper = SHOULD_CREATE_WRAPPER(f, receiver);
}
var is_array = IS_ARRAY(array);
var stepping = DEBUG_IS_ACTIVE && %DebugCallbackSupportsStepping(f);
for (var i = 0; i < length; i++) {
if (i in array) {
if (HAS_INDEX(array, i, is_array)) {
var element = array[i];
// Prepare break slots for debugger step in.
if (stepping) %DebugPrepareStepInIfStepping(f);
......@@ -1286,9 +1292,10 @@ function ArrayMap(f, receiver) {
var result = new $Array();
var accumulator = new InternalArray(length);
var is_array = IS_ARRAY(array);
var stepping = DEBUG_IS_ACTIVE && %DebugCallbackSupportsStepping(f);
for (var i = 0; i < length; i++) {
if (i in array) {
if (HAS_INDEX(array, i, is_array)) {
var element = array[i];
// Prepare break slots for debugger step in.
if (stepping) %DebugPrepareStepInIfStepping(f);
......@@ -1423,10 +1430,11 @@ function ArrayReduce(callback, current) {
throw MakeTypeError('called_non_callable', [callback]);
}
var is_array = IS_ARRAY(array);
var i = 0;
find_initial: if (%_ArgumentsLength() < 2) {
for (; i < length; i++) {
if (i in array) {
if (HAS_INDEX(array, i, is_array)) {
current = array[i++];
break find_initial;
}
......@@ -1437,7 +1445,7 @@ function ArrayReduce(callback, current) {
var receiver = %GetDefaultReceiver(callback);
var stepping = DEBUG_IS_ACTIVE && %DebugCallbackSupportsStepping(callback);
for (; i < length; i++) {
if (i in array) {
if (HAS_INDEX(array, i, is_array)) {
var element = array[i];
// Prepare break slots for debugger step in.
if (stepping) %DebugPrepareStepInIfStepping(callback);
......@@ -1459,10 +1467,11 @@ function ArrayReduceRight(callback, current) {
throw MakeTypeError('called_non_callable', [callback]);
}
var is_array = IS_ARRAY(array);
var i = length - 1;
find_initial: if (%_ArgumentsLength() < 2) {
for (; i >= 0; i--) {
if (i in array) {
if (HAS_INDEX(array, i, is_array)) {
current = array[i--];
break find_initial;
}
......@@ -1473,7 +1482,7 @@ function ArrayReduceRight(callback, current) {
var receiver = %GetDefaultReceiver(callback);
var stepping = DEBUG_IS_ACTIVE && %DebugCallbackSupportsStepping(callback);
for (; i >= 0; i--) {
if (i in array) {
if (HAS_INDEX(array, i, is_array)) {
var element = array[i];
// Prepare break slots for debugger step in.
if (stepping) %DebugPrepareStepInIfStepping(callback);
......
......@@ -11665,6 +11665,35 @@ void HOptimizedGraphBuilder::GenerateIsJSProxy(CallRuntime* call) {
}
void HOptimizedGraphBuilder::GenerateHasFastPackedElements(CallRuntime* call) {
DCHECK(call->arguments()->length() == 1);
CHECK_ALIVE(VisitForValue(call->arguments()->at(0)));
HValue* object = Pop();
HIfContinuation continuation(graph()->CreateBasicBlock(),
graph()->CreateBasicBlock());
IfBuilder if_not_smi(this);
if_not_smi.IfNot<HIsSmiAndBranch>(object);
if_not_smi.Then();
{
NoObservableSideEffectsScope no_effects(this);
IfBuilder if_fast_packed(this);
HValue* elements_kind = BuildGetElementsKind(object);
if_fast_packed.If<HCompareNumericAndBranch>(
elements_kind, Add<HConstant>(FAST_SMI_ELEMENTS), Token::EQ);
if_fast_packed.Or();
if_fast_packed.If<HCompareNumericAndBranch>(
elements_kind, Add<HConstant>(FAST_ELEMENTS), Token::EQ);
if_fast_packed.Or();
if_fast_packed.If<HCompareNumericAndBranch>(
elements_kind, Add<HConstant>(FAST_DOUBLE_ELEMENTS), Token::EQ);
if_fast_packed.JoinContinuation(&continuation);
}
if_not_smi.JoinContinuation(&continuation);
return ast_context()->ReturnContinuation(&continuation, call->id());
}
void HOptimizedGraphBuilder::GenerateIsNonNegativeSmi(CallRuntime* call) {
return Bailout(kInlinedRuntimeFunctionIsNonNegativeSmi);
}
......
......@@ -168,6 +168,7 @@ macro TO_OBJECT_INLINE(arg) = (IS_SPEC_OBJECT(%IS_VAR(arg)) ? arg : ToObject(arg
macro JSON_NUMBER_TO_STRING(arg) = ((%_IsSmi(%IS_VAR(arg)) || arg - arg == 0) ? %_NumberToString(arg) : "null");
macro HAS_OWN_PROPERTY(obj, index) = (%_CallFunction(obj, index, ObjectHasOwnProperty));
macro SHOULD_CREATE_WRAPPER(functionName, receiver) = (!IS_SPEC_OBJECT(receiver) && %IsSloppyModeFunction(functionName));
macro HAS_INDEX(array, index, is_array) = ((is_array && %_HasFastPackedElements(%IS_VAR(array))) ? (index < array.length) : (index in array));
# Private names.
# GET_PRIVATE should only be used if the property is known to exists on obj
......
......@@ -324,8 +324,13 @@ function IN(x) {
if (!IS_SPEC_OBJECT(x)) {
throw %MakeTypeError('invalid_in_operator_use', [this, x]);
}
return %_IsNonNegativeSmi(this) ?
%HasElement(x, this) : %HasProperty(x, %ToName(this));
if (%_IsNonNegativeSmi(this)) {
if (IS_ARRAY(x) && %_HasFastPackedElements(x)) {
return this < x.length;
}
return %HasElement(x, this);
}
return %HasProperty(x, %ToName(this));
}
......
......@@ -1524,6 +1524,15 @@ RUNTIME_FUNCTION(Runtime_GetDataProperty) {
}
RUNTIME_FUNCTION(Runtime_HasFastPackedElements) {
SealHandleScope shs(isolate);
DCHECK(args.length() == 1);
CONVERT_ARG_CHECKED(HeapObject, obj, 0);
return isolate->heap()->ToBoolean(
IsFastPackedElementsKind(obj->map()->elements_kind()));
}
RUNTIME_FUNCTION(RuntimeReference_ValueOf) {
SealHandleScope shs(isolate);
DCHECK(args.length() == 1);
......
......@@ -732,7 +732,9 @@ namespace internal {
F(MapGetSize, 1, 1) \
F(MapHas, 2, 1) \
F(SetGetSize, 1, 1) \
F(SetHas, 2, 1)
F(SetHas, 2, 1) \
/* Arrays */ \
F(HasFastPackedElements, 1, 1)
//---------------------------------------------------------------------------
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment