Overlay next_ and value_/string_ in AstValue/AstString

BUG=

Review-Url: https://codereview.chromium.org/2449223002
Cr-Commit-Position: refs/heads/master@{#40581}

src/ast/ast-value-factory.cc

@@ -97,13 +97,11 @@ void AstString::Internalize(Isolate* isolate) {
 }
 
 void AstRawString::Internalize(Isolate* isolate) {
-  // Skip over already internalized strings.
-  if (!string_.is_null()) return;
   if (literal_bytes_.length() == 0) {
-    string_ = isolate->factory()->empty_string();
+    set_string(isolate->factory()->empty_string());
   } else {
     AstRawStringInternalizationKey key(this);
-    string_ = StringTable::LookupKey(isolate, &key);
+    set_string(StringTable::LookupKey(isolate, &key));
   }
 }
 
@@ -133,9 +131,9 @@ bool AstRawString::IsOneByteEqualTo(const char* data) const {
 void AstConsString::Internalize(Isolate* isolate) {
   // AstRawStrings are internalized before AstConsStrings so left and right are
   // already internalized.
-  string_ = isolate->factory()
-                ->NewConsString(left_->string(), right_->string())
-                .ToHandleChecked();
+  set_string(isolate->factory()
+                 ->NewConsString(left_->string(), right_->string())
+                 .ToHandleChecked());
 }
 
 bool AstValue::IsPropertyName() const {
@@ -179,44 +177,44 @@ bool AstValue::BooleanValue() const {
 void AstValue::Internalize(Isolate* isolate) {
   switch (type_) {
     case STRING:
-      DCHECK(string_ != NULL);
+      DCHECK_NOT_NULL(string_);
       // Strings are already internalized.
       DCHECK(!string_->string().is_null());
       break;
     case SYMBOL:
       if (symbol_name_[0] == 'i') {
         DCHECK_EQ(0, strcmp(symbol_name_, "iterator_symbol"));
-        value_ = isolate->factory()->iterator_symbol();
+        set_value(isolate->factory()->iterator_symbol());
       } else if (strcmp(symbol_name_, "hasInstance_symbol") == 0) {
-        value_ = isolate->factory()->has_instance_symbol();
+        set_value(isolate->factory()->has_instance_symbol());
       } else {
         DCHECK_EQ(0, strcmp(symbol_name_, "home_object_symbol"));
-        value_ = isolate->factory()->home_object_symbol();
+        set_value(isolate->factory()->home_object_symbol());
       }
       break;
     case NUMBER_WITH_DOT:
     case NUMBER:
-      value_ = isolate->factory()->NewNumber(number_, TENURED);
+      set_value(isolate->factory()->NewNumber(number_, TENURED));
       break;
     case SMI_WITH_DOT:
     case SMI:
-      value_ = handle(Smi::FromInt(smi_), isolate);
+      set_value(handle(Smi::FromInt(smi_), isolate));
       break;
     case BOOLEAN:
       if (bool_) {
-        value_ = isolate->factory()->true_value();
+        set_value(isolate->factory()->true_value());
       } else {
-        value_ = isolate->factory()->false_value();
+        set_value(isolate->factory()->false_value());
       }
       break;
     case NULL_TYPE:
-      value_ = isolate->factory()->null_value();
+      set_value(isolate->factory()->null_value());
       break;
     case THE_HOLE:
-      value_ = isolate->factory()->the_hole_value();
+      set_value(isolate->factory()->the_hole_value());
       break;
     case UNDEFINED:
-      value_ = isolate->factory()->undefined_value();
+      set_value(isolate->factory()->undefined_value());
       break;
   }
 }
@@ -258,7 +256,7 @@ const AstConsString* AstValueFactory::NewConsString(
   // the AstRawString will not be moved).
   AstConsString* new_string = new (zone_) AstConsString(left, right);
   CHECK(new_string != nullptr);
-  AddConsString(new_string);
+  AddString(new_string);
   return new_string;
 }
 
@@ -297,15 +295,9 @@ const AstRawString* AstValueFactory::ConcatStrings(const AstRawString* left,
 
 void AstValueFactory::Internalize(Isolate* isolate) {
   // Strings need to be internalized before values, because values refer to
-  // strings. Internalize flat strings before cons strings since cons strings
-  // may point to flat strings.
-  for (base::CustomMatcherHashMap::Entry* entry = string_table_.Start();
-       entry != nullptr; entry = string_table_.Next(entry)) {
-    reinterpret_cast<AstRawString*>(entry->key)->Internalize(isolate);
-  }
-
-  for (AstConsString* current = cons_strings_; current != nullptr;) {
-    AstConsString* next = current->next();
+  // strings.
+  for (AstString* current = strings_; current != nullptr;) {
+    AstString* next = current->next();
     current->Internalize(isolate);
     current = next;
   }
@@ -315,14 +307,14 @@ void AstValueFactory::Internalize(Isolate* isolate) {
     current->Internalize(isolate);
     current = next;
   }
-  ResetConsStrings();
+  ResetStrings();
   values_ = nullptr;
 }
 
 
 const AstValue* AstValueFactory::NewString(const AstRawString* string) {
   AstValue* value = new (zone_) AstValue(string);
-  CHECK(string != nullptr);
+  CHECK_NOT_NULL(string);
   return AddValue(value);
 }
 
@@ -392,7 +384,8 @@ AstRawString* AstValueFactory::GetString(uint32_t hash, bool is_one_byte,
     memcpy(new_literal_bytes, literal_bytes.start(), length);
     AstRawString* new_string = new (zone_) AstRawString(
         is_one_byte, Vector<const byte>(new_literal_bytes, length), hash);
-    CHECK(new_string != nullptr);
+    CHECK_NOT_NULL(new_string);
+    AddString(new_string);
     entry->key = new_string;
     entry->value = reinterpret_cast<void*>(1);
   }

src/ast/ast-value-factory.h

@@ -44,7 +44,7 @@ namespace internal {
 class AstString : public ZoneObject {
  public:
   explicit AstString(bool is_raw)
-      : bit_field_(IsRawStringBits::encode(is_raw)) {}
+      : next_(nullptr), bit_field_(IsRawStringBits::encode(is_raw)) {}
 
   int length() const;
   bool IsEmpty() const { return length() == 0; }
@@ -54,13 +54,21 @@ class AstString : public ZoneObject {
 
   // This function can be called after internalizing.
   V8_INLINE Handle<String> string() const {
-    DCHECK(!string_.is_null());
-    return string_;
+    DCHECK_NOT_NULL(string_);
+    return Handle<String>(string_);
   }
 
+  AstString* next() { return next_; }
+  AstString** next_location() { return &next_; }
+
  protected:
-  // Handle<String>::null() until internalized.
-  Handle<String> string_;
+  void set_string(Handle<String> string) { string_ = string.location(); }
+  // {string_} is stored as String** instead of a Handle<String> so it can be
+  // stored in a union with {next_}.
+  union {
+    AstString* next_;
+    String** string_;
+  };
   // Poor-man's virtual dispatch to AstRawString / AstConsString. Takes less
   // memory.
   class IsRawStringBits : public BitField<bool, 0, 1> {};
@@ -129,21 +137,16 @@ class AstConsString final : public AstString {
       : AstString(false),
         length_(left->length() + right->length()),
         left_(left),
-        right_(right),
-        next_(nullptr) {}
+        right_(right) {}
 
   int length() const { return length_; }
 
   void Internalize(Isolate* isolate);
 
-  AstConsString* next() { return next_; }
-  AstConsString** next_location() { return &next_; }
-
  private:
   const int length_;
   const AstString* left_;
   const AstString* right_;
-  AstConsString* next_;
 };
 
 
@@ -205,13 +208,14 @@ class AstValue : public ZoneObject {
     if (type_ == STRING) {
       return string_->string();
     }
-    DCHECK(!value_.is_null());
-    return value_;
+    DCHECK_NOT_NULL(value_);
+    return Handle<Object>(value_);
   }
   AstValue* next() const { return next_; }
   void set_next(AstValue* next) { next_ = next; }
 
  private:
+  void set_value(Handle<Object> object) { value_ = object.location(); }
   friend class AstValueFactory;
 
   enum Type {
@@ -259,6 +263,13 @@ class AstValue : public ZoneObject {
 
   Type type_;
 
+  // {value_} is stored as Object** instead of a Handle<Object> so it can be
+  // stored in a union with {next_}.
+  union {
+    Object** value_;  // if internalized
+    AstValue* next_;  // if !internalized
+  };
+
   // Uninternalized value.
   union {
     const AstRawString* string_;
@@ -267,10 +278,6 @@ class AstValue : public ZoneObject {
     bool bool_;
     const char* symbol_name_;
   };
-
-  // Handle<String>::null() until internalized.
-  Handle<Object> value_;
-  AstValue* next_;
 };
 
 
@@ -325,8 +332,8 @@ class AstValueFactory {
   AstValueFactory(Zone* zone, uint32_t hash_seed)
       : string_table_(AstRawStringCompare),
         values_(nullptr),
-        cons_strings_(nullptr),
-        cons_strings_end_(&cons_strings_),
+        strings_(nullptr),
+        strings_end_(&strings_),
         zone_(zone),
         hash_seed_(hash_seed) {
 #define F(name, str) name##_string_ = NULL;
@@ -387,14 +394,14 @@ class AstValueFactory {
     values_ = value;
     return value;
   }
-  AstConsString* AddConsString(AstConsString* string) {
-    *cons_strings_end_ = string;
-    cons_strings_end_ = string->next_location();
+  AstString* AddString(AstString* string) {
+    *strings_end_ = string;
+    strings_end_ = string->next_location();
     return string;
   }
-  void ResetConsStrings() {
-    cons_strings_ = nullptr;
-    cons_strings_end_ = &cons_strings_;
+  void ResetStrings() {
+    strings_ = nullptr;
+    strings_end_ = &strings_;
   }
   V8_EXPORT_PRIVATE AstRawString* GetOneByteStringInternal(
       Vector<const uint8_t> literal);
@@ -409,10 +416,10 @@ class AstValueFactory {
   // For keeping track of all AstValues and AstRawStrings we've created (so that
   // they can be internalized later).
   AstValue* values_;
-  // We need to keep track of cons_strings_ in order since they require their
+  // We need to keep track of strings_ in order since cons strings require their
   // members to be internalized first.
-  AstConsString* cons_strings_;
-  AstConsString** cons_strings_end_;
+  AstString* strings_;
+  AstString** strings_end_;
   Zone* zone_;
 
   uint32_t hash_seed_;