Revert "Support sharing ReadOnly heap for multi-cage mode."

This reverts commit 57232a7. Reason for revert: Performance regressions: crbug.com/357931034 Original change's description: > Support sharing ReadOnly heap for multi-cage mode. > > RO heap will be shared only for isolates inside one isolate group > and so, the RO heap and its artifacts will be per isolate group. > > This patch unifies code for obtaining RO artifacts for all configuration > and removes dead code like PointerCompressedReadOnlyArtifacts > since we use isolate groups in all configurations. > > Also, current tests that test sharing heap are skipped > for multi-cage mode because in the current execution model each isolate > creates its isolate group so the heap isn't shared. > > Bug: 347026227 > Change-Id: I7219e4d7d526560542e1a059ad7bffc385bf31ab > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/ /5632279 > Reviewed-by: Andy Wingo <[email protected]> > Commit-Queue: Dmitry Bezhetskov <[email protected]> > Reviewed-by: Leszek Swirski <[email protected]> > Cr-Commit-Position: refs/heads/main@{#95463} Bug: 347026227 Change-Id: Ie5bc50d656170a918a8b9e09f7a7377a12b6ebf3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/ /5769212 Auto-Submit: Leszek Swirski <[email protected]> Commit-Queue: Leszek Swirski <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Cr-Commit-Position: refs/heads/main@{#95516}
v8 · Aug 7, 2024 · ea7e53d · ea7e53d
1 parent 854b664
commit ea7e53d
Show file tree

Hide file tree

Showing 22 changed files with 543 additions and 218 deletions.
diff --git a/BUILD.gn b/BUILD.gn
@@ -589,7 589,8 @@ if (v8_enable_short_builtin_calls &&
  v8_enable_short_builtin_calls = false
 }
 if (v8_enable_shared_ro_heap == "") {
- v8_enable_shared_ro_heap = true
  v8_enable_shared_ro_heap = !v8_enable_pointer_compression ||
  v8_enable_pointer_compression_shared_cage
 }
 
 if (v8_enable_sandbox == "") {
@@ -683,6 684,13 @@ assert(!v8_disable_write_barriers || v8_enable_single_generation,
 assert(v8_current_cpu == "arm64" || !v8_control_flow_integrity,
  "Control-flow integrity is only supported on arm64")
 
 if (v8_enable_shared_ro_heap && v8_enable_pointer_compression &&
  !v8_enable_pointer_compression_shared_cage) {
  assert(
  is_linux || is_chromeos || is_android,
  "Sharing read-only heap with pointer compression is only supported on Linux or Android")
 }
 
 assert(!v8_enable_map_packing || !v8_enable_pointer_compression,
  "Map packing does not support pointer compression")
 
@@ -2710,7 2718,10 @@ action("v8_dump_build_config") {
  mips_use_msa_var = mips_use_msa
  }
 
- js_shared_memory = v8_enable_shared_ro_heap && !v8_disable_write_barriers
  js_shared_memory =
  v8_enable_shared_ro_heap && (!v8_enable_pointer_compression ||
  v8_enable_pointer_compression_shared_cage) &&
  !v8_disable_write_barriers
  simd_mips = mips_arch_variant_var == "r6" && mips_use_msa
  simulator_run = target_cpu != v8_target_cpu
  use_sanitizer = is_asan || is_cfi || is_msan || is_tsan || is_ubsan

diff --git a/src/DEPS b/src/DEPS
@@ -18,7 18,6 @@ include_rules = [
  "-src/heap",
  " src/heap/memory-chunk-metadata.h",
  " src/heap/code-range.h",
- " src/heap/read-only-spaces.h",
  " src/heap/trusted-range.h",
  " src/heap/combined-heap.h",
  " src/heap/factory.h",

diff --git a/src/builtins/builtins.h b/src/builtins/builtins.h
@@ -75,12 75,13 @@ class Builtins {
  // Disassembler support.
  const char* Lookup(Address pc);
 
-#if !defined(V8_SHORT_BUILTIN_CALLS) || defined(V8_COMPRESS_POINTERS)
 #if !defined(V8_SHORT_BUILTIN_CALLS) || \
  defined(V8_COMPRESS_POINTERS_IN_SHARED_CAGE)
  static constexpr bool kCodeObjectsAreInROSpace = true;
 #else
  static constexpr bool kCodeObjectsAreInROSpace = false;
 #endif // !defined(V8_SHORT_BUILTIN_CALLS) || \
- // defined(V8_COMPRESS_POINTERS)
  // defined(V8_COMPRESS_POINTERS_IN_SHARED_CAGE)
 
 #define ADD_ONE(Name, ...) 1
  static constexpr int kBuiltinCount = 0 BUILTIN_LIST(

diff --git a/src/common/globals.h b/src/common/globals.h
@@ -119,7 119,10 @@ namespace internal {
 #define COMPRESS_POINTERS_IN_MULTIPLE_CAGES_BOOL false
 #endif
 
-#if defined(V8_SHARED_RO_HEAP) && !defined(V8_DISABLE_WRITE_BARRIERS)
 #if defined(V8_SHARED_RO_HEAP) && \
  (!defined(V8_COMPRESS_POINTERS) || \
  defined(V8_COMPRESS_POINTERS_IN_SHARED_CAGE)) && \
  !defined(V8_DISABLE_WRITE_BARRIERS)
 #define V8_CAN_CREATE_SHARED_HEAP_BOOL true
 #else
 #define V8_CAN_CREATE_SHARED_HEAP_BOOL false

diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
@@ -504,6 504,8 @@ size_t Isolate::HashIsolateForEmbeddedBlob() {
  return hash;
 }
 
 Isolate* Isolate::process_wide_shared_space_isolate_{nullptr};
 
 thread_local Isolate::PerIsolateThreadData* g_current_per_isolate_thread_data_
  V8_CONSTINIT = nullptr;
 thread_local Isolate* g_current_isolate_ V8_CONSTINIT = nullptr;
@@ -3970,10 3972,11 @@ void Isolate::Delete(Isolate* isolate) {
  SetIsolateThreadLocals(saved_isolate, saved_data);
 }
 
-void Isolate::SetUpFromReadOnlyArtifacts(ReadOnlyArtifacts* artifacts,
-   ReadOnlyHeap* ro_heap) {
 void Isolate::SetUpFromReadOnlyArtifacts(
  std::shared_ptr<ReadOnlyArtifacts> artifacts, ReadOnlyHeap* ro_heap) {
  if (ReadOnlyHeap::IsReadOnlySpaceShared()) {
  DCHECK_NOT_NULL(artifacts);
  artifacts_ = artifacts;
  InitializeNextUniqueSfiId(artifacts->initial_next_unique_sfi_id());
  } else {
  DCHECK_NULL(artifacts);
@@ -4010,8 4013,6 @@ Isolate::Isolate(IsolateGroup* isolate_group)
  TRACE_ISOLATE(constructor);
  CheckIsolateLayout();
 
- isolate_group->IncrementIsolateCount();
-
  // ThreadManager is initialized early to support locking an isolate
  // before it is entered.
  thread_manager_ = new ThreadManager(this);
@@ -4342,7 4343,6 @@ void Isolate::Deinit() {
  DumpAndResetStats();
 
  heap_.TearDown();
- ReadOnlyHeap::TearDown(this);
 
  delete inner_pointer_to_code_cache_;
  inner_pointer_to_code_cache_ = nullptr;
@@ -4441,13 4441,17 @@ void Isolate::SetIsolateThreadLocals(Isolate* isolate,
  g_current_per_isolate_thread_data_ = data;
 
 #ifdef V8_COMPRESS_POINTERS_IN_MULTIPLE_CAGES
- V8HeapCompressionScheme::InitBase(isolate ? isolate->cage_base()
- : kNullAddress);
- IsolateGroup::set_current(isolate ? isolate->isolate_group() : nullptr);
  if (isolate) {
  V8HeapCompressionScheme::InitBase(isolate->cage_base());
 #ifdef V8_EXTERNAL_CODE_SPACE
- ExternalCodeCompressionScheme::InitBase(isolate ? isolate->code_cage_base()
- : kNullAddress);
-#endif
  ExternalCodeCompressionScheme::InitBase(isolate->code_cage_base());
 #endif // V8_EXTERNAL_CODE_SPACE
  } else {
  V8HeapCompressionScheme::InitBase(kNullAddress);
 #ifdef V8_EXTERNAL_CODE_SPACE
  ExternalCodeCompressionScheme::InitBase(kNullAddress);
 #endif // V8_EXTERNAL_CODE_SPACE
  }
 #endif // V8_COMPRESS_POINTERS_IN_MULTIPLE_CAGES
 
  if (isolate && isolate->main_thread_local_isolate()) {
@@ -4540,6 4544,13 @@ Isolate::~Isolate() {
  delete default_microtask_queue_;
  default_microtask_queue_ = nullptr;
 
  // The ReadOnlyHeap should not be destroyed when sharing without pointer
  // compression as the object itself is shared.
  if (read_only_heap_->IsOwnedByIsolate()) {
  delete read_only_heap_;
  read_only_heap_ = nullptr;
  }
 
  // isolate_group_ released in caller, to ensure that all member destructors
  // run before potentially unmapping the isolate's VirtualMemoryArea.
 }
@@ -5243,12 5254,12 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
  Isolate* use_shared_space_isolate = nullptr;
 
  if (HasFlagThatRequiresSharedHeap()) {
- if (isolate_group_->has_shared_space_isolate()) {
  if (process_wide_shared_space_isolate_) {
  owns_shareable_data_ = false;
- use_shared_space_isolate = isolate_group_->shared_space_isolate();
  use_shared_space_isolate = process_wide_shared_space_isolate_;
  } else {
- isolate_group_->init_shared_space_isolate(this);
- use_shared_space_isolate = isolate_group_->shared_space_isolate();
  process_wide_shared_space_isolate_ = this;
  use_shared_space_isolate = this;
  is_shared_space_isolate_ = true;
  DCHECK(owns_shareable_data_);
  }

diff --git a/src/execution/isolate.h b/src/execution/isolate.h
@@ -646,7 646,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
  // for legacy API reasons.
  static void Delete(Isolate* isolate);
 
- void SetUpFromReadOnlyArtifacts(ReadOnlyArtifacts* artifacts,
  void SetUpFromReadOnlyArtifacts(std::shared_ptr<ReadOnlyArtifacts> artifacts,
  ReadOnlyHeap* ro_heap);
  void set_read_only_heap(ReadOnlyHeap* ro_heap) { read_only_heap_ = ro_heap; }
 
@@ -757,7 757,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
  }
 
  ReadOnlyArtifacts* read_only_artifacts() const {
- ReadOnlyArtifacts* artifacts = isolate_group()->read_only_artifacts();
  ReadOnlyArtifacts* artifacts = artifacts_.get();
  DCHECK_IMPLIES(ReadOnlyHeap::IsReadOnlySpaceShared(), artifacts != nullptr);
  return artifacts;
  }
@@ -2305,6 2305,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
  EntryStackItem* previous_item;
  };
 
  static Isolate* process_wide_shared_space_isolate_;
 
  void Deinit();
 
  static void SetIsolateThreadLocals(Isolate* isolate,
@@ -2364,6 2366,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
  IsolateGroup* isolate_group_;
  Heap heap_;
  ReadOnlyHeap* read_only_heap_ = nullptr;
  std::shared_ptr<ReadOnlyArtifacts> artifacts_;
 
  // These are guaranteed empty when !OwnsStringTables().
  std::unique_ptr<StringTable> string_table_;

diff --git a/src/heap/heap.cc b/src/heap/heap.cc
@@ -6270,6 6270,7 @@ void Heap::TearDown() {
  space_[i].reset();
  }
 
  isolate()->read_only_heap()->OnHeapTearDown(this);
  read_only_space_ = nullptr;
 
  memory_allocator()->TearDown();

diff --git a/src/heap/read-only-heap-inl.h b/src/heap/read-only-heap-inl.h
@@ -15,8 15,7 @@ namespace internal {
 // static
 ReadOnlyRoots ReadOnlyHeap::EarlyGetReadOnlyRoots(Tagged<HeapObject> object) {
 #ifdef V8_SHARED_RO_HEAP
- ReadOnlyHeap* shared_ro_heap =
- IsolateGroup::current()->shared_read_only_heap();
  auto* shared_ro_heap = SoleReadOnlyHeap::shared_ro_heap_;
  if (shared_ro_heap && shared_ro_heap->roots_init_complete()) {
  return ReadOnlyRoots(shared_ro_heap->read_only_roots_);
  }
@@ -29,8 28,7 @@ ReadOnlyRoots ReadOnlyHeap::EarlyGetReadOnlyRoots(Tagged<HeapObject> object) {
 // static
 ReadOnlyRoots ReadOnlyHeap::GetReadOnlyRoots(Tagged<HeapObject> object) {
 #ifdef V8_SHARED_RO_HEAP
- ReadOnlyHeap* shared_ro_heap =
- IsolateGroup::current()->shared_read_only_heap();
  auto* shared_ro_heap = SoleReadOnlyHeap::shared_ro_heap_;
  // If this check fails in code that runs during initialization use
  // EarlyGetReadOnlyRoots instead.
  DCHECK(shared_ro_heap && shared_ro_heap->roots_init_complete());