Reviewers: ulan,
Message:
Hi Ulan,
Here is the change we discussed, thx!
--Michael
Description:
MaterializedLiteral expressions need to cache expression depth.
A problem arises in recursive literal expressions due to recent
changes that defer allocation of constant literal properties
from parse time. We were calculating expression depth as a
side-effect of a lazy constant property build, but subsequent
calls for the depth always returned 1. Cache the correct depth
in the MaterializedLiteral instead.
(Related-to/very-partial-revert-of
https://codereview.chromium.org/61873003)
Please review this at https://codereview.chromium.org/78493002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+50, -42 lines):
M src/arm/full-codegen-arm.cc
M src/ast.h
M src/ast.cc
M src/ia32/full-codegen-ia32.cc
M src/mips/full-codegen-mips.cc
M src/x64/full-codegen-x64.cc
Index: src/arm/full-codegen-arm.cc
diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index
d95055c9cd1bd80b64c8278400ffdabbd8692a97..64a9fdf08bc868101c1ca2c4dc464e852f411742
100644
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -1635,8 +1635,7 @@ void FullCodeGenerator::EmitAccessor(Expression*
expression) {
void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
Comment cmnt(masm_, "[ ObjectLiteral");
- int depth = 1;
- expr->BuildConstantProperties(isolate(), &depth);
+ expr->BuildConstantProperties(isolate());
Handle<FixedArray> constant_properties = expr->constant_properties();
__ ldr(r3, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset));
__ ldr(r3, FieldMemOperand(r3, JSFunction::kLiteralsOffset));
@@ -1651,7 +1650,7 @@ void
FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
__ mov(r0, Operand(Smi::FromInt(flags)));
int properties_count = constant_properties->length() / 2;
if ((FLAG_track_double_fields && expr->may_store_doubles()) ||
- depth > 1 || Serializer::enabled() ||
+ expr->depth() > 1 || Serializer::enabled() ||
flags != ObjectLiteral::kFastElements ||
properties_count >
FastCloneShallowObjectStub::kMaximumClonedProperties) {
__ Push(r3, r2, r1, r0);
@@ -1770,8 +1769,7 @@ void
FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
Comment cmnt(masm_, "[ ArrayLiteral");
- int depth = 1;
- expr->BuildConstantElements(isolate(), &depth);
+ expr->BuildConstantElements(isolate());
ZoneList<Expression*>* subexprs = expr->values();
int length = subexprs->length();
Handle<FixedArray> constant_elements = expr->constant_elements();
@@ -1795,7 +1793,7 @@ void
FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
__ CallStub(&stub);
__ IncrementCounter(
isolate()->counters()->cow_arrays_created_stub(), 1, r1, r2);
- } else if (depth > 1 || Serializer::enabled() ||
+ } else if (expr->depth() > 1 || Serializer::enabled() ||
length > FastCloneShallowArrayStub::kMaximumClonedLength) {
__ Push(r3, r2, r1);
__ CallRuntime(Runtime::kCreateArrayLiteral, 3);
Index: src/ast.cc
diff --git a/src/ast.cc b/src/ast.cc
index
adf0fb8703dc68800cf8b34fbf1624d71a70383f..3ca1449409466af29bca87c184efa14b6c381a4b
100644
--- a/src/ast.cc
+++ b/src/ast.cc
@@ -262,7 +262,7 @@ bool
ObjectLiteral::IsBoilerplateProperty(ObjectLiteral::Property* property) {
}
-void ObjectLiteral::BuildConstantProperties(Isolate* isolate, int* depth) {
+void ObjectLiteral::BuildConstantProperties(Isolate* isolate) {
if (!constant_properties_.is_null()) return;
// Allocate a fixed array to hold all the constant properties.
@@ -283,9 +283,8 @@ void ObjectLiteral::BuildConstantProperties(Isolate*
isolate, int* depth) {
}
MaterializedLiteral* m_literal =
property->value()->AsMaterializedLiteral();
if (m_literal != NULL) {
- int inner_depth = 1;
- m_literal->BuildConstants(isolate, &inner_depth);
- if (inner_depth >= depth_acc) depth_acc = inner_depth + 1;
+ m_literal->BuildConstants(isolate);
+ if (m_literal->depth() >= depth_acc) depth_acc = m_literal->depth()
+ 1;
}
// Add CONSTANT and COMPUTED properties to boilerplate. Use undefined
@@ -334,11 +333,11 @@ void ObjectLiteral::BuildConstantProperties(Isolate*
isolate, int* depth) {
fast_elements_ =
(max_element_index <= 32) || ((2 * elements) >= max_element_index);
set_is_simple(is_simple);
- if (depth != NULL) *depth = depth_acc;
+ set_depth(depth_acc);
}
-void ArrayLiteral::BuildConstantElements(Isolate* isolate, int* depth) {
+void ArrayLiteral::BuildConstantElements(Isolate* isolate) {
if (!constant_elements_.is_null()) return;
// Allocate a fixed array to hold all the object literals.
@@ -355,9 +354,10 @@ void ArrayLiteral::BuildConstantElements(Isolate*
isolate, int* depth) {
Expression* element = values()->at(i);
MaterializedLiteral* m_literal = element->AsMaterializedLiteral();
if (m_literal != NULL) {
- int inner_depth = 1;
- m_literal->BuildConstants(isolate, &inner_depth);
- if (inner_depth + 1 > depth_acc) depth_acc = inner_depth + 1;
+ m_literal->BuildConstants(isolate);
+ if (m_literal->depth() + 1 > depth_acc) {
+ depth_acc = m_literal->depth() + 1;
+ }
}
Handle<Object> boilerplate_value = GetBoilerplateValue(element,
isolate);
if (boilerplate_value->IsTheHole()) {
@@ -392,7 +392,7 @@ void ArrayLiteral::BuildConstantElements(Isolate*
isolate, int* depth) {
constant_elements_ = literals;
set_is_simple(is_simple);
- if (depth != NULL) *depth = depth_acc;
+ set_depth(depth_acc);
}
@@ -408,14 +408,15 @@ Handle<Object>
MaterializedLiteral::GetBoilerplateValue(Expression* expression,
}
-void MaterializedLiteral::BuildConstants(Isolate* isolate, int* depth) {
+void MaterializedLiteral::BuildConstants(Isolate* isolate) {
if (IsArrayLiteral()) {
- return AsArrayLiteral()->BuildConstantElements(isolate, depth);
+ return AsArrayLiteral()->BuildConstantElements(isolate);
}
if (IsObjectLiteral()) {
- return AsObjectLiteral()->BuildConstantProperties(isolate, depth);
+ return AsObjectLiteral()->BuildConstantProperties(isolate);
}
ASSERT(IsRegExpLiteral());
+ ASSERT(depth() >= 1); // Depth should be initialized.
}
Index: src/ast.h
diff --git a/src/ast.h b/src/ast.h
index
2a8669690d835a1d36682e385cfe4612fa9b67ef..acbdab3d8f2dfbad5f4c1c31f0628e81e1139368
100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -1409,13 +1409,20 @@ class MaterializedLiteral : public Expression {
int literal_index() { return literal_index_; }
+ int depth() const {
+ // only callable after initialization.
+ ASSERT(depth_ >= 1);
+ return depth_;
+ }
+
protected:
MaterializedLiteral(Isolate* isolate,
int literal_index,
int pos)
: Expression(isolate, pos),
literal_index_(literal_index),
- is_simple_(false) {}
+ is_simple_(false),
+ depth_(-1) {}
// A materialized literal is simple if the values consist of only
// constants and simple object and array literals.
@@ -1423,8 +1430,13 @@ class MaterializedLiteral : public Expression {
void set_is_simple(bool is_simple) { is_simple_ = is_simple; }
friend class CompileTimeValue;
+ void set_depth(int depth) {
+ ASSERT(depth >= 1);
+ depth_ = depth;
+ }
+
// Populate the constant properties/elements fixed array.
- void BuildConstants(Isolate* isolate, int* depth);
+ void BuildConstants(Isolate* isolate);
friend class ArrayLiteral;
friend class ObjectLiteral;
@@ -1438,6 +1450,7 @@ class MaterializedLiteral : public Expression {
private:
int literal_index_;
bool is_simple_;
+ int depth_;
};
@@ -1505,7 +1518,7 @@ class ObjectLiteral V8_FINAL : public
MaterializedLiteral {
static bool IsBoilerplateProperty(Property* property);
// Populate the constant properties fixed array.
- void BuildConstantProperties(Isolate* isolate, int* depth = NULL);
+ void BuildConstantProperties(Isolate* isolate);
// Mark all computed expressions that are bound to a key that
// is shadowed by a later occurrence of the same key. For the
@@ -1564,7 +1577,9 @@ class RegExpLiteral V8_FINAL : public
MaterializedLiteral {
int pos)
: MaterializedLiteral(isolate, literal_index, pos),
pattern_(pattern),
- flags_(flags) {}
+ flags_(flags) {
+ set_depth(1);
+ }
private:
Handle<String> pattern_;
@@ -1587,7 +1602,7 @@ class ArrayLiteral V8_FINAL : public
MaterializedLiteral {
}
// Populate the constant elements fixed array.
- void BuildConstantElements(Isolate* isolate, int* depth = NULL);
+ void BuildConstantElements(Isolate* isolate);
protected:
ArrayLiteral(Isolate* isolate,
Index: src/ia32/full-codegen-ia32.cc
diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index
bacfe835dbbeeeb20c66fcbb83870009b71324a1..86c525d7f63e034af66868d20a94b34338cf6d3a
100644
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -1575,8 +1575,7 @@ void FullCodeGenerator::EmitAccessor(Expression*
expression) {
void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
Comment cmnt(masm_, "[ ObjectLiteral");
- int depth = 1;
- expr->BuildConstantProperties(isolate(), &depth);
+ expr->BuildConstantProperties(isolate());
Handle<FixedArray> constant_properties = expr->constant_properties();
int flags = expr->fast_elements()
? ObjectLiteral::kFastElements
@@ -1586,7 +1585,7 @@ void
FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
: ObjectLiteral::kNoFlags;
int properties_count = constant_properties->length() / 2;
if ((FLAG_track_double_fields && expr->may_store_doubles()) ||
- depth > 1 || Serializer::enabled() ||
+ expr->depth() > 1 || Serializer::enabled() ||
flags != ObjectLiteral::kFastElements ||
properties_count >
FastCloneShallowObjectStub::kMaximumClonedProperties) {
__ mov(edi, Operand(ebp, JavaScriptFrameConstants::kFunctionOffset));
@@ -1705,8 +1704,7 @@ void
FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
Comment cmnt(masm_, "[ ArrayLiteral");
- int depth = 1;
- expr->BuildConstantElements(isolate(), &depth);
+ expr->BuildConstantElements(isolate());
ZoneList<Expression*>* subexprs = expr->values();
int length = subexprs->length();
Handle<FixedArray> constant_elements = expr->constant_elements();
@@ -1733,7 +1731,7 @@ void
FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
DONT_TRACK_ALLOCATION_SITE,
length);
__ CallStub(&stub);
- } else if (depth > 1 || Serializer::enabled() ||
+ } else if (expr->depth() > 1 || Serializer::enabled() ||
length > FastCloneShallowArrayStub::kMaximumClonedLength) {
__ mov(ebx, Operand(ebp, JavaScriptFrameConstants::kFunctionOffset));
__ push(FieldOperand(ebx, JSFunction::kLiteralsOffset));
Index: src/mips/full-codegen-mips.cc
diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc
index
9d93aafb9d6aed3b8148de70d9ddae680b0f899f..944a5e7cd0b4f85290ceb408286f77e0f2065832
100644
--- a/src/mips/full-codegen-mips.cc
+++ b/src/mips/full-codegen-mips.cc
@@ -1645,8 +1645,7 @@ void FullCodeGenerator::EmitAccessor(Expression*
expression) {
void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
Comment cmnt(masm_, "[ ObjectLiteral");
- int depth = 1;
- expr->BuildConstantProperties(isolate(), &depth);
+ expr->BuildConstantProperties(isolate());
Handle<FixedArray> constant_properties = expr->constant_properties();
__ lw(a3, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset));
__ lw(a3, FieldMemOperand(a3, JSFunction::kLiteralsOffset));
@@ -1661,7 +1660,7 @@ void
FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
__ li(a0, Operand(Smi::FromInt(flags)));
int properties_count = constant_properties->length() / 2;
if ((FLAG_track_double_fields && expr->may_store_doubles()) ||
- depth > 1 || Serializer::enabled() ||
+ expr->depth() > 1 || Serializer::enabled() ||
flags != ObjectLiteral::kFastElements ||
properties_count >
FastCloneShallowObjectStub::kMaximumClonedProperties) {
__ Push(a3, a2, a1, a0);
@@ -1780,8 +1779,7 @@ void
FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
Comment cmnt(masm_, "[ ArrayLiteral");
- int depth = 1;
- expr->BuildConstantElements(isolate(), &depth);
+ expr->BuildConstantElements(isolate());
ZoneList<Expression*>* subexprs = expr->values();
int length = subexprs->length();
@@ -1808,7 +1806,7 @@ void
FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
__ CallStub(&stub);
__ IncrementCounter(isolate()->counters()->cow_arrays_created_stub(),
1, a1, a2);
- } else if (depth > 1 || Serializer::enabled() ||
+ } else if (expr->depth() > 1 || Serializer::enabled() ||
length > FastCloneShallowArrayStub::kMaximumClonedLength) {
__ Push(a3, a2, a1);
__ CallRuntime(Runtime::kCreateArrayLiteral, 3);
Index: src/x64/full-codegen-x64.cc
diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index
8947e2f84c045e795cec832103ca3c0af43dc0c9..71b54680f2035d352122fe631018cdd09b8590cc
100644
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -1596,8 +1596,7 @@ void FullCodeGenerator::EmitAccessor(Expression*
expression) {
void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
Comment cmnt(masm_, "[ ObjectLiteral");
- int depth = 1;
- expr->BuildConstantProperties(isolate(), &depth);
+ expr->BuildConstantProperties(isolate());
Handle<FixedArray> constant_properties = expr->constant_properties();
int flags = expr->fast_elements()
? ObjectLiteral::kFastElements
@@ -1607,7 +1606,7 @@ void
FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
: ObjectLiteral::kNoFlags;
int properties_count = constant_properties->length() / 2;
if ((FLAG_track_double_fields && expr->may_store_doubles()) ||
- depth > 1 || Serializer::enabled() ||
+ expr->depth() > 1 || Serializer::enabled() ||
flags != ObjectLiteral::kFastElements ||
properties_count >
FastCloneShallowObjectStub::kMaximumClonedProperties) {
__ movq(rdi, Operand(rbp, JavaScriptFrameConstants::kFunctionOffset));
@@ -1726,8 +1725,7 @@ void
FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
Comment cmnt(masm_, "[ ArrayLiteral");
- int depth = 1;
- expr->BuildConstantElements(isolate(), &depth);
+ expr->BuildConstantElements(isolate());
ZoneList<Expression*>* subexprs = expr->values();
int length = subexprs->length();
Handle<FixedArray> constant_elements = expr->constant_elements();
@@ -1754,7 +1752,7 @@ void
FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
DONT_TRACK_ALLOCATION_SITE,
length);
__ CallStub(&stub);
- } else if (depth > 1 || Serializer::enabled() ||
+ } else if (expr->depth() > 1 || Serializer::enabled() ||
length > FastCloneShallowArrayStub::kMaximumClonedLength) {
__ movq(rbx, Operand(rbp, JavaScriptFrameConstants::kFunctionOffset));
__ push(FieldOperand(rbx, JSFunction::kLiteralsOffset));
--
--
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.
