Make the AstValueFactory more efficient and less memory hungry

src/ast/ast-value-factory.cc

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 79 matching lines @@
 }
 
 void AstString::Internalize(Isolate* isolate) {
   if (IsRawStringBits::decode(bit_field_)) {
     return reinterpret_cast<AstRawString*>(this)->Internalize(isolate);
   }
   return reinterpret_cast<AstConsString*>(this)->Internalize(isolate);
 }
 
 void AstRawString::Internalize(Isolate* isolate) {
-  if (!string_.is_null()) return;
   if (literal_bytes_.length() == 0) {
     string_ = isolate->factory()->empty_string();
   } else {
     AstRawStringInternalizationKey key(this);
     string_ = StringTable::LookupKey(isolate, &key);
   }
 }
 
 
 bool AstRawString::AsArrayIndex(uint32_t* index) const {
-  if (!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);
+  if ((hash_ & Name::kIsNotArrayIndexMask) != 0) return false;

[marja, 2016/08/10 08:33:17]

I was trying to follow which part sets this, and it was kinda far away. Could
you add a comment here, something like "kIsNotArrayIndexMap is set by
StringHasher."

+  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) {
     const char* token = reinterpret_cast<const char*>(literal_bytes_.start());
     return !strncmp(token, data, length);
   }
   return false;
@@ skipping 117 matching lines @@
     DisallowHeapAllocation no_gc;
     String::FlatContent content = literal->GetFlatContent();
     if (content.IsOneByte()) {
       result = GetOneByteStringInternal(content.ToOneByteVector());
     } else {
       DCHECK(content.IsTwoByte());
       result = GetTwoByteStringInternal(content.ToUC16Vector());
     }
   }
   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;
 }
 
 
 const AstConsString* AstValueFactory::NewConsString(
     const AstString* left, const AstString* right) {
   // This Vector will be valid as long as the Collector is alive (meaning that
   // 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);
   } else {
     GENERATE_VALUE_GETTER(false_value_, false);
   }
 }
 
 
 const AstValue* AstValueFactory::NewNull() {
@@ skipping 23 matching lines @@
   base::HashMap::Entry* entry = string_table_.LookupOrInsert(&key, hash);
   if (entry->value == NULL) {
     // Copy literal contents for later comparison.
     int length = literal_bytes.length();
     byte* new_literal_bytes = zone_->NewArray<byte>(length);
     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);
     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);
 }
 
 
 bool AstValueFactory::AstRawStringCompare(void* a, void* b) {
   const AstRawString* lhs = static_cast<AstRawString*>(a);
   const AstRawString* rhs = static_cast<AstRawString*>(b);
   DCHECK_EQ(lhs->hash(), rhs->hash());
@@ skipping 18 matching lines @@
                                   length) == 0;
     } else {
       return CompareCharsUnsigned(reinterpret_cast<const uint16_t*>(l),
                                   reinterpret_cast<const uint16_t*>(r),
                                   length) == 0;
     }
   }
 }
 }  // namespace internal
 }  // namespace v8