[snapshot] Keep fewer objects in the shared heap object cache

Tip of tree puts both internalized and in-place-internalizable strings
into the shared heap object cache. But only internalized strings need
to go in there, since we can't have duplicates of those. It's fine to
allocate in-place-internalizable strings in the shared heap each time
a new Isolate is initialized, it'll be deduplicated if it's
internalized eventually.

Bug: chromium:1258918, v8:12007
Change-Id: I0e46b73a5ac3be83d0eaa31915a3a24f47a8c2bd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3219690
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77388}

src/heap/heap.cc

@@ -4293,6 +4293,17 @@ bool Heap::SharedHeapContains(HeapObject value) const {
   return false;
 }
 
+bool Heap::ShouldBeInSharedOldSpace(HeapObject value) {
+  if (isolate()->OwnsStringTable()) return false;
+  if (ReadOnlyHeap::Contains(value)) return false;
+  if (Heap::InYoungGeneration(value)) return false;
+  if (value.IsString()) {
+    return value.IsInternalizedString() ||
+           String::IsInPlaceInternalizable(String::cast(value));
+  }
+  return false;
+}
+
 bool Heap::InSpace(HeapObject value, AllocationSpace space) const {
   if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL)
     return third_party_heap::Heap::InSpace(value.address(), space);

src/heap/heap.h

@@ -1279,6 +1279,9 @@ class Heap {
   // heaps is required.
   V8_EXPORT_PRIVATE bool SharedHeapContains(HeapObject value) const;
 
+  // Returns whether the object should be in the shared old space.
+  V8_EXPORT_PRIVATE bool ShouldBeInSharedOldSpace(HeapObject value);
+
   // Checks whether an address/object in a space.
   // Currently used by tests, serialization and heap verification only.
   V8_EXPORT_PRIVATE bool InSpace(HeapObject value, AllocationSpace space) const;

src/heap/mark-compact.cc

@@ -268,10 +268,7 @@ class FullMarkingVerifier : public MarkingVerifier {
         BasicMemoryChunk::FromHeapObject(heap_object)->InSharedHeap())
       return;
 
-    if (!heap_->isolate()->OwnsStringTable() &&
-        !Heap::InYoungGeneration(heap_object) &&
-        SharedHeapSerializer::ShouldBeInSharedOldSpace(heap_->isolate(),
-                                                       heap_object)) {
+    if (heap_->ShouldBeInSharedOldSpace(heap_object)) {
       CHECK(heap_->SharedHeapContains(heap_object));
     }
 

src/snapshot/deserializer.cc

@@ -979,8 +979,8 @@ int Deserializer<IsolateT>::ReadSingleBytecodeData(byte data,
       // shared_heap_object_cache entry as a Handle backing?
       HeapObject heap_object = HeapObject::cast(
           main_thread_isolate()->shared_heap_object_cache()->at(cache_index));
-      DCHECK(SharedHeapSerializer::ShouldBeInSharedOldSpace(
-          main_thread_isolate(), heap_object));
+      DCHECK(
+          SharedHeapSerializer::ShouldBeInSharedHeapObjectCache(heap_object));
       return slot_accessor.Write(heap_object, GetAndResetNextReferenceType());
     }
 

src/snapshot/shared-heap-serializer.cc

@@ -13,8 +13,7 @@ namespace v8 {
 namespace internal {
 
 // static
-bool SharedHeapSerializer::ShouldBeInSharedOldSpace(Isolate* isolate,
-                                                    HeapObject obj) {
+bool SharedHeapSerializer::CanBeInSharedOldSpace(HeapObject obj) {
   if (ReadOnlyHeap::Contains(obj)) return false;
   if (obj.IsString()) {
     return obj.IsInternalizedString() ||
@@ -23,6 +22,18 @@ bool SharedHeapSerializer::ShouldBeInSharedOldSpace(Isolate* isolate,
   return false;
 }
 
+// static
+bool SharedHeapSerializer::ShouldBeInSharedHeapObjectCache(HeapObject obj) {
+  // To keep the shared heap object cache lean, only include objects that should
+  // not be duplicated. Currently, that is only internalized strings. In-place
+  // internalizable strings will still be allocated in the shared heap by the
+  // deserializer, but do not need to be kept alive forever in the cache.
+  if (CanBeInSharedOldSpace(obj)) {
+    if (obj.IsInternalizedString()) return true;
+  }
+  return false;
+}
+
 SharedHeapSerializer::SharedHeapSerializer(
     Isolate* isolate, Snapshot::SerializerFlags flags,
     ReadOnlySerializer* read_only_serializer)
@@ -64,7 +75,7 @@ void SharedHeapSerializer::FinalizeSerialization() {
       &serialized_objects_);
   for (auto it = it_scope.begin(); it != it_scope.end(); ++it) {
     HeapObject obj = HeapObject::cast(it.key());
-    CHECK(ShouldBeInSharedOldSpace(isolate(), obj));
+    CHECK(CanBeInSharedOldSpace(obj));
     CHECK(!ReadOnlyHeap::Contains(obj));
   }
 #endif
@@ -77,7 +88,7 @@ bool SharedHeapSerializer::SerializeUsingReadOnlyObjectCache(
 
 bool SharedHeapSerializer::SerializeUsingSharedHeapObjectCache(
     SnapshotByteSink* sink, Handle<HeapObject> obj) {
-  if (!ShouldBeInSharedOldSpace(isolate(), *obj)) return false;
+  if (!ShouldBeInSharedHeapObjectCache(*obj)) return false;
   int cache_index = SerializeInObjectCache(obj);
   sink->Put(kSharedHeapObjectCache, "SharedHeapObjectCache");
   sink->PutInt(cache_index, "shared_heap_object_cache_index");
@@ -138,8 +149,7 @@ void SharedHeapSerializer::SerializeStringTable(StringTable* string_table) {
 void SharedHeapSerializer::SerializeObjectImpl(Handle<HeapObject> obj) {
   // Objects in the shared heap cannot depend on per-Isolate roots but can
   // depend on RO roots since sharing objects requires sharing the RO space.
-  DCHECK(ShouldBeInSharedOldSpace(isolate(), *obj) ||
-         ReadOnlyHeap::Contains(*obj));
+  DCHECK(CanBeInSharedOldSpace(*obj) || ReadOnlyHeap::Contains(*obj));
 
   if (SerializeHotObject(obj)) return;
   if (IsRootAndHasBeenSerialized(*obj) && SerializeRoot(obj)) return;

src/snapshot/shared-heap-serializer.h

@@ -43,7 +43,9 @@ class V8_EXPORT_PRIVATE SharedHeapSerializer : public RootsSerializer {
   bool SerializeUsingSharedHeapObjectCache(SnapshotByteSink* sink,
                                            Handle<HeapObject> obj);
 
-  static bool ShouldBeInSharedOldSpace(Isolate* isolate, HeapObject obj);
+  static bool CanBeInSharedOldSpace(HeapObject obj);
+
+  static bool ShouldBeInSharedHeapObjectCache(HeapObject obj);
 
  private:
   void SerializeStringTable(StringTable* string_table);

test/cctest/test-serialize.cc

@@ -4347,11 +4347,12 @@ UNINITIALIZED_TEST(NoStackFrameCacheSerialization) {
 
 namespace {
 void CheckObjectsAreInSharedHeap(Isolate* isolate) {
-  HeapObjectIterator iterator(isolate->heap());
+  Heap* heap = isolate->heap();
+  HeapObjectIterator iterator(heap);
   DisallowGarbageCollection no_gc;
   for (HeapObject obj = iterator.Next(); !obj.is_null();
        obj = iterator.Next()) {
-    if (SharedHeapSerializer::ShouldBeInSharedOldSpace(isolate, obj)) {
+    if (heap->ShouldBeInSharedOldSpace(obj)) {
       CHECK(obj.InSharedHeap());
     }
   }

tools/v8heapconst.py

@@ -454,62 +454,62 @@ KNOWN_OBJECTS = {
   ("read_only_space", 0x034b1): "EmptyFunctionScopeInfo",
   ("read_only_space", 0x034d5): "NativeScopeInfo",
   ("read_only_space", 0x034ed): "HashSeed",
-  ("old_space", 0x04aa1): "ArgumentsIteratorAccessor",
-  ("old_space", 0x04ae5): "ArrayLengthAccessor",
-  ("old_space", 0x04b29): "BoundFunctionLengthAccessor",
-  ("old_space", 0x04b6d): "BoundFunctionNameAccessor",
-  ("old_space", 0x04bb1): "ErrorStackAccessor",
-  ("old_space", 0x04bf5): "FunctionArgumentsAccessor",
-  ("old_space", 0x04c39): "FunctionCallerAccessor",
-  ("old_space", 0x04c7d): "FunctionNameAccessor",
-  ("old_space", 0x04cc1): "FunctionLengthAccessor",
-  ("old_space", 0x04d05): "FunctionPrototypeAccessor",
-  ("old_space", 0x04d49): "StringLengthAccessor",
-  ("old_space", 0x04d8d): "InvalidPrototypeValidityCell",
-  ("old_space", 0x04d95): "EmptyScript",
-  ("old_space", 0x04dd5): "ManyClosuresCell",
-  ("old_space", 0x04de1): "ArrayConstructorProtector",
-  ("old_space", 0x04df5): "NoElementsProtector",
-  ("old_space", 0x04e09): "MegaDOMProtector",
-  ("old_space", 0x04e1d): "IsConcatSpreadableProtector",
-  ("old_space", 0x04e31): "ArraySpeciesProtector",
-  ("old_space", 0x04e45): "TypedArraySpeciesProtector",
-  ("old_space", 0x04e59): "PromiseSpeciesProtector",
-  ("old_space", 0x04e6d): "RegExpSpeciesProtector",
-  ("old_space", 0x04e81): "StringLengthProtector",
-  ("old_space", 0x04e95): "ArrayIteratorProtector",
-  ("old_space", 0x04ea9): "ArrayBufferDetachingProtector",
-  ("old_space", 0x04ebd): "PromiseHookProtector",
-  ("old_space", 0x04ed1): "PromiseResolveProtector",
-  ("old_space", 0x04ee5): "MapIteratorProtector",
-  ("old_space", 0x04ef9): "PromiseThenProtector",
-  ("old_space", 0x04f0d): "SetIteratorProtector",
-  ("old_space", 0x04f21): "StringIteratorProtector",
-  ("old_space", 0x04f35): "SingleCharacterStringCache",
-  ("old_space", 0x0533d): "StringSplitCache",
-  ("old_space", 0x05745): "RegExpMultipleCache",
-  ("old_space", 0x05b4d): "BuiltinsConstantsTable",
-  ("old_space", 0x05f75): "AsyncFunctionAwaitRejectSharedFun",
-  ("old_space", 0x05f99): "AsyncFunctionAwaitResolveSharedFun",
-  ("old_space", 0x05fbd): "AsyncGeneratorAwaitRejectSharedFun",
-  ("old_space", 0x05fe1): "AsyncGeneratorAwaitResolveSharedFun",
-  ("old_space", 0x06005): "AsyncGeneratorYieldResolveSharedFun",
-  ("old_space", 0x06029): "AsyncGeneratorReturnResolveSharedFun",
-  ("old_space", 0x0604d): "AsyncGeneratorReturnClosedRejectSharedFun",
-  ("old_space", 0x06071): "AsyncGeneratorReturnClosedResolveSharedFun",
-  ("old_space", 0x06095): "AsyncIteratorValueUnwrapSharedFun",
-  ("old_space", 0x060b9): "PromiseAllResolveElementSharedFun",
-  ("old_space", 0x060dd): "PromiseAllSettledResolveElementSharedFun",
-  ("old_space", 0x06101): "PromiseAllSettledRejectElementSharedFun",
-  ("old_space", 0x06125): "PromiseAnyRejectElementSharedFun",
-  ("old_space", 0x06149): "PromiseCapabilityDefaultRejectSharedFun",
-  ("old_space", 0x0616d): "PromiseCapabilityDefaultResolveSharedFun",
-  ("old_space", 0x06191): "PromiseCatchFinallySharedFun",
-  ("old_space", 0x061b5): "PromiseGetCapabilitiesExecutorSharedFun",
-  ("old_space", 0x061d9): "PromiseThenFinallySharedFun",
-  ("old_space", 0x061fd): "PromiseThrowerFinallySharedFun",
-  ("old_space", 0x06221): "PromiseValueThunkFinallySharedFun",
-  ("old_space", 0x06245): "ProxyRevokeSharedFun",
+  ("old_space", 0x04211): "ArgumentsIteratorAccessor",
+  ("old_space", 0x04255): "ArrayLengthAccessor",
+  ("old_space", 0x04299): "BoundFunctionLengthAccessor",
+  ("old_space", 0x042dd): "BoundFunctionNameAccessor",
+  ("old_space", 0x04321): "ErrorStackAccessor",
+  ("old_space", 0x04365): "FunctionArgumentsAccessor",
+  ("old_space", 0x043a9): "FunctionCallerAccessor",
+  ("old_space", 0x043ed): "FunctionNameAccessor",
+  ("old_space", 0x04431): "FunctionLengthAccessor",
+  ("old_space", 0x04475): "FunctionPrototypeAccessor",
+  ("old_space", 0x044b9): "StringLengthAccessor",
+  ("old_space", 0x044fd): "InvalidPrototypeValidityCell",
+  ("old_space", 0x04505): "EmptyScript",
+  ("old_space", 0x04545): "ManyClosuresCell",
+  ("old_space", 0x04551): "ArrayConstructorProtector",
+  ("old_space", 0x04565): "NoElementsProtector",
+  ("old_space", 0x04579): "MegaDOMProtector",
+  ("old_space", 0x0458d): "IsConcatSpreadableProtector",
+  ("old_space", 0x045a1): "ArraySpeciesProtector",
+  ("old_space", 0x045b5): "TypedArraySpeciesProtector",
+  ("old_space", 0x045c9): "PromiseSpeciesProtector",
+  ("old_space", 0x045dd): "RegExpSpeciesProtector",
+  ("old_space", 0x045f1): "StringLengthProtector",
+  ("old_space", 0x04605): "ArrayIteratorProtector",
+  ("old_space", 0x04619): "ArrayBufferDetachingProtector",
+  ("old_space", 0x0462d): "PromiseHookProtector",
+  ("old_space", 0x04641): "PromiseResolveProtector",
+  ("old_space", 0x04655): "MapIteratorProtector",
+  ("old_space", 0x04669): "PromiseThenProtector",
+  ("old_space", 0x0467d): "SetIteratorProtector",
+  ("old_space", 0x04691): "StringIteratorProtector",
+  ("old_space", 0x046a5): "SingleCharacterStringCache",
+  ("old_space", 0x04aad): "StringSplitCache",
+  ("old_space", 0x04eb5): "RegExpMultipleCache",
+  ("old_space", 0x052bd): "BuiltinsConstantsTable",
+  ("old_space", 0x056e5): "AsyncFunctionAwaitRejectSharedFun",
+  ("old_space", 0x05709): "AsyncFunctionAwaitResolveSharedFun",
+  ("old_space", 0x0572d): "AsyncGeneratorAwaitRejectSharedFun",
+  ("old_space", 0x05751): "AsyncGeneratorAwaitResolveSharedFun",
+  ("old_space", 0x05775): "AsyncGeneratorYieldResolveSharedFun",
+  ("old_space", 0x05799): "AsyncGeneratorReturnResolveSharedFun",
+  ("old_space", 0x057bd): "AsyncGeneratorReturnClosedRejectSharedFun",
+  ("old_space", 0x057e1): "AsyncGeneratorReturnClosedResolveSharedFun",
+  ("old_space", 0x05805): "AsyncIteratorValueUnwrapSharedFun",
+  ("old_space", 0x05829): "PromiseAllResolveElementSharedFun",
+  ("old_space", 0x0584d): "PromiseAllSettledResolveElementSharedFun",
+  ("old_space", 0x05871): "PromiseAllSettledRejectElementSharedFun",
+  ("old_space", 0x05895): "PromiseAnyRejectElementSharedFun",
+  ("old_space", 0x058b9): "PromiseCapabilityDefaultRejectSharedFun",
+  ("old_space", 0x058dd): "PromiseCapabilityDefaultResolveSharedFun",
+  ("old_space", 0x05901): "PromiseCatchFinallySharedFun",
+  ("old_space", 0x05925): "PromiseGetCapabilitiesExecutorSharedFun",
+  ("old_space", 0x05949): "PromiseThenFinallySharedFun",
+  ("old_space", 0x0596d): "PromiseThrowerFinallySharedFun",
+  ("old_space", 0x05991): "PromiseValueThunkFinallySharedFun",
+  ("old_space", 0x059b5): "ProxyRevokeSharedFun",
 }
 
 # Lower 32 bits of first page addresses for various heap spaces.