Reviewers: Michael Starzinger,
Message:
Hi Michael,
Can you have a look? Unfortunately I had to undo much of the refactoring
that
consolidated CreateArrayLiteral and CreateArrayLiteralShallow. Sending a
follow-up mail.
thx,
--Michael
Description:
A performance regression in array literal creation was caused by refactoring
that eliminated a special fast case for shallow arrays. At the same time the
general case got a bit slower. This CL restores the special fast case, but
in a
slightly different way, by adding a flags field to
Runtime::CreateArrayLiteral.
The flags delivers information about shallowness but also whether or not
allocation mementos should be created. This is useful for crankshafted code.
BUG=v8:3008
Please review this at https://codereview.chromium.org/77293003/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+77, -17 lines):
M src/ast.h
M src/hydrogen.cc
M src/ia32/code-stubs-ia32.cc
M src/ia32/full-codegen-ia32.cc
M src/runtime.h
M src/runtime.cc
M test/mjsunit/array-literal-feedback.js
Index: src/ast.h
diff --git a/src/ast.h b/src/ast.h
index
e3fc053d8e8bd54d6d42d187c2ab1d4f899fae59..4158907d993ecdb4a30c64bed47e40d41fdc5d0e
100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -1604,6 +1604,12 @@ class ArrayLiteral V8_FINAL : public
MaterializedLiteral {
// Populate the constant elements fixed array.
void BuildConstantElements(Isolate* isolate);
+ enum Flags {
+ kNoFlags = 0,
+ kShallowElements = 1,
+ kDisableMementos = 1 << 1
+ };
+
protected:
ArrayLiteral(Isolate* isolate,
ZoneList<Expression*>* values,
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
f7b310003e0c1140936a8c86d51638534acd824a..2b77c35af4c08f81d90c72b2e043fa8c737c2f26
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -5120,10 +5120,15 @@ void
HOptimizedGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) {
// pass an empty fixed array to the runtime function instead.
Handle<FixedArray> constants =
isolate()->factory()->empty_fixed_array();
int literal_index = expr->literal_index();
+ int flags = expr->depth() == 1
+ ? ArrayLiteral::kShallowElements
+ : ArrayLiteral::kNoFlags;
+ flags |= ArrayLiteral::kDisableMementos;
Add<HPushArgument>(Add<HConstant>(literals));
Add<HPushArgument>(Add<HConstant>(literal_index));
Add<HPushArgument>(Add<HConstant>(constants));
+ Add<HPushArgument>(Add<HConstant>(flags));
// TODO(mvstanton): Consider a flag to turn off creation of any
// AllocationMementos for this call: we are in crankshaft and should
have
@@ -5131,7 +5136,7 @@ void
HOptimizedGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) {
Runtime::FunctionId function_id = Runtime::kCreateArrayLiteral;
literal = Add<HCallRuntime>(isolate()->factory()->empty_string(),
Runtime::FunctionForId(function_id),
- 3);
+ 4);
// De-opt if elements kind changed from boilerplate_elements_kind.
Handle<Map> map = Handle<Map>(boilerplate_object->map(), isolate());
Index: src/ia32/code-stubs-ia32.cc
diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index
f06cc5b700f95afd82c97cab5b60a2c1660a6e33..715feca05d62fba99e9a168372b59eba25dfc5ea
100644
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -82,7 +82,7 @@ void
FastCloneShallowArrayStub::InitializeInterfaceDescriptor(
descriptor->register_param_count_ = 3;
descriptor->register_params_ = registers;
descriptor->deoptimization_handler_ =
- Runtime::FunctionForId(Runtime::kCreateArrayLiteral)->entry;
+
Runtime::FunctionForId(Runtime::kCreateArrayLiteralStubBailout)->entry;
}
Index: src/ia32/full-codegen-ia32.cc
diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index
86c525d7f63e034af66868d20a94b34338cf6d3a..180198b74d67d67b3ac7934721597c439e2f9c8c
100644
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -1705,6 +1705,10 @@ void
FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
Comment cmnt(masm_, "[ ArrayLiteral");
expr->BuildConstantElements(isolate());
+ int flags = expr->depth() == 1
+ ? ArrayLiteral::kShallowElements
+ : ArrayLiteral::kNoFlags;
+
ZoneList<Expression*>* subexprs = expr->values();
int length = subexprs->length();
Handle<FixedArray> constant_elements = expr->constant_elements();
@@ -1737,7 +1741,8 @@ void
FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
__ push(FieldOperand(ebx, JSFunction::kLiteralsOffset));
__ push(Immediate(Smi::FromInt(expr->literal_index())));
__ push(Immediate(constant_elements));
- __ CallRuntime(Runtime::kCreateArrayLiteral, 3);
+ __ push(Immediate(Smi::FromInt(flags)));
+ __ CallRuntime(Runtime::kCreateArrayLiteral, 4);
} else {
ASSERT(IsFastSmiOrObjectElementsKind(constant_elements_kind) ||
FLAG_smi_only_arrays);
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
b5a108159ad81bac2a1ef8a7e28a287751e712c0..ba0bbc3963dafd02b1ce32c59c33220d77687043
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -554,24 +554,63 @@ static Handle<AllocationSite>
GetLiteralAllocationSite(
}
+static MaybeObject* CreateArrayLiteralImpl(Isolate* isolate,
+ Handle<FixedArray> literals,
+ int literals_index,
+ Handle<FixedArray> elements,
+ int flags) {
+ Handle<AllocationSite> site = GetLiteralAllocationSite(isolate, literals,
+ literals_index, elements);
+ RETURN_IF_EMPTY_HANDLE(isolate, site);
+
+ Handle<JSObject> boilerplate(JSObject::cast(site->transition_info()));
+ if ((flags & ArrayLiteral::kShallowElements) != 0) {
+ if (boilerplate->elements()->map() ==
+ isolate->heap()->fixed_cow_array_map()) {
+ isolate->counters()->cow_arrays_created_runtime()->Increment();
+ }
+
+ if ((flags & ArrayLiteral::kDisableMementos) == 0 &&
+ AllocationSite::GetMode(boilerplate->GetElementsKind()) ==
+ TRACK_ALLOCATION_SITE) {
+ return isolate->heap()->CopyJSObject(*boilerplate, *site);
+ }
+
+ return isolate->heap()->CopyJSObject(*boilerplate);
+ } else {
+ AllocationSiteUsageContext usage_context(isolate, site,
+ (flags & ArrayLiteral::kDisableMementos) == 0);
+ usage_context.EnterNewScope();
+ Handle<JSObject> copy = JSObject::DeepCopy(boilerplate,
&usage_context);
+ usage_context.ExitScope(site, boilerplate);
+ RETURN_IF_EMPTY_HANDLE(isolate, copy);
+ return *copy;
+ }
+}
+
+
RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateArrayLiteral) {
HandleScope scope(isolate);
- ASSERT(args.length() == 3);
+ ASSERT(args.length() == 4);
CONVERT_ARG_HANDLE_CHECKED(FixedArray, literals, 0);
CONVERT_SMI_ARG_CHECKED(literals_index, 1);
CONVERT_ARG_HANDLE_CHECKED(FixedArray, elements, 2);
+ CONVERT_SMI_ARG_CHECKED(flags, 3);
- Handle<AllocationSite> site = GetLiteralAllocationSite(isolate, literals,
- literals_index, elements);
- RETURN_IF_EMPTY_HANDLE(isolate, site);
+ return CreateArrayLiteralImpl(isolate, literals, literals_index,
elements,
+ flags);
+}
- Handle<JSObject> boilerplate(JSObject::cast(site->transition_info()));
- AllocationSiteUsageContext usage_context(isolate, site, true);
- usage_context.EnterNewScope();
- Handle<JSObject> copy = JSObject::DeepCopy(boilerplate, &usage_context);
- usage_context.ExitScope(site, boilerplate);
- RETURN_IF_EMPTY_HANDLE(isolate, copy);
- return *copy;
+
+RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateArrayLiteralStubBailout) {
+ HandleScope scope(isolate);
+ ASSERT(args.length() == 3);
+ CONVERT_ARG_HANDLE_CHECKED(FixedArray, literals, 0);
+ CONVERT_SMI_ARG_CHECKED(literals_index, 1);
+ CONVERT_ARG_HANDLE_CHECKED(FixedArray, elements, 2);
+
+ return CreateArrayLiteralImpl(isolate, literals, literals_index,
elements,
+ ArrayLiteral::kShallowElements);
}
Index: src/runtime.h
diff --git a/src/runtime.h b/src/runtime.h
index
4872ab51ccbd1e245a1c5a3f18b05d0b286ba69b..b6a9dcf8a18333d4fa1612288fa3f60d52eceb8c
100644
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -301,7 +301,8 @@ namespace internal {
/* Literals */ \
F(MaterializeRegExpLiteral, 4, 1)\
F(CreateObjectLiteral, 4, 1) \
- F(CreateArrayLiteral, 3, 1) \
+ F(CreateArrayLiteral, 4, 1) \
+ F(CreateArrayLiteralStubBailout, 3, 1) \
\
/* Harmony generators */ \
F(CreateJSGeneratorObject, 0, 1) \
Index: test/mjsunit/array-literal-feedback.js
diff --git a/test/mjsunit/array-literal-feedback.js
b/test/mjsunit/array-literal-feedback.js
index
93ed3bce9988bb95dd7e7b6ac6b665a1cdc96534..3e95eab1d0f1655d89407c4e4760381c62e11c25
100644
--- a/test/mjsunit/array-literal-feedback.js
+++ b/test/mjsunit/array-literal-feedback.js
@@ -93,11 +93,15 @@ if (support_smi_only_arrays) {
}
get_literal(3);
- get_literal(3);
- %OptimizeFunctionOnNextCall(get_literal);
+ // It's important to store a from before we crankshaft get_literal,
because
+ // mementos won't be created from crankshafted code at all.
a = get_literal(3);
+ %OptimizeFunctionOnNextCall(get_literal);
+ get_literal(3);
assertOptimized(get_literal);
assertTrue(%HasFastSmiElements(a));
+ // a has a memento so the transition caused by the store will affect the
+ // boilerplate.
a[0] = 3.5;
// We should have transitioned the boilerplate array to double, and
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
