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

@@ -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<int>(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<String> 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<const byte>(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<void*>(1);
   }
   return reinterpret_cast<AstRawString*>(entry->key);

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<String>::null() until internalized.
   Handle<String> string_;
+  AstString* next_;
   // Poor-man's virtual dispatch to AstRawString / AstConsString. Takes less
   // memory.
   class IsRawStringBits : public BitField<bool, 0, 1> {};
@@ -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<const AstRawString*>* strings_;
+    const AstRawString* strings_;
     const char* symbol_name_;
   };
 
-  // Internalized value (empty before internalized).
+  // Handle<String>::null() until internalized.
   Handle<Object> 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<const uint8_t> literal);
   AstRawString* GetTwoByteStringInternal(Vector<const uint16_t> 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<AstValue*> values_;
-  List<AstString*> 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_;
 

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;

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<String> AsPropertyName() {
-    DCHECK(IsPropertyName(HandleDereferenceMode::kDisallowed));
+    DCHECK(IsPropertyName());
     return Handle<String>::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);
   }