[heap] Split heap setup to simplify read-only heap API

Splits heap setup to enable a single setup method on read-only heap.
This simplifies shared read-only heap initialization code.

Bug: v8:7464
Change-Id: If4f61e1fbc4780e19dcda2b2d50050b2c204b0e3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1518179
Commit-Queue: Maciej Goszczycki <goszczycki@google.com>
Reviewed-by: Dan Elphick <delphick@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60224}

src/heap/heap.cc

@@ -4500,7 +4500,7 @@ HeapObject Heap::AllocateRawCodeInLargeObjectSpace(int size) {
   return HeapObject();
 }
 
-void Heap::SetUp(ReadOnlyHeap* ro_heap) {
+void Heap::SetUp() {
 #ifdef V8_ENABLE_ALLOCATION_TIMEOUT
   allocation_timeout_ = NextAllocationTimeout();
 #endif
@@ -4513,9 +4513,6 @@ void Heap::SetUp(ReadOnlyHeap* ro_heap) {
   // and old_generation_size_ otherwise.
   if (!configured_) ConfigureHeapDefault();
 
-  DCHECK_NOT_NULL(ro_heap);
-  read_only_heap_ = ro_heap;
-
   mmap_region_base_ =
       reinterpret_cast<uintptr_t>(v8::internal::GetRandomMmapAddr()) &
       ~kMmapRegionMask;
@@ -4550,8 +4547,17 @@ void Heap::SetUp(ReadOnlyHeap* ro_heap) {
   for (int i = FIRST_SPACE; i <= LAST_SPACE; i++) {
     space_[i] = nullptr;
   }
+}
 
+void Heap::SetUpFromReadOnlyHeap(ReadOnlyHeap* ro_heap) {
+  DCHECK_NULL(read_only_space_);
+  DCHECK_NOT_NULL(ro_heap);
+  read_only_heap_ = ro_heap;
   space_[RO_SPACE] = read_only_space_ = ro_heap->read_only_space();
+}
+
+void Heap::SetUpSpaces() {
+  // Ensure SetUpFromReadOnlySpace has been ran.
   DCHECK_NOT_NULL(read_only_space_);
   space_[NEW_SPACE] = new_space_ =
       new NewSpace(this, memory_allocator_->data_page_allocator(),

src/heap/heap.h

@@ -580,9 +580,14 @@ class Heap {
                      size_t code_range_size_in_mb);
   void ConfigureHeapDefault();
 
-  // Prepares the heap, setting up memory areas that are needed in the isolate
-  // without actually creating any objects.
-  void SetUp(ReadOnlyHeap* ro_heap);
+  // Prepares the heap, setting up for deserialization.
+  void SetUp();
+
+  // Sets read-only heap and space.
+  void SetUpFromReadOnlyHeap(ReadOnlyHeap* ro_heap);
+
+  // Sets up the heap memory without creating any objects.
+  void SetUpSpaces();
 
   // (Re-)Initialize hash seed from flag or RNG.
   void InitializeHashSeed();

src/heap/read-only-heap.cc

@@ -5,22 +5,25 @@
 #include "src/heap/read-only-heap.h"
 
 #include "src/heap/spaces.h"
+#include "src/snapshot/read-only-deserializer.h"
 
 namespace v8 {
 namespace internal {
 
 // static
-ReadOnlyHeap* ReadOnlyHeap::GetOrCreateReadOnlyHeap(Heap* heap) {
-  return new ReadOnlyHeap(new ReadOnlySpace(heap));
+void ReadOnlyHeap::SetUp(Isolate* isolate, ReadOnlyDeserializer* des) {
+  auto* ro_heap = new ReadOnlyHeap(new ReadOnlySpace(isolate->heap()));
+  isolate->heap()->SetUpFromReadOnlyHeap(ro_heap);
+  if (des != nullptr) {
+    des->DeserializeInto(isolate);
+    ro_heap->read_only_space_->MarkAsReadOnly();
+  }
 }
 
-void ReadOnlyHeap::MaybeDeserialize(Isolate* isolate,
-                                    ReadOnlyDeserializer* des) {
-  des->DeserializeInto(isolate);
+void ReadOnlyHeap::OnCreateHeapObjectsComplete() {
+  read_only_space_->MarkAsReadOnly();
 }
 
-void ReadOnlyHeap::NotifySetupComplete() { read_only_space_->MarkAsReadOnly(); }
-
 void ReadOnlyHeap::OnHeapTearDown() {
   delete read_only_space_;
   delete this;

src/heap/read-only-heap.h

@@ -15,22 +15,22 @@ namespace v8 {
 namespace internal {
 
 class ReadOnlySpace;
+class ReadOnlyDeserializer;
 
 // This class transparently manages read-only space, roots and cache creation
 // and destruction. Eventually this will allow sharing these artifacts between
 // isolates.
-class ReadOnlyHeap {
+class ReadOnlyHeap final {
  public:
-  static ReadOnlyHeap* GetOrCreateReadOnlyHeap(Heap* heap);
-  // If necessary, deserialize read-only objects and set up read-only object
-  // cache.
-  void MaybeDeserialize(Isolate* isolate, ReadOnlyDeserializer* des);
-  // Notify read-only heap that all read-only space objects have been
-  // initialized and will not be written to.
-  void NotifySetupComplete();
-  // Frees ReadOnlySpace and itself when sharing is disabled. No-op otherwise.
-  // Read-only data should not be used within the current isolate after this is
-  // called.
+  // If necessary create read-only heap and initialize its artifacts (if the
+  // deserializer is provided).
+  // TODO(goszczycki): Ideally we'd create this without needing a heap.
+  static void SetUp(Isolate* isolate, ReadOnlyDeserializer* des);
+  // Indicate that all read-only space objects have been created and will not
+  // be written to.
+  void OnCreateHeapObjectsComplete();
+  // Indicate that the current isolate no longer requires the read-only heap and
+  // it may be safely disposed of.
   void OnHeapTearDown();
 
   // Returns whether the object resides in the read-only space.

src/isolate.cc

@@ -3334,8 +3334,9 @@ bool Isolate::Init(ReadOnlyDeserializer* read_only_deserializer,
 
   // SetUp the object heap.
   DCHECK(!heap_.HasBeenSetUp());
-  auto* read_only_heap = ReadOnlyHeap::GetOrCreateReadOnlyHeap(&heap_);
-  heap_.SetUp(read_only_heap);
+  heap_.SetUp();
+  ReadOnlyHeap::SetUp(this, read_only_deserializer);
+  heap_.SetUpSpaces();
 
   isolate_data_.external_reference_table()->Init(this);
 
@@ -3419,11 +3420,9 @@ bool Isolate::Init(ReadOnlyDeserializer* read_only_deserializer,
     CodeSpaceMemoryModificationScope modification_scope(&heap_);
 
     if (!create_heap_objects) {
-      read_only_heap->MaybeDeserialize(this, read_only_deserializer);
-      read_only_heap->NotifySetupComplete();
       startup_deserializer->DeserializeInto(this);
     } else {
-      read_only_heap->NotifySetupComplete();
+      heap_.read_only_heap()->OnCreateHeapObjectsComplete();
     }
     load_stub_cache_->Initialize();
     store_stub_cache_->Initialize();

src/snapshot/deserializer.cc

@@ -48,14 +48,18 @@ void Deserializer::Initialize(Isolate* isolate) {
   DCHECK_NOT_NULL(isolate);
   isolate_ = isolate;
   allocator()->Initialize(isolate->heap());
-  DCHECK_NULL(external_reference_table_);
-  external_reference_table_ = isolate->external_reference_table();
+
 #ifdef DEBUG
-  // Count the number of external references registered through the API.
-  num_api_references_ = 0;
-  if (isolate_->api_external_references() != nullptr) {
-    while (isolate_->api_external_references()[num_api_references_] != 0) {
-      num_api_references_++;
+  // The read-only deserializer is run by read-only heap set-up before the heap
+  // is fully set up. External reference table relies on a few parts of this
+  // set-up (like old-space), so it may be uninitialized at this point.
+  if (isolate->isolate_data()->external_reference_table()->is_initialized()) {
+    // Count the number of external references registered through the API.
+    num_api_references_ = 0;
+    if (isolate_->api_external_references() != nullptr) {
+      while (isolate_->api_external_references()[num_api_references_] != 0) {
+        num_api_references_++;
+      }
     }
   }
 #endif  // DEBUG
@@ -771,7 +775,7 @@ bool Deserializer::ReadData(TSlot current, TSlot limit, int source_space,
 
 Address Deserializer::ReadExternalReferenceCase() {
   uint32_t reference_id = static_cast<uint32_t>(source_.GetInt());
-  return external_reference_table_->address(reference_id);
+  return isolate_->external_reference_table()->address(reference_id);
 }
 
 template <typename TSlot, SerializerDeserializer::Bytecode bytecode,

src/snapshot/deserializer.h

@@ -47,7 +47,6 @@ class Deserializer : public SerializerDeserializer {
       : isolate_(nullptr),
         source_(data->Payload()),
         magic_number_(data->GetMagicNumber()),
-        external_reference_table_(nullptr),
         deserializing_user_code_(deserializing_user_code),
         can_rehash_(false) {
     allocator()->DecodeReservation(data->Reservations());
@@ -160,8 +159,6 @@ class Deserializer : public SerializerDeserializer {
   SnapshotByteSource source_;
   uint32_t magic_number_;
 
-  ExternalReferenceTable* external_reference_table_;
-
   std::vector<Map> new_maps_;
   std::vector<AllocationSite> new_allocation_sites_;
   std::vector<Code> new_code_objects_;