[runtime] Cache hash on HashTableKey and hash_field on StringTableKey

This CL
- removes InternalizeStringIfExists/LookupStringIfExists
- makes the distinction between hash_field and hash clear to AstRawString

Bug: 
Change-Id: Ia98c2236be4154a7db2741f2cf73681cfdcf03c7
Reviewed-on: https://chromium-review.googlesource.com/532954
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45902}

src/ast/ast-value-factory.cc

@@ -67,14 +67,15 @@ class AstRawStringInternalizationKey : public StringTableKey {
         Vector<const uint16_t>::cast(string_->literal_bytes_));
   }
 
-  uint32_t Hash() override { return string_->hash() >> Name::kHashShift; }
+  uint32_t ComputeHashField() override { return string_->hash_field(); }
 
   Handle<Object> AsHandle(Isolate* isolate) override {
     if (string_->is_one_byte())
       return isolate->factory()->NewOneByteInternalizedString(
-          string_->literal_bytes_, string_->hash());
+          string_->literal_bytes_, string_->hash_field());
     return isolate->factory()->NewTwoByteInternalizedString(
-        Vector<const uint16_t>::cast(string_->literal_bytes_), string_->hash());
+        Vector<const uint16_t>::cast(string_->literal_bytes_),
+        string_->hash_field());
   }
 
  private:
@@ -94,9 +95,9 @@ void AstRawString::Internalize(Isolate* isolate) {
 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 ((hash_field_ & Name::kIsNotArrayIndexMask) != 0) return false;
   if (length() <= Name::kMaxCachedArrayIndexLength) {
-    *index = Name::ArrayIndexValueBits::decode(hash_);
+    *index = Name::ArrayIndexValueBits::decode(hash_field_);
   } else {
     OneByteStringStream stream(literal_bytes_);
     CHECK(StringToArrayIndex(&stream, index));
@@ -123,7 +124,7 @@ uint16_t AstRawString::FirstCharacter() const {
 bool AstRawString::Compare(void* a, void* b) {
   const AstRawString* lhs = static_cast<AstRawString*>(a);
   const AstRawString* rhs = static_cast<AstRawString*>(b);
-  DCHECK_EQ(lhs->hash(), rhs->hash());
+  DCHECK_EQ(lhs->Hash(), rhs->Hash());
 
   if (lhs->length() != rhs->length()) return false;
   const unsigned char* l = lhs->raw_data();
@@ -248,23 +249,23 @@ AstRawString* AstValueFactory::GetOneByteStringInternal(
   if (literal.length() == 1 && IsInRange(literal[0], 'a', 'z')) {
     int key = literal[0] - 'a';
     if (one_character_strings_[key] == nullptr) {
-      uint32_t hash = StringHasher::HashSequentialString<uint8_t>(
+      uint32_t hash_field = StringHasher::HashSequentialString<uint8_t>(
           literal.start(), literal.length(), hash_seed_);
-      one_character_strings_[key] = GetString(hash, true, literal);
+      one_character_strings_[key] = GetString(hash_field, true, literal);
     }
     return one_character_strings_[key];
   }
-  uint32_t hash = StringHasher::HashSequentialString<uint8_t>(
+  uint32_t hash_field = StringHasher::HashSequentialString<uint8_t>(
       literal.start(), literal.length(), hash_seed_);
-  return GetString(hash, true, literal);
+  return GetString(hash_field, true, literal);
 }
 
 
 AstRawString* AstValueFactory::GetTwoByteStringInternal(
     Vector<const uint16_t> literal) {
-  uint32_t hash = StringHasher::HashSequentialString<uint16_t>(
+  uint32_t hash_field = StringHasher::HashSequentialString<uint16_t>(
       literal.start(), literal.length(), hash_seed_);
-  return GetString(hash, false, Vector<const byte>::cast(literal));
+  return GetString(hash_field, false, Vector<const byte>::cast(literal));
 }
 
 
@@ -380,21 +381,21 @@ const AstValue* AstValueFactory::NewTheHole() {
 
 #undef GENERATE_VALUE_GETTER
 
-AstRawString* AstValueFactory::GetString(uint32_t hash, bool is_one_byte,
+AstRawString* AstValueFactory::GetString(uint32_t hash_field, bool is_one_byte,
                                          Vector<const byte> literal_bytes) {
   // literal_bytes here points to whatever the user passed, and this is OK
   // because we use vector_compare (which checks the contents) to compare
   // against the AstRawStrings which are in the string_table_. We should not
   // return this AstRawString.
-  AstRawString key(is_one_byte, literal_bytes, hash);
-  base::HashMap::Entry* entry = string_table_.LookupOrInsert(&key, hash);
+  AstRawString key(is_one_byte, literal_bytes, hash_field);
+  base::HashMap::Entry* entry = string_table_.LookupOrInsert(&key, key.Hash());
   if (entry->value == nullptr) {
     // 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);
+        is_one_byte, Vector<const byte>(new_literal_bytes, length), hash_field);
     CHECK_NOT_NULL(new_string);
     AddString(new_string);
     entry->key = new_string;

src/ast/ast-value-factory.h

@@ -64,9 +64,8 @@ class AstRawString final : public ZoneObject {
   }
 
   // For storing AstRawStrings in a hash map.
-  uint32_t hash() const {
-    return hash_;
-  }
+  uint32_t hash_field() const { return hash_field_; }
+  uint32_t Hash() const { return hash_field_ >> Name::kHashShift; }
 
   // This function can be called after internalizing.
   V8_INLINE Handle<String> string() const {
@@ -83,10 +82,10 @@ class AstRawString final : public ZoneObject {
   // Members accessed only by the AstValueFactory & related classes:
   static bool Compare(void* a, void* b);
   AstRawString(bool is_one_byte, const Vector<const byte>& literal_bytes,
-               uint32_t hash)
+               uint32_t hash_field)
       : next_(nullptr),
         literal_bytes_(literal_bytes),
-        hash_(hash),
+        hash_field_(hash_field),
         is_one_byte_(is_one_byte) {}
   AstRawString* next() {
     DCHECK(!has_string_);
@@ -114,7 +113,7 @@ class AstRawString final : public ZoneObject {
   };
 
   Vector<const byte> literal_bytes_;  // Memory owned by Zone.
-  uint32_t hash_;
+  uint32_t hash_field_;
   bool is_one_byte_;
 #ifdef DEBUG
   // (Debug-only:) Verify the object life-cylce: Some functions may only be
@@ -367,21 +366,21 @@ class AstStringConstants final {
         string_table_(AstRawString::Compare),
         hash_seed_(hash_seed) {
     DCHECK(ThreadId::Current().Equals(isolate->thread_id()));
-#define F(name, str)                                                      \
-  {                                                                       \
-    const char* data = str;                                               \
-    Vector<const uint8_t> literal(reinterpret_cast<const uint8_t*>(data), \
-                                  static_cast<int>(strlen(data)));        \
-    uint32_t hash = StringHasher::HashSequentialString<uint8_t>(          \
-        literal.start(), literal.length(), hash_seed_);                   \
-    name##_string_ = new (&zone_) AstRawString(true, literal, hash);      \
-    /* The Handle returned by the factory is located on the roots */      \
-    /* array, not on the temporary HandleScope, so this is safe.  */      \
-    name##_string_->set_string(isolate->factory()->name##_string());      \
-    base::HashMap::Entry* entry =                                         \
-        string_table_.InsertNew(name##_string_, name##_string_->hash());  \
-    DCHECK(entry->value == nullptr);                                      \
-    entry->value = reinterpret_cast<void*>(1);                            \
+#define F(name, str)                                                       \
+  {                                                                        \
+    const char* data = str;                                                \
+    Vector<const uint8_t> literal(reinterpret_cast<const uint8_t*>(data),  \
+                                  static_cast<int>(strlen(data)));         \
+    uint32_t hash_field = StringHasher::HashSequentialString<uint8_t>(     \
+        literal.start(), literal.length(), hash_seed_);                    \
+    name##_string_ = new (&zone_) AstRawString(true, literal, hash_field); \
+    /* The Handle returned by the factory is located on the roots */       \
+    /* array, not on the temporary HandleScope, so this is safe.  */       \
+    name##_string_->set_string(isolate->factory()->name##_string());       \
+    base::HashMap::Entry* entry =                                          \
+        string_table_.InsertNew(name##_string_, name##_string_->Hash());   \
+    DCHECK_NULL(entry->value);                                             \
+    entry->value = reinterpret_cast<void*>(1);                             \
   }
     STRING_CONSTANTS(F)
 #undef F

src/ast/ast.cc

@@ -1140,7 +1140,7 @@ void CaseClause::AssignFeedbackSlots(FeedbackVectorSpec* spec,
 
 uint32_t Literal::Hash() {
   return raw_value()->IsString()
-             ? raw_value()->AsString()->hash()
+             ? raw_value()->AsString()->Hash()
              : ComputeLongHash(double_to_uint64(raw_value()->AsNumber()));
 }
 

src/ast/context-slot-cache.cc

@@ -30,8 +30,9 @@ int ContextSlotCache::Lookup(Object* data, String* name, VariableMode* mode,
                              InitializationFlag* init_flag,
                              MaybeAssignedFlag* maybe_assigned_flag) {
   int index = Hash(data, name);
+  DCHECK(name->IsInternalizedString());
   Key& key = keys_[index];
-  if ((key.data == data) && key.name->Equals(name)) {
+  if (key.data == data && key.name == name) {
     Value result(values_[index]);
     if (mode != nullptr) *mode = result.mode();
     if (init_flag != nullptr) *init_flag = result.initialization_flag();
@@ -46,23 +47,18 @@ void ContextSlotCache::Update(Handle<Object> data, Handle<String> name,
                               VariableMode mode, InitializationFlag init_flag,
                               MaybeAssignedFlag maybe_assigned_flag,
                               int slot_index) {
-  DisallowHeapAllocation no_gc;
-  Handle<String> internalized_name;
-  DCHECK(slot_index > kNotFound);
-  if (StringTable::InternalizeStringIfExists(name->GetIsolate(), name)
-          .ToHandle(&internalized_name)) {
-    int index = Hash(*data, *internalized_name);
-    Key& key = keys_[index];
-    key.data = *data;
-    key.name = *internalized_name;
-    // Please note value only takes a uint as index.
-    values_[index] =
-        Value(mode, init_flag, maybe_assigned_flag, slot_index - kNotFound)
-            .raw();
+  DCHECK(name->IsInternalizedString());
+  DCHECK_LT(kNotFound, slot_index);
+  int index = Hash(*data, *name);
+  Key& key = keys_[index];
+  key.data = *data;
+  key.name = *name;
+  // Please note value only takes a uint as index.
+  values_[index] =
+      Value(mode, init_flag, maybe_assigned_flag, slot_index - kNotFound).raw();
 #ifdef DEBUG
-    ValidateEntry(data, name, mode, init_flag, maybe_assigned_flag, slot_index);
+  ValidateEntry(data, name, mode, init_flag, maybe_assigned_flag, slot_index);
 #endif
-  }
 }
 
 void ContextSlotCache::Clear() {
@@ -76,20 +72,16 @@ void ContextSlotCache::ValidateEntry(Handle<Object> data, Handle<String> name,
                                      InitializationFlag init_flag,
                                      MaybeAssignedFlag maybe_assigned_flag,
                                      int slot_index) {
-  DisallowHeapAllocation no_gc;
-  Handle<String> internalized_name;
-  if (StringTable::InternalizeStringIfExists(name->GetIsolate(), name)
-          .ToHandle(&internalized_name)) {
-    int index = Hash(*data, *name);
-    Key& key = keys_[index];
-    DCHECK(key.data == *data);
-    DCHECK(key.name->Equals(*name));
-    Value result(values_[index]);
-    DCHECK(result.mode() == mode);
-    DCHECK(result.initialization_flag() == init_flag);
-    DCHECK(result.maybe_assigned_flag() == maybe_assigned_flag);
-    DCHECK(result.index() + kNotFound == slot_index);
-  }
+  DCHECK(name->IsInternalizedString());
+  int index = Hash(*data, *name);
+  Key& key = keys_[index];
+  DCHECK_EQ(key.data, *data);
+  DCHECK_EQ(key.name, *name);
+  Value result(values_[index]);
+  DCHECK_EQ(result.mode(), mode);
+  DCHECK_EQ(result.initialization_flag(), init_flag);
+  DCHECK_EQ(result.maybe_assigned_flag(), maybe_assigned_flag);
+  DCHECK_EQ(result.index() + kNotFound, slot_index);
 }
 
 #endif  // DEBUG

src/ast/scopes.cc

@@ -54,7 +54,7 @@ Variable* VariableMap::Declare(Zone* zone, Scope* scope,
   // by the same AstRawString*.
   // FIXME(marja): fix the type of Lookup.
   Entry* p =
-      ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->hash(),
+      ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->Hash(),
                                   ZoneAllocationPolicy(zone));
   if (added) *added = p->value == nullptr;
   if (p->value == nullptr) {
@@ -69,7 +69,7 @@ Variable* VariableMap::Declare(Zone* zone, Scope* scope,
 Variable* VariableMap::DeclareName(Zone* zone, const AstRawString* name,
                                    VariableMode mode) {
   Entry* p =
-      ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->hash(),
+      ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->Hash(),
                                   ZoneAllocationPolicy(zone));
   if (p->value == nullptr) {
     // The variable has not been declared yet -> insert it.
@@ -82,13 +82,13 @@ Variable* VariableMap::DeclareName(Zone* zone, const AstRawString* name,
 
 void VariableMap::Remove(Variable* var) {
   const AstRawString* name = var->raw_name();
-  ZoneHashMap::Remove(const_cast<AstRawString*>(name), name->hash());
+  ZoneHashMap::Remove(const_cast<AstRawString*>(name), name->Hash());
 }
 
 void VariableMap::Add(Zone* zone, Variable* var) {
   const AstRawString* name = var->raw_name();
   Entry* p =
-      ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->hash(),
+      ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->Hash(),
                                   ZoneAllocationPolicy(zone));
   DCHECK_NULL(p->value);
   DCHECK_EQ(name, p->key);
@@ -96,7 +96,7 @@ void VariableMap::Add(Zone* zone, Variable* var) {
 }
 
 Variable* VariableMap::Lookup(const AstRawString* name) {
-  Entry* p = ZoneHashMap::Lookup(const_cast<AstRawString*>(name), name->hash());
+  Entry* p = ZoneHashMap::Lookup(const_cast<AstRawString*>(name), name->Hash());
   if (p != NULL) {
     DCHECK(reinterpret_cast<const AstRawString*>(p->key) == name);
     DCHECK(p->value != NULL);
@@ -121,7 +121,7 @@ void SloppyBlockFunctionMap::Declare(Zone* zone, const AstRawString* name,
   // AstRawStrings are unambiguous, i.e., the same string is always represented
   // by the same AstRawString*.
   Entry* p =
-      ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->hash(),
+      ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->Hash(),
                                   ZoneAllocationPolicy(zone));
   delegate->set_next(static_cast<SloppyBlockFunctionMap::Delegate*>(p->value));
   p->value = delegate;
@@ -1149,7 +1149,7 @@ Variable* Scope::DeclareVariable(
             GetDeclarationScope()->sloppy_block_function_map();
         duplicate_allowed = map != nullptr &&
                             map->Lookup(const_cast<AstRawString*>(name),
-                                        name->hash()) != nullptr &&
+                                        name->Hash()) != nullptr &&
                             !IsAsyncFunction(function_kind) &&
                             !(allow_harmony_restrictive_generators &&
                               IsGeneratorFunction(function_kind));

src/interpreter/constant-array-builder.cc

@@ -184,7 +184,7 @@ size_t ConstantArrayBuilder::Insert(Smi* smi) {
 size_t ConstantArrayBuilder::Insert(const AstRawString* raw_string) {
   return constants_map_
       .LookupOrInsert(reinterpret_cast<intptr_t>(raw_string),
-                      raw_string->hash(),
+                      raw_string->Hash(),
                       [&]() { return AllocateIndex(Entry(raw_string)); },
                       ZoneAllocationPolicy(zone_))
       ->value;

src/objects-inl.h

@@ -2669,6 +2669,10 @@ uint32_t StringSetShape::HashForObject(Object* object) {
   return String::cast(object)->Hash();
 }
 
+uint32_t StringTableKey::ComputeHash() {
+  return HashField() >> Name::kHashShift;
+}
+
 Handle<Object> StringTableShape::AsHandle(Isolate* isolate,
                                           StringTableKey* key) {
   return key->AsHandle(isolate);

src/objects.cc

@@ -15917,7 +15917,7 @@ class StringSharedKey : public HashTableKey {
     return source->Equals(*source_);
   }
 
-  uint32_t Hash() override {
+  uint32_t ComputeHash() override {
     return CompilationCacheShape::StringSharedHash(*source_, *shared_,
                                                    language_mode_, position_);
   }
@@ -16156,7 +16156,7 @@ class RegExpKey : public HashTableKey {
         && (flags_ == val->get(JSRegExp::kFlagsIndex));
   }
 
-  uint32_t Hash() override {
+  uint32_t ComputeHash() override {
     return CompilationCacheShape::RegExpHash(*string_, flags_);
   }
 
@@ -16166,21 +16166,18 @@ class RegExpKey : public HashTableKey {
 
 
 Handle<Object> OneByteStringKey::AsHandle(Isolate* isolate) {
-  if (hash_field_ == 0) Hash();
-  return isolate->factory()->NewOneByteInternalizedString(string_, hash_field_);
+  return isolate->factory()->NewOneByteInternalizedString(string_, HashField());
 }
 
 
 Handle<Object> TwoByteStringKey::AsHandle(Isolate* isolate) {
-  if (hash_field_ == 0) Hash();
-  return isolate->factory()->NewTwoByteInternalizedString(string_, hash_field_);
+  return isolate->factory()->NewTwoByteInternalizedString(string_, HashField());
 }
 
 
 Handle<Object> SeqOneByteSubStringKey::AsHandle(Isolate* isolate) {
-  if (hash_field_ == 0) Hash();
   return isolate->factory()->NewOneByteInternalizedSubString(
-      string_, from_, length_, hash_field_);
+      string_, from_, length_, HashField());
 }
 
 
@@ -16194,13 +16191,19 @@ bool SeqOneByteSubStringKey::IsMatch(Object* string) {
 class InternalizedStringKey : public StringTableKey {
  public:
   explicit InternalizedStringKey(Handle<String> string)
-      : string_(String::Flatten(string)) {}
+      : string_(String::Flatten(string)) {
+    DCHECK(!string->IsInternalizedString());
+  }
 
   bool IsMatch(Object* string) override {
-    return String::cast(string)->Equals(*string_);
+    return string_->SlowEquals(String::cast(string));
   }
 
-  uint32_t Hash() override { return string_->Hash(); }
+  uint32_t ComputeHashField() override {
+    // Make sure hash_field() is computed.
+    string_->Hash();
+    return string_->hash_field();
+  }
 
   Handle<Object> AsHandle(Isolate* isolate) override {
     // Internalize the string if possible.
@@ -17152,6 +17155,7 @@ class TwoCharHashTableKey : public StringTableKey {
     hash ^= hash >> 11;
     hash += hash << 15;
     if ((hash & String::kHashBitMask) == 0) hash = StringHasher::kZeroHash;
+    hash = (hash << String::kHashShift) | String::kIsNotArrayIndexMask;
     hash_ = hash;
 #ifdef DEBUG
     // If this assert fails then we failed to reproduce the two-character
@@ -17160,20 +17164,19 @@ class TwoCharHashTableKey : public StringTableKey {
     // algorithm is different in that case.
     uint16_t chars[2] = {c1, c2};
     uint32_t check_hash = StringHasher::HashSequentialString(chars, 2, seed);
-    hash = (hash << String::kHashShift) | String::kIsNotArrayIndexMask;
     DCHECK_EQ(static_cast<int32_t>(hash), static_cast<int32_t>(check_hash));
 #endif
   }
 
   bool IsMatch(Object* o) override {
-    if (!o->IsString()) return false;
     String* other = String::cast(o);
     if (other->length() != 2) return false;
     if (other->Get(0) != c1_) return false;
     return other->Get(1) == c2_;
   }
 
-  uint32_t Hash() override { return hash_; }
+  // TODO(verwaest): Store in hash_field_ of superclass.
+  uint32_t ComputeHashField() override { return hash_; }
 
   Handle<Object> AsHandle(Isolate* isolate) override {
     // The TwoCharHashTableKey is only used for looking in the string
@@ -17187,52 +17190,20 @@ class TwoCharHashTableKey : public StringTableKey {
   uint32_t hash_;
 };
 
-
-MaybeHandle<String> StringTable::InternalizeStringIfExists(
-    Isolate* isolate,
-    Handle<String> string) {
-  if (string->IsInternalizedString()) {
-    return string;
-  }
-  if (string->IsThinString()) {
-    return handle(Handle<ThinString>::cast(string)->actual(), isolate);
-  }
-  return LookupStringIfExists(isolate, string);
-}
-
-
-MaybeHandle<String> StringTable::LookupStringIfExists(
-    Isolate* isolate,
-    Handle<String> string) {
-  Handle<StringTable> string_table = isolate->factory()->string_table();
-  InternalizedStringKey key(string);
-  int entry = string_table->FindEntry(&key);
-  if (entry == kNotFound) {
-    return MaybeHandle<String>();
-  } else {
-    Handle<String> result(String::cast(string_table->KeyAt(entry)), isolate);
-    DCHECK(StringShape(*result).IsInternalized());
-    return result;
-  }
-}
-
-
 MaybeHandle<String> StringTable::LookupTwoCharsStringIfExists(
     Isolate* isolate,
     uint16_t c1,
     uint16_t c2) {
-  Handle<StringTable> string_table = isolate->factory()->string_table();
   TwoCharHashTableKey key(c1, c2, isolate->heap()->HashSeed());
+  Handle<StringTable> string_table = isolate->factory()->string_table();
   int entry = string_table->FindEntry(&key);
-  if (entry == kNotFound) {
-    return MaybeHandle<String>();
-  } else {
-    Handle<String> result(String::cast(string_table->KeyAt(entry)), isolate);
-    DCHECK(StringShape(*result).IsInternalized());
-    return result;
-  }
-}
+  if (entry == kNotFound) return MaybeHandle<String>();
 
+  Handle<String> result(String::cast(string_table->KeyAt(entry)), isolate);
+  DCHECK(StringShape(*result).IsInternalized());
+  DCHECK_EQ(result->Hash(), key.Hash());
+  return result;
+}
 
 void StringTable::EnsureCapacityForDeserialization(Isolate* isolate,
                                                    int expected) {
@@ -17392,8 +17363,8 @@ class StringTableNoAllocateKey : public StringTableKey {
     } else {
       special_flattening_ = false;
       one_byte_content_ = nullptr;
+      string->Hash();
     }
-    hash_ = string->Hash();
   }
 
   ~StringTableNoAllocateKey() {
@@ -17408,7 +17379,7 @@ class StringTableNoAllocateKey : public StringTableKey {
     String* other = String::cast(otherstring);
     DCHECK(other->IsInternalizedString());
     DCHECK(other->IsFlat());
-    if (hash_ != other->Hash()) return false;
+    if (Hash() != other->Hash()) return false;
     int len = length_;
     if (len != other->length()) return false;
 
@@ -17462,7 +17433,7 @@ class StringTableNoAllocateKey : public StringTableKey {
     }
   }
 
-  uint32_t Hash() override { return hash_; }
+  uint32_t ComputeHashField() override { return string_->hash_field(); }
 
   MUST_USE_RESULT Handle<Object> AsHandle(Isolate* isolate) override {
     UNREACHABLE();
@@ -17473,7 +17444,6 @@ class StringTableNoAllocateKey : public StringTableKey {
   int length_;
   bool one_byte_;
   bool special_flattening_;
-  uint32_t hash_ = 0;
   union {
     uint8_t* one_byte_content_;
     uint16_t* two_byte_content_;
@@ -17492,7 +17462,6 @@ Object* StringTable::LookupStringIfExists_NoAllocate(String* string) {
   StringTableNoAllocateKey key(string, heap->HashSeed());
 
   // String could be an array index.
-  DCHECK(string->HasHashCode());
   uint32_t hash = string->hash_field();
 
   // Valid array indices are >= 0, so they cannot be mixed up with any of

src/objects/hash-table.h

@@ -258,9 +258,22 @@ class HashTableKey {
   // Returns whether the other object matches this key.
   virtual bool IsMatch(Object* other) = 0;
   // Returns the hash value for this key.
-  virtual uint32_t Hash() = 0;
   // Required.
   virtual ~HashTableKey() {}
+
+  uint32_t Hash() {
+    if (hash_ == 0) {
+      hash_ = ComputeHash();
+      DCHECK_NE(0, hash_);
+    }
+    return hash_;
+  }
+
+ protected:
+  virtual uint32_t ComputeHash() = 0;
+
+ private:
+  uint32_t hash_ = 0;
 };
 
 class ObjectHashTableShape : public BaseShape<Handle<Object>> {

src/objects/string-inl.h

@@ -195,19 +195,14 @@ template <typename Char>
 class SequentialStringKey : public StringTableKey {
  public:
   explicit SequentialStringKey(Vector<const Char> string, uint32_t seed)
-      : string_(string), hash_field_(0), seed_(seed) {}
+      : string_(string), seed_(seed) {}
 
-  uint32_t Hash() override {
-    hash_field_ = StringHasher::HashSequentialString<Char>(
-        string_.start(), string_.length(), seed_);
-
-    uint32_t result = hash_field_ >> String::kHashShift;
-    DCHECK(result != 0);  // Ensure that the hash value of 0 is never computed.
-    return result;
+  uint32_t ComputeHashField() override {
+    return StringHasher::HashSequentialString<Char>(string_.start(),
+                                                    string_.length(), seed_);
   }
 
   Vector<const Char> string_;
-  uint32_t hash_field_;
   uint32_t seed_;
 };
 
@@ -238,15 +233,12 @@ class SeqOneByteSubStringKey : public StringTableKey {
 #pragma warning(push)
 #pragma warning(disable : 4789)
 #endif
-  uint32_t Hash() override {
+  uint32_t ComputeHashField() override {
     DCHECK(length_ >= 0);
     DCHECK(from_ + length_ <= string_->length());
     const uint8_t* chars = string_->GetChars() + from_;
-    hash_field_ = StringHasher::HashSequentialString(
-        chars, length_, string_->GetHeap()->HashSeed());
-    uint32_t result = hash_field_ >> String::kHashShift;
-    DCHECK(result != 0);  // Ensure that the hash value of 0 is never computed.
-    return result;
+    return StringHasher::HashSequentialString(chars, length_,
+                                              string_->GetHeap()->HashSeed());
   }
 #if defined(V8_CC_MSVC)
 #pragma warning(pop)
@@ -259,7 +251,6 @@ class SeqOneByteSubStringKey : public StringTableKey {
   Handle<SeqOneByteString> string_;
   int from_;
   int length_;
-  uint32_t hash_field_;
 };
 
 class TwoByteStringKey : public SequentialStringKey<uc16> {
@@ -278,28 +269,22 @@ class TwoByteStringKey : public SequentialStringKey<uc16> {
 class Utf8StringKey : public StringTableKey {
  public:
   explicit Utf8StringKey(Vector<const char> string, uint32_t seed)
-      : string_(string), hash_field_(0), seed_(seed) {}
+      : string_(string), seed_(seed) {}
 
   bool IsMatch(Object* string) override {
     return String::cast(string)->IsUtf8EqualTo(string_);
   }
 
-  uint32_t Hash() override {
-    if (hash_field_ != 0) return hash_field_ >> String::kHashShift;
-    hash_field_ = StringHasher::ComputeUtf8Hash(string_, seed_, &chars_);
-    uint32_t result = hash_field_ >> String::kHashShift;
-    DCHECK(result != 0);  // Ensure that the hash value of 0 is never computed.
-    return result;
+  uint32_t ComputeHashField() override {
+    return StringHasher::ComputeUtf8Hash(string_, seed_, &chars_);
   }
 
   Handle<Object> AsHandle(Isolate* isolate) override {
-    if (hash_field_ == 0) Hash();
     return isolate->factory()->NewInternalizedStringFromUtf8(string_, chars_,
-                                                             hash_field_);
+                                                             HashField());
   }
 
   Vector<const char> string_;
-  uint32_t hash_field_;
   int chars_;  // Caches the number of characters when computing the hash code.
   uint32_t seed_;
 };

src/objects/string-table.h

@@ -16,6 +16,15 @@ namespace internal {
 class StringTableKey : public HashTableKey {
  public:
   virtual Handle<Object> AsHandle(Isolate* isolate) = 0;
+  inline uint32_t ComputeHash() final;
+  uint32_t HashField() {
+    if (hash_field_ == 0) hash_field_ = ComputeHashField();
+    return hash_field_;
+  }
+  virtual uint32_t ComputeHashField() = 0;
+
+ private:
+  uint32_t hash_field_ = 0;
 };
 
 class StringTableShape : public BaseShape<StringTableKey*> {
@@ -49,15 +58,8 @@ class StringTable : public HashTable<StringTable, StringTableShape> {
   static Handle<String> LookupKey(Isolate* isolate, StringTableKey* key);
   static String* LookupKeyIfExists(Isolate* isolate, StringTableKey* key);
 
-  // Tries to internalize given string and returns string handle on success
-  // or an empty handle otherwise.
-  MUST_USE_RESULT static MaybeHandle<String> InternalizeStringIfExists(
-      Isolate* isolate, Handle<String> string);
-
   // Looks up a string that is equal to the given string and returns
   // string handle if it is found, or an empty handle otherwise.
-  MUST_USE_RESULT static MaybeHandle<String> LookupStringIfExists(
-      Isolate* isolate, Handle<String> str);
   MUST_USE_RESULT static MaybeHandle<String> LookupTwoCharsStringIfExists(
       Isolate* isolate, uint16_t c1, uint16_t c2);
   static Object* LookupStringIfExists_NoAllocate(String* string);

src/objects/string.h

@@ -435,6 +435,7 @@ class String : public Name {
  private:
   friend class Name;
   friend class StringTableInsertionKey;
+  friend class InternalizedStringKey;
 
   static Handle<String> SlowFlatten(Handle<ConsString> cons,
                                     PretenureFlag tenure);

src/snapshot/deserializer.cc

@@ -283,20 +283,23 @@ void Deserializer::PrintDisassembledCodeObjects() {
 // Used to insert a deserialized internalized string into the string table.
 class StringTableInsertionKey : public StringTableKey {
  public:
-  explicit StringTableInsertionKey(String* string)
-      : string_(string), hash_(string->Hash()) {
+  explicit StringTableInsertionKey(String* string) : string_(string) {
     DCHECK(string->IsInternalizedString());
   }
 
   bool IsMatch(Object* string) override {
     // We know that all entries in a hash table had their hash keys created.
     // Use that knowledge to have fast failure.
-    if (hash_ != String::cast(string)->Hash()) return false;
+    if (Hash() != String::cast(string)->Hash()) return false;
     // We want to compare the content of two internalized strings here.
     return string_->SlowEquals(String::cast(string));
   }
 
-  uint32_t Hash() override { return hash_; }
+  uint32_t ComputeHashField() override {
+    // Make sure hash_field() is computed.
+    string_->Hash();
+    return string_->hash_field();
+  }
 
   MUST_USE_RESULT Handle<Object> AsHandle(Isolate* isolate) override {
     return handle(string_, isolate);
@@ -304,7 +307,6 @@ class StringTableInsertionKey : public StringTableKey {
 
  private:
   String* string_;
-  uint32_t hash_;
   DisallowHeapAllocation no_gc;
 };