[runtime] Move ForwardStringIfExists to snapshot deserializer

That's the only place where it's used, with a known StringTableKey type.
This also makes MakeThin available on String.

Change-Id: If1aca892045dadae91c902e783822280ae01f485
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1598764
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61299}

src/objects.cc

@@ -6764,66 +6764,6 @@ void StringTable::EnsureCapacityForDeserialization(Isolate* isolate,
   isolate->heap()->SetRootStringTable(*table);
 }
 
-namespace {
-
-template <class StringClass>
-void MigrateExternalStringResource(Isolate* isolate, String from, String to) {
-  StringClass cast_from = StringClass::cast(from);
-  StringClass cast_to = StringClass::cast(to);
-  const typename StringClass::Resource* to_resource = cast_to->resource();
-  if (to_resource == nullptr) {
-    // |to| is a just-created internalized copy of |from|. Migrate the resource.
-    cast_to->SetResource(isolate, cast_from->resource());
-    // Zap |from|'s resource pointer to reflect the fact that |from| has
-    // relinquished ownership of its resource.
-    isolate->heap()->UpdateExternalString(
-        from, ExternalString::cast(from)->ExternalPayloadSize(), 0);
-    cast_from->SetResource(isolate, nullptr);
-  } else if (to_resource != cast_from->resource()) {
-    // |to| already existed and has its own resource. Finalize |from|.
-    isolate->heap()->FinalizeExternalString(from);
-  }
-}
-
-void MakeStringThin(String string, String internalized, Isolate* isolate) {
-  DCHECK_NE(string, internalized);
-  DCHECK(internalized->IsInternalizedString());
-
-  if (string->IsExternalString()) {
-    if (internalized->IsExternalOneByteString()) {
-      MigrateExternalStringResource<ExternalOneByteString>(isolate, string,
-                                                           internalized);
-    } else if (internalized->IsExternalTwoByteString()) {
-      MigrateExternalStringResource<ExternalTwoByteString>(isolate, string,
-                                                           internalized);
-    } else {
-      // If the external string is duped into an existing non-external
-      // internalized string, free its resource (it's about to be rewritten
-      // into a ThinString below).
-      isolate->heap()->FinalizeExternalString(string);
-    }
-  }
-
-  DisallowHeapAllocation no_gc;
-  int old_size = string->Size();
-  isolate->heap()->NotifyObjectLayoutChange(string, old_size, no_gc);
-  bool one_byte = internalized->IsOneByteRepresentation();
-  Handle<Map> map = one_byte ? isolate->factory()->thin_one_byte_string_map()
-                             : isolate->factory()->thin_string_map();
-  DCHECK_GE(old_size, ThinString::kSize);
-  string->synchronized_set_map(*map);
-  ThinString thin = ThinString::cast(string);
-  thin->set_actual(internalized);
-  Address thin_end = thin->address() + ThinString::kSize;
-  int size_delta = old_size - ThinString::kSize;
-  if (size_delta != 0) {
-    Heap* heap = isolate->heap();
-    heap->CreateFillerObjectAt(thin_end, size_delta, ClearRecordedSlots::kNo);
-  }
-}
-
-}  // namespace
-
 // static
 Handle<String> StringTable::LookupString(Isolate* isolate,
                                          Handle<String> string) {
@@ -6835,7 +6775,7 @@ Handle<String> StringTable::LookupString(Isolate* isolate,
 
   if (FLAG_thin_strings) {
     if (!string->IsInternalizedString()) {
-      MakeStringThin(*string, *result, isolate);
+      string->MakeThin(isolate, *result);
     }
   } else {  // !FLAG_thin_strings
     if (string->IsConsString()) {
@@ -6969,7 +6909,7 @@ Address LookupString(Isolate* isolate, String string) {
 
   String internalized = String::cast(table->KeyAt(entry));
   if (FLAG_thin_strings) {
-    MakeStringThin(string, internalized, isolate);
+    string->MakeThin(isolate, internalized);
   }
   return internalized.ptr();
 }
@@ -6995,17 +6935,6 @@ Address StringTable::LookupStringIfExists_NoAllocate(Isolate* isolate,
   return i::LookupString<uint16_t>(isolate, string);
 }
 
-String StringTable::ForwardStringIfExists(Isolate* isolate, StringTableKey* key,
-                                          String string) {
-  Handle<StringTable> table = isolate->factory()->string_table();
-  int entry = table->FindEntry(isolate, key);
-  if (entry == kNotFound) return String();
-
-  String canonical = String::cast(table->KeyAt(entry));
-  if (canonical != string) MakeStringThin(string, canonical, isolate);
-  return canonical;
-}
-
 Handle<StringSet> StringSet::New(Isolate* isolate) {
   return HashTable::New(isolate, 0);
 }

src/objects/string-table.h

@@ -68,8 +68,6 @@ class StringTable : public HashTable<StringTable, StringTableShape> {
   template <typename StringTableKey>
   static Handle<String> LookupKey(Isolate* isolate, StringTableKey* key);
   static Handle<String> AddKeyNoResize(Isolate* isolate, StringTableKey* key);
-  static String ForwardStringIfExists(Isolate* isolate, StringTableKey* key,
-                                      String string);
 
   // Shink the StringTable if it's very empty (kMaxEmptyFactor) to avoid the
   // performance overhead of re-allocating the StringTable over and over again.

src/objects/string.cc

@@ -66,6 +66,66 @@ Handle<String> String::SlowFlatten(Isolate* isolate, Handle<ConsString> cons,
   return result;
 }
 
+namespace {
+
+template <class StringClass>
+void MigrateExternalStringResource(Isolate* isolate, String from, String to) {
+  StringClass cast_from = StringClass::cast(from);
+  StringClass cast_to = StringClass::cast(to);
+  const typename StringClass::Resource* to_resource = cast_to->resource();
+  if (to_resource == nullptr) {
+    // |to| is a just-created internalized copy of |from|. Migrate the resource.
+    cast_to->SetResource(isolate, cast_from->resource());
+    // Zap |from|'s resource pointer to reflect the fact that |from| has
+    // relinquished ownership of its resource.
+    isolate->heap()->UpdateExternalString(
+        from, ExternalString::cast(from)->ExternalPayloadSize(), 0);
+    cast_from->SetResource(isolate, nullptr);
+  } else if (to_resource != cast_from->resource()) {
+    // |to| already existed and has its own resource. Finalize |from|.
+    isolate->heap()->FinalizeExternalString(from);
+  }
+}
+
+}  // namespace
+
+void String::MakeThin(Isolate* isolate, String internalized) {
+  DisallowHeapAllocation no_gc;
+  DCHECK_NE(*this, internalized);
+  DCHECK(internalized->IsInternalizedString());
+
+  if (this->IsExternalString()) {
+    if (internalized->IsExternalOneByteString()) {
+      MigrateExternalStringResource<ExternalOneByteString>(isolate, *this,
+                                                           internalized);
+    } else if (internalized->IsExternalTwoByteString()) {
+      MigrateExternalStringResource<ExternalTwoByteString>(isolate, *this,
+                                                           internalized);
+    } else {
+      // If the external string is duped into an existing non-external
+      // internalized string, free its resource (it's about to be rewritten
+      // into a ThinString below).
+      isolate->heap()->FinalizeExternalString(*this);
+    }
+  }
+
+  int old_size = this->Size();
+  isolate->heap()->NotifyObjectLayoutChange(*this, old_size, no_gc);
+  bool one_byte = internalized->IsOneByteRepresentation();
+  Handle<Map> map = one_byte ? isolate->factory()->thin_one_byte_string_map()
+                             : isolate->factory()->thin_string_map();
+  DCHECK_GE(old_size, ThinString::kSize);
+  this->synchronized_set_map(*map);
+  ThinString thin = ThinString::cast(*this);
+  thin->set_actual(internalized);
+  Address thin_end = thin->address() + ThinString::kSize;
+  int size_delta = old_size - ThinString::kSize;
+  if (size_delta != 0) {
+    Heap* heap = isolate->heap();
+    heap->CreateFillerObjectAt(thin_end, size_delta, ClearRecordedSlots::kNo);
+  }
+}
+
 bool String::MakeExternal(v8::String::ExternalStringResource* resource) {
   DisallowHeapAllocation no_allocation;
   // Externalizing twice leaks the external resource, so it's

src/objects/string.h

@@ -142,6 +142,8 @@ class String : public Name {
     friend class IterableSubString;
   };
 
+  void MakeThin(Isolate* isolate, String canonical);
+
   template <typename Char>
   V8_INLINE Vector<const Char> GetCharVector(
       const DisallowHeapAllocation& no_gc);

src/snapshot/deserializer.cc

@@ -186,6 +186,21 @@ uint32_t StringTableInsertionKey::ComputeHashField(String string) {
   return string->hash_field();
 }
 
+namespace {
+
+String ForwardStringIfExists(Isolate* isolate, StringTableInsertionKey* key) {
+  StringTable table = isolate->heap()->string_table();
+  int entry = table.FindEntry(isolate, key);
+  if (entry == kNotFound) return String();
+
+  String canonical = String::cast(table->KeyAt(entry));
+  DCHECK_NE(canonical, key->string());
+  key->string().MakeThin(isolate, canonical);
+  return canonical;
+}
+
+}  // namespace
+
 HeapObject Deserializer::PostProcessNewObject(HeapObject obj, int space) {
   if ((FLAG_rehash_snapshot && can_rehash_) || deserializing_user_code()) {
     if (obj->IsString()) {
@@ -209,8 +224,7 @@ HeapObject Deserializer::PostProcessNewObject(HeapObject obj, int space) {
         // Canonicalize the internalized string. If it already exists in the
         // string table, set it to forward to the existing one.
         StringTableInsertionKey key(string);
-        String canonical =
-            StringTable::ForwardStringIfExists(isolate_, &key, string);
+        String canonical = ForwardStringIfExists(isolate_, &key);
 
         if (!canonical.is_null()) return canonical;
 

src/snapshot/deserializer.h

@@ -194,6 +194,8 @@ class StringTableInsertionKey final : public StringTableKey {
 
   V8_WARN_UNUSED_RESULT Handle<String> AsHandle(Isolate* isolate) override;
 
+  String string() const { return string_; }
+
  private:
   uint32_t ComputeHashField(String string);
 

src/snapshot/object-deserializer.cc

@@ -73,8 +73,6 @@ void ObjectDeserializer::CommitPostProcessedObjects() {
   for (Handle<String> string : new_internalized_strings()) {
     DisallowHeapAllocation no_gc;
     StringTableInsertionKey key(*string);
-    DCHECK(
-        StringTable::ForwardStringIfExists(isolate(), &key, *string).is_null());
     StringTable::AddKeyNoResize(isolate(), &key);
   }