Serializer: commit new internalized strings after deserialization.

Reserving space for deserialization can cause GC, which
can evict entries from the string table. Having more deleted
entries now, StringTable::EnsureCapacity could cause a GC
later during deserialization even when we actually still
have enough capacity.

Instead, we now keep new internalized strings in a separate list
and commit them to the string table at the end.

R=ulan@chromium.org
BUG=chromium:502085
LOG=N

Review URL: https://codereview.chromium.org/1204863006

Cr-Commit-Position: refs/heads/master@{#29308}

src/objects.cc

@@ -14494,6 +14494,14 @@ Handle<String> StringTable::LookupKey(Isolate* isolate, HashTableKey* key) {
 }
 
 
+String* StringTable::LookupKeyIfExists(Isolate* isolate, HashTableKey* key) {
+  Handle<StringTable> table = isolate->factory()->string_table();
+  int entry = table->FindEntry(key);
+  if (entry != kNotFound) return String::cast(table->KeyAt(entry));
+  return NULL;
+}
+
+
 Handle<Object> CompilationCacheTable::Lookup(Handle<String> src,
                                              Handle<Context> context,
                                              LanguageMode language_mode) {

src/objects.h

@@ -3179,6 +3179,7 @@ class StringTable: public HashTable<StringTable,
   // added. The return value is the string found.
   static Handle<String> LookupString(Isolate* isolate, Handle<String> key);
   static Handle<String> LookupKey(Isolate* isolate, HashTableKey* key);
+  static String* LookupKeyIfExists(Isolate* isolate, HashTableKey* key);
 
   // Tries to internalize given string and returns string handle on success
   // or an empty handle otherwise.

src/snapshot/serialize.cc

@@ -642,11 +642,17 @@ MaybeHandle<SharedFunctionInfo> Deserializer::DeserializeCode(
     return Handle<SharedFunctionInfo>();
   } else {
     deserializing_user_code_ = true;
-    DisallowHeapAllocation no_gc;
-    Object* root;
-    VisitPointer(&root);
-    DeserializeDeferredObjects();
-    return Handle<SharedFunctionInfo>(SharedFunctionInfo::cast(root));
+    HandleScope scope(isolate);
+    Handle<SharedFunctionInfo> result;
+    {
+      DisallowHeapAllocation no_gc;
+      Object* root;
+      VisitPointer(&root);
+      DeserializeDeferredObjects();
+      result = Handle<SharedFunctionInfo>(SharedFunctionInfo::cast(root));
+    }
+    CommitNewInternalizedStrings(isolate);
+    return scope.CloseAndEscape(result);
   }
 }
 
@@ -711,8 +717,10 @@ class StringTableInsertionKey : public HashTableKey {
     return handle(string_, isolate);
   }
 
+ private:
   String* string_;
   uint32_t hash_;
+  DisallowHeapAllocation no_gc;
 };
 
 
@@ -725,12 +733,15 @@ HeapObject* Deserializer::PostProcessNewObject(HeapObject* obj, int space) {
       if (string->IsInternalizedString()) {
         // Canonicalize the internalized string. If it already exists in the
         // string table, set it to forward to the existing one.
-        DisallowHeapAllocation no_gc;
-        HandleScope scope(isolate_);
         StringTableInsertionKey key(string);
-        String* canonical = *StringTable::LookupKey(isolate_, &key);
-        string->SetForwardedInternalizedString(canonical);
-        return canonical;
+        String* canonical = StringTable::LookupKeyIfExists(isolate_, &key);
+        if (canonical == NULL) {
+          new_internalized_strings_.Add(handle(string));
+          return string;
+        } else {
+          string->SetForwardedInternalizedString(canonical);
+          return canonical;
+        }
       }
     } else if (obj->IsScript()) {
       // Assign a new script id to avoid collision.
@@ -767,6 +778,17 @@ HeapObject* Deserializer::PostProcessNewObject(HeapObject* obj, int space) {
 }
 
 
+void Deserializer::CommitNewInternalizedStrings(Isolate* isolate) {
+  StringTable::EnsureCapacityForDeserialization(
+      isolate, new_internalized_strings_.length());
+  for (Handle<String> string : new_internalized_strings_) {
+    StringTableInsertionKey key(*string);
+    DCHECK_NULL(StringTable::LookupKeyIfExists(isolate, &key));
+    StringTable::LookupKey(isolate, &key);
+  }
+}
+
+
 HeapObject* Deserializer::GetBackReferencedObject(int space) {
   HeapObject* obj;
   BackReference back_reference(source_.GetInt());
@@ -2352,8 +2374,6 @@ void CodeSerializer::SerializeObject(HeapObject* obj, HowToCode how_to_code,
 void CodeSerializer::SerializeGeneric(HeapObject* heap_object,
                                       HowToCode how_to_code,
                                       WhereToPoint where_to_point) {
-  if (heap_object->IsInternalizedString()) num_internalized_strings_++;
-
   // Object has not yet been serialized.  Serialize it here.
   ObjectSerializer serializer(this, heap_object, sink_, how_to_code,
                               where_to_point);
@@ -2475,10 +2495,6 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
         CodeStub::GetCode(isolate, code_stub_keys[i]).ToHandleChecked();
   }
 
-  // Eagerly expand string table to avoid allocations during deserialization.
-  StringTable::EnsureCapacityForDeserialization(isolate,
-                                                scd->NumInternalizedStrings());
-
   Deserializer deserializer(scd.get());
   deserializer.SetAttachedObjects(attached_objects);
 
@@ -2637,7 +2653,6 @@ SerializedCodeData::SerializedCodeData(const List<byte>& payload,
   SetHeaderValue(kCpuFeaturesOffset,
                  static_cast<uint32_t>(CpuFeatures::SupportedFeatures()));
   SetHeaderValue(kFlagHashOffset, FlagList::Hash());
-  SetHeaderValue(kNumInternalizedStringsOffset, cs.num_internalized_strings());
   SetHeaderValue(kNumReservationsOffset, reservations.length());
   SetHeaderValue(kNumCodeStubKeysOffset, num_stub_keys);
   SetHeaderValue(kPayloadLengthOffset, payload.length());
@@ -2715,10 +2730,6 @@ Vector<const byte> SerializedCodeData::Payload() const {
 }
 
 
-int SerializedCodeData::NumInternalizedStrings() const {
-  return GetHeaderValue(kNumInternalizedStringsOffset);
-}
-
 Vector<const uint32_t> SerializedCodeData::CodeStubKeys() const {
   int reservations_size = GetHeaderValue(kNumReservationsOffset) * kInt32Size;
   const byte* start = data_ + kHeaderSize + reservations_size;

src/snapshot/serialize.h

@@ -568,6 +568,8 @@ class Deserializer: public SerializerDeserializer {
 
   void DeserializeDeferredObjects();
 
+  void CommitNewInternalizedStrings(Isolate* isolate);
+
   // Fills in some heap data in an area from start to end (non-inclusive).  The
   // space id is used for the write barrier.  The object_address is the address
   // of the object we are writing into, or NULL if we are not writing into an
@@ -606,6 +608,7 @@ class Deserializer: public SerializerDeserializer {
 
   List<HeapObject*> deserialized_large_objects_;
   List<Code*> new_code_objects_;
+  List<Handle<String> > new_internalized_strings_;
 
   bool deserializing_user_code_;
 
@@ -909,15 +912,11 @@ class CodeSerializer : public Serializer {
   }
 
   const List<uint32_t>* stub_keys() const { return &stub_keys_; }
-  int num_internalized_strings() const { return num_internalized_strings_; }
 
  private:
   CodeSerializer(Isolate* isolate, SnapshotByteSink* sink, String* source,
                  Code* main_code)
-      : Serializer(isolate, sink),
-        source_(source),
-        main_code_(main_code),
-        num_internalized_strings_(0) {
+      : Serializer(isolate, sink), source_(source), main_code_(main_code) {
     back_reference_map_.AddSourceString(source);
   }
 
@@ -939,7 +938,6 @@ class CodeSerializer : public Serializer {
   DisallowHeapAllocation no_gc_;
   String* source_;
   Code* main_code_;
-  int num_internalized_strings_;
   List<uint32_t> stub_keys_;
   DISALLOW_COPY_AND_ASSIGN(CodeSerializer);
 };
@@ -998,7 +996,6 @@ class SerializedCodeData : public SerializedData {
   Vector<const Reservation> Reservations() const;
   Vector<const byte> Payload() const;
 
-  int NumInternalizedStrings() const;
   Vector<const uint32_t> CodeStubKeys() const;
 
  private:
@@ -1019,17 +1016,16 @@ class SerializedCodeData : public SerializedData {
   uint32_t SourceHash(String* source) const { return source->length(); }
 
   // The data header consists of uint32_t-sized entries:
-  // [ 0] magic number and external reference count
-  // [ 1] version hash
-  // [ 2] source hash
-  // [ 3] cpu features
-  // [ 4] flag hash
-  // [ 5] number of internalized strings
-  // [ 6] number of code stub keys
-  // [ 7] number of reservation size entries
-  // [ 8] payload length
-  // [ 9] payload checksum part 1
-  // [10] payload checksum part 2
+  // [0] magic number and external reference count
+  // [1] version hash
+  // [2] source hash
+  // [3] cpu features
+  // [4] flag hash
+  // [5] number of code stub keys
+  // [6] number of reservation size entries
+  // [7] payload length
+  // [8] payload checksum part 1
+  // [9] payload checksum part 2
   // ...  reservations
   // ...  code stub keys
   // ...  serialized payload
@@ -1037,9 +1033,7 @@ class SerializedCodeData : public SerializedData {
   static const int kSourceHashOffset = kVersionHashOffset + kInt32Size;
   static const int kCpuFeaturesOffset = kSourceHashOffset + kInt32Size;
   static const int kFlagHashOffset = kCpuFeaturesOffset + kInt32Size;
-  static const int kNumInternalizedStringsOffset = kFlagHashOffset + kInt32Size;
-  static const int kNumReservationsOffset =
-      kNumInternalizedStringsOffset + kInt32Size;
+  static const int kNumReservationsOffset = kFlagHashOffset + kInt32Size;
   static const int kNumCodeStubKeysOffset = kNumReservationsOffset + kInt32Size;
   static const int kPayloadLengthOffset = kNumCodeStubKeysOffset + kInt32Size;
   static const int kChecksum1Offset = kPayloadLengthOffset + kInt32Size;