From cbe5d41d88f35dc8ee29d2e0f9ce739d9e132481 Mon Sep 17 00:00:00 2001 From: verwaest Date: Wed, 10 Aug 2016 04:55:01 -0700 Subject: [PATCH] Make the AstValueFactory more efficient and less memory hungry This makes strings_ and values_ in AstValueFactory a linked list through the AstString and AstValue objects. Additionally the CL computes whether strings are convertible to array indexes directly using the AstString's hash + literal bytes just as Name does, rather than indirecting over name if available. BUG= Review-Url: https://codereview.chromium.org/2225423002 Cr-Commit-Position: refs/heads/master@{#38536} --- src/ast/ast-value-factory.cc | 98 ++++++++++++++++-------------------- src/ast/ast-value-factory.h | 71 +++++++++++++++++++------- src/ast/ast.cc | 6 +-- src/ast/ast.h | 21 +++----- 4 files changed, 104 insertions(+), 92 deletions(-) diff --git a/src/ast/ast-value-factory.cc b/src/ast/ast-value-factory.cc index f6ead8d6e4..a18f6141cf 100644 --- a/src/ast/ast-value-factory.cc +++ b/src/ast/ast-value-factory.cc @@ -97,7 +97,6 @@ void AstString::Internalize(Isolate* isolate) { } void AstRawString::Internalize(Isolate* isolate) { - if (!string_.is_null()) return; if (literal_bytes_.length() == 0) { string_ = isolate->factory()->empty_string(); } else { @@ -106,18 +105,19 @@ void AstRawString::Internalize(Isolate* isolate) { } } -bool AstRawString::AsArrayIndex(uint32_t* index, - HandleDereferenceMode deref_mode) const { - if (deref_mode == HandleDereferenceMode::kAllowed && !string_.is_null()) - return string_->AsArrayIndex(index); - if (!is_one_byte() || literal_bytes_.length() == 0 || - literal_bytes_.length() > String::kMaxArrayIndexSize) - return false; - OneByteStringStream stream(literal_bytes_); - return StringToArrayIndex(&stream, index); +bool AstRawString::AsArrayIndex(uint32_t* index) const { + // The StringHasher will set up the hash in such a way that we can use it to + // figure out whether the string is convertible to an array index. + if ((hash_ & Name::kIsNotArrayIndexMask) != 0) return false; + if (length() <= Name::kMaxCachedArrayIndexLength) { + *index = Name::ArrayIndexValueBits::decode(hash_); + } else { + OneByteStringStream stream(literal_bytes_); + CHECK(StringToArrayIndex(&stream, index)); + } + return true; } - bool AstRawString::IsOneByteEqualTo(const char* data) const { int length = static_cast(strlen(data)); if (is_one_byte() && literal_bytes_.length() == length) { @@ -136,10 +136,10 @@ void AstConsString::Internalize(Isolate* isolate) { .ToHandleChecked(); } -bool AstValue::IsPropertyName(HandleDereferenceMode deref_mode) const { +bool AstValue::IsPropertyName() const { if (type_ == STRING) { uint32_t index; - return !string_->AsArrayIndex(&index, deref_mode); + return !string_->AsArrayIndex(&index); } return false; } @@ -251,7 +251,13 @@ const AstRawString* AstValueFactory::GetString(Handle literal) { } } isolate_ = saved_isolate; - if (isolate_) result->Internalize(isolate_); + if (strings_ != nullptr && isolate_) { + // Only the string we are creating is uninternalized at this point. + DCHECK_EQ(result, strings_); + DCHECK_NULL(strings_->next()); + result->Internalize(isolate_); + ResetStrings(); + } return result; } @@ -262,85 +268,68 @@ const AstConsString* AstValueFactory::NewConsString( // the AstRawString will not be moved). AstConsString* new_string = new (zone_) AstConsString(left, right); CHECK(new_string != nullptr); - strings_.Add(new_string); - if (isolate_) { - new_string->Internalize(isolate_); - } + AddString(new_string); return new_string; } void AstValueFactory::Internalize(Isolate* isolate) { if (isolate_) { + DCHECK_NULL(strings_); + DCHECK_NULL(values_); // Everything is already internalized. return; } // Strings need to be internalized before values, because values refer to // strings. - for (int i = 0; i < strings_.length(); ++i) { - strings_[i]->Internalize(isolate); + for (AstString* current = strings_; current != nullptr;) { + AstString* next = current->next(); + current->Internalize(isolate); + current = next; } - for (int i = 0; i < values_.length(); ++i) { - values_[i]->Internalize(isolate); + for (AstValue* current = values_; current != nullptr;) { + AstValue* next = current->next(); + current->Internalize(isolate); + current = next; } isolate_ = isolate; + ResetStrings(); + values_ = nullptr; } const AstValue* AstValueFactory::NewString(const AstRawString* string) { AstValue* value = new (zone_) AstValue(string); CHECK(string != nullptr); - if (isolate_) { - value->Internalize(isolate_); - } - values_.Add(value); - return value; + return AddValue(value); } const AstValue* AstValueFactory::NewSymbol(const char* name) { AstValue* value = new (zone_) AstValue(name); - if (isolate_) { - value->Internalize(isolate_); - } - values_.Add(value); - return value; + return AddValue(value); } const AstValue* AstValueFactory::NewNumber(double number, bool with_dot) { AstValue* value = new (zone_) AstValue(number, with_dot); - if (isolate_) { - value->Internalize(isolate_); - } - values_.Add(value); - return value; + return AddValue(value); } const AstValue* AstValueFactory::NewSmi(int number) { AstValue* value = new (zone_) AstValue(AstValue::SMI, number); - if (isolate_) { - value->Internalize(isolate_); - } - values_.Add(value); - return value; + return AddValue(value); } - -#define GENERATE_VALUE_GETTER(value, initializer) \ - if (!value) { \ - value = new (zone_) AstValue(initializer); \ - if (isolate_) { \ - value->Internalize(isolate_); \ - } \ - values_.Add(value); \ - } \ +#define GENERATE_VALUE_GETTER(value, initializer) \ + if (!value) { \ + value = AddValue(new (zone_) AstValue(initializer)); \ + } \ return value; - const AstValue* AstValueFactory::NewBoolean(bool b) { if (b) { GENERATE_VALUE_GETTER(true_value_, true); @@ -384,10 +373,7 @@ AstRawString* AstValueFactory::GetString(uint32_t hash, bool is_one_byte, is_one_byte, Vector(new_literal_bytes, length), hash); CHECK(new_string != nullptr); entry->key = new_string; - strings_.Add(new_string); - if (isolate_) { - new_string->Internalize(isolate_); - } + AddString(new_string); entry->value = reinterpret_cast(1); } return reinterpret_cast(entry->key); diff --git a/src/ast/ast-value-factory.h b/src/ast/ast-value-factory.h index 21103a0e7a..818bcc7dc3 100644 --- a/src/ast/ast-value-factory.h +++ b/src/ast/ast-value-factory.h @@ -43,9 +43,7 @@ namespace internal { class AstString : public ZoneObject { public: explicit AstString(bool is_raw) - : bit_field_(IsRawStringBits::encode(is_raw)) {} - - ~AstString() {} + : next_(nullptr), bit_field_(IsRawStringBits::encode(is_raw)) {} int length() const; bool IsEmpty() const { return length() == 0; } @@ -59,9 +57,13 @@ class AstString : public ZoneObject { return string_; } + AstString** next_location() { return &next_; } + AstString* next() const { return next_; } + protected: - // This is null until the string is internalized. + // Handle::null() until internalized. Handle string_; + AstString* next_; // Poor-man's virtual dispatch to AstRawString / AstConsString. Takes less // memory. class IsRawStringBits : public BitField {}; @@ -80,8 +82,7 @@ class AstRawString final : public AstString { void Internalize(Isolate* isolate); - bool AsArrayIndex(uint32_t* index, HandleDereferenceMode deref_mode = - HandleDereferenceMode::kAllowed) const; + bool AsArrayIndex(uint32_t* index) const; // The string is not null-terminated, use length() to find out the length. const unsigned char* raw_data() const { @@ -181,8 +182,7 @@ class AstValue : public ZoneObject { return type_ == STRING && string_ == string; } - bool IsPropertyName( - HandleDereferenceMode deref_mode = HandleDereferenceMode::kAllowed) const; + bool IsPropertyName() const; bool BooleanValue() const; @@ -203,6 +203,8 @@ class AstValue : public ZoneObject { DCHECK(!value_.is_null()); return value_; } + AstValue* next() const { return next_; } + void set_next(AstValue* next) { next_ = next; } private: friend class AstValueFactory; @@ -219,11 +221,15 @@ class AstValue : public ZoneObject { THE_HOLE }; - explicit AstValue(const AstRawString* s) : type_(STRING) { string_ = s; } + explicit AstValue(const AstRawString* s) : type_(STRING), next_(nullptr) { + string_ = s; + } - explicit AstValue(const char* name) : type_(SYMBOL) { symbol_name_ = name; } + explicit AstValue(const char* name) : type_(SYMBOL), next_(nullptr) { + symbol_name_ = name; + } - explicit AstValue(double n, bool with_dot) { + explicit AstValue(double n, bool with_dot) : next_(nullptr) { if (with_dot) { type_ = NUMBER_WITH_DOT; number_ = n; @@ -239,14 +245,14 @@ class AstValue : public ZoneObject { } } - AstValue(Type t, int i) : type_(t) { + AstValue(Type t, int i) : type_(t), next_(nullptr) { DCHECK(type_ == SMI); smi_ = i; } - explicit AstValue(bool b) : type_(BOOLEAN) { bool_ = b; } + explicit AstValue(bool b) : type_(BOOLEAN), next_(nullptr) { bool_ = b; } - explicit AstValue(Type t) : type_(t) { + explicit AstValue(Type t) : type_(t), next_(nullptr) { DCHECK(t == NULL_TYPE || t == UNDEFINED || t == THE_HOLE); } @@ -258,12 +264,13 @@ class AstValue : public ZoneObject { double number_; int smi_; bool bool_; - ZoneList* strings_; + const AstRawString* strings_; const char* symbol_name_; }; - // Internalized value (empty before internalized). + // Handle::null() until internalized. Handle value_; + AstValue* next_; }; @@ -316,9 +323,12 @@ class AstValueFactory { public: AstValueFactory(Zone* zone, uint32_t hash_seed) : string_table_(AstRawStringCompare), + values_(nullptr), + strings_end_(&strings_), zone_(zone), isolate_(NULL), hash_seed_(hash_seed) { + ResetStrings(); #define F(name, str) name##_string_ = NULL; STRING_CONSTANTS(F) #undef F @@ -373,6 +383,28 @@ class AstValueFactory { const AstValue* NewTheHole(); private: + AstValue* AddValue(AstValue* value) { + if (isolate_) { + value->Internalize(isolate_); + } else { + value->set_next(values_); + values_ = value; + } + return value; + } + AstString* AddString(AstString* string) { + if (isolate_) { + string->Internalize(isolate_); + } else { + *strings_end_ = string; + strings_end_ = string->next_location(); + } + return string; + } + void ResetStrings() { + strings_ = nullptr; + strings_end_ = &strings_; + } AstRawString* GetOneByteStringInternal(Vector literal); AstRawString* GetTwoByteStringInternal(Vector literal); AstRawString* GetString(uint32_t hash, bool is_one_byte, @@ -384,8 +416,11 @@ class AstValueFactory { base::HashMap string_table_; // For keeping track of all AstValues and AstRawStrings we've created (so that // they can be internalized later). - List values_; - List strings_; + AstValue* values_; + // We need to keep track of strings_ in order, since cons strings require + // their members to be internalized first. + AstString* strings_; + AstString** strings_end_; Zone* zone_; Isolate* isolate_; diff --git a/src/ast/ast.cc b/src/ast/ast.cc index ee39bfc4da..c1b424488a 100644 --- a/src/ast/ast.cc +++ b/src/ast/ast.cc @@ -74,8 +74,8 @@ bool Expression::IsStringLiteral() const { return IsLiteral() && AsLiteral()->value()->IsString(); } -bool Expression::IsPropertyName(HandleDereferenceMode deref_mode) const { - return IsLiteral() && AsLiteral()->IsPropertyName(deref_mode); +bool Expression::IsPropertyName() const { + return IsLiteral() && AsLiteral()->IsPropertyName(); } bool Expression::IsNullLiteral() const { @@ -933,7 +933,7 @@ Call::CallType Call::GetCallType(Isolate* isolate, Property* property = expression()->AsProperty(); if (property != nullptr) { bool is_super = property->IsSuperAccess(); - if (property->key()->IsPropertyName(deref_mode)) { + if (property->key()->IsPropertyName()) { return is_super ? NAMED_SUPER_PROPERTY_CALL : NAMED_PROPERTY_CALL; } else { return is_super ? KEYED_SUPER_PROPERTY_CALL : KEYED_PROPERTY_CALL; diff --git a/src/ast/ast.h b/src/ast/ast.h index 898927d57a..ebb3220801 100644 --- a/src/ast/ast.h +++ b/src/ast/ast.h @@ -314,10 +314,7 @@ class Expression : public AstNode { // Symbols that cannot be parsed as array indices are considered property // names. We do not treat symbols that can be array indexes as property // names because [] for string objects is handled only by keyed ICs. - // Produces the same result whether |deref_mode| is kAllowed or kDisallowed, - // although may be faster with kAllowed. - bool IsPropertyName( - HandleDereferenceMode deref_mode = HandleDereferenceMode::kAllowed) const; + bool IsPropertyName() const; // True iff the expression is a class or function expression without // a syntactic name. @@ -1196,15 +1193,11 @@ class SloppyBlockFunctionStatement final : public Statement { class Literal final : public Expression { public: // Returns true if literal represents a property name (i.e. cannot be parsed - // as array indices). Produces the same result whether |deref_mode| is - // kAllowed or kDisallowed, although may be faster with kAllowed. - bool IsPropertyName(HandleDereferenceMode deref_mode = - HandleDereferenceMode::kAllowed) const { - return value_->IsPropertyName(deref_mode); - } + // as array indices). + bool IsPropertyName() const { return value_->IsPropertyName(); } Handle AsPropertyName() { - DCHECK(IsPropertyName(HandleDereferenceMode::kDisallowed)); + DCHECK(IsPropertyName()); return Handle::cast(value()); } @@ -1744,15 +1737,13 @@ class Property final : public Expression { return property_feedback_slot_; } - // Returns the properties assign type. Produces the same result whether - // |deref_mode| is kAllowed or kDisallowed, although may be faster with - // kAllowed. + // Returns the properties assign type. static LhsKind GetAssignType( Property* property, HandleDereferenceMode deref_mode = HandleDereferenceMode::kAllowed) { if (property == NULL) return VARIABLE; bool super_access = property->IsSuperAccess(); - return (property->key()->IsPropertyName(deref_mode)) + return (property->key()->IsPropertyName()) ? (super_access ? NAMED_SUPER_PROPERTY : NAMED_PROPERTY) : (super_access ? KEYED_SUPER_PROPERTY : KEYED_PROPERTY); }