Reland "[wasm] Provide a global WasmCodeManager"

This is a reland of 0f90a2aa.
The issue was inverted destructor order between WasmCodeManager and
WasmEngine. WasmEngine has to be destructed first, because it contains
a barrier to ensure that background compile threads finished before
global state is being destructed.

Original change's description:
> [wasm] Provide a global WasmCodeManager
>
> The WasmCodeManager was part of the WasmEngine so far, but there is only
> exactly one WasmEngine. Hence we can pull it out, and also remove the
> pointer in the WasmCodeAllocator.
>
> The argument passed from the single constructor call is now inlined in
> the constructor itself.
>
> Drive-by: Replace "GetPlatformPageAllocator()->CommitPageSize()" by just
> "CommitPageSize()".
>
> R=jkummerow@chromium.org
>
> Bug: v8:11879
> Change-Id: I6c0e74cea308f5806d1aa479945d90b6ef8d1613
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2972909
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#75270}

Bug: v8:11879
Change-Id: I0eaa2395f5c1e30f3f7303c5f3df70c227b74d3d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2975859
Auto-Submit: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75289}

src/diagnostics/disassembler.cc

@@ -99,7 +99,7 @@ const char* V8NameConverter::NameOfAddress(byte* pc) const {
 
 #if V8_ENABLE_WEBASSEMBLY
     wasm::WasmCodeRefScope wasm_code_ref_scope;
-    if (auto* wasm_code = wasm::GetWasmEngine()->code_manager()->LookupCode(
+    if (auto* wasm_code = wasm::GetWasmCodeManager()->LookupCode(
             reinterpret_cast<Address>(pc))) {
       SNPrintF(v8_buffer_, "%p  (%s)", static_cast<void*>(pc),
                wasm::GetWasmCodeKindAsString(wasm_code->kind()));

src/diagnostics/objects-printer.cc

@@ -2862,8 +2862,7 @@ V8_EXPORT_PRIVATE extern void _v8_internal_Print_Code(void* object) {
 #if V8_ENABLE_WEBASSEMBLY
   {
     i::wasm::WasmCodeRefScope scope;
-    if (auto* wasm_code =
-            i::wasm::GetWasmEngine()->code_manager()->LookupCode(address)) {
+    if (auto* wasm_code = i::wasm::GetWasmCodeManager()->LookupCode(address)) {
       i::StdoutStream os;
       wasm_code->Disassemble(nullptr, os, address);
       return;

src/execution/frames.cc

@@ -571,7 +571,7 @@ StackFrame::Type StackFrame::ComputeType(const StackFrameIteratorBase* iterator,
     // returned {wasm_code} to be null and fall back to {GetContainingCode}.
     wasm::WasmCodeRefScope code_ref_scope;
     if (wasm::WasmCode* wasm_code =
-            wasm::GetWasmEngine()->code_manager()->LookupCode(pc)) {
+            wasm::GetWasmCodeManager()->LookupCode(pc)) {
       switch (wasm_code->kind()) {
         case wasm::WasmCode::kFunction:
           return WASM;
@@ -944,8 +944,7 @@ void CommonFrame::IterateCompiledFrame(RootVisitor* v) const {
   bool is_wasm = false;
 
 #if V8_ENABLE_WEBASSEMBLY
-  if (auto* wasm_code =
-          wasm::GetWasmEngine()->code_manager()->LookupCode(inner_pointer)) {
+  if (auto* wasm_code = wasm::GetWasmCodeManager()->LookupCode(inner_pointer)) {
     is_wasm = true;
     SafepointTable table(wasm_code);
     safepoint_entry = table.FindEntry(inner_pointer);
@@ -981,7 +980,7 @@ void CommonFrame::IterateCompiledFrame(RootVisitor* v) const {
     // directly call a Wasm function from JavaScript. In this case the
     // parameters we pass to the callee are not tagged.
     wasm::WasmCode* wasm_callee =
-        wasm::GetWasmEngine()->code_manager()->LookupCode(callee_pc());
+        wasm::GetWasmCodeManager()->LookupCode(callee_pc());
     bool is_wasm_call = (wasm_callee != nullptr);
     if (is_wasm_call) has_tagged_outgoing_params = false;
 #endif  // V8_ENABLE_WEBASSEMBLY
@@ -1861,10 +1860,7 @@ void WasmFrame::Print(StringStream* accumulator, PrintMode mode,
   wasm::WasmCodeRefScope code_ref_scope;
   accumulator->Add("WASM [");
   accumulator->PrintName(script().name());
-  Address instruction_start = wasm::GetWasmEngine()
-                                  ->code_manager()
-                                  ->LookupCode(pc())
-                                  ->instruction_start();
+  Address instruction_start = wasm_code()->instruction_start();
   base::Vector<const uint8_t> raw_func_name =
       module_object().GetRawFunctionName(function_index());
   const int kMaxPrintedFunctionName = 64;
@@ -1884,7 +1880,7 @@ void WasmFrame::Print(StringStream* accumulator, PrintMode mode,
 }
 
 wasm::WasmCode* WasmFrame::wasm_code() const {
-  return wasm::GetWasmEngine()->code_manager()->LookupCode(pc());
+  return wasm::GetWasmCodeManager()->LookupCode(pc());
 }
 
 WasmInstanceObject WasmFrame::wasm_instance() const {
@@ -1946,7 +1942,7 @@ bool WasmFrame::at_to_number_conversion() const {
   // ToNumber conversion call.
   wasm::WasmCode* code =
       callee_pc() != kNullAddress
-          ? wasm::GetWasmEngine()->code_manager()->LookupCode(callee_pc())
+          ? wasm::GetWasmCodeManager()->LookupCode(callee_pc())
           : nullptr;
   if (!code || code->kind() != wasm::WasmCode::kWasmToJsWrapper) return false;
   int offset = static_cast<int>(callee_pc() - code->instruction_start());
@@ -1958,8 +1954,7 @@ bool WasmFrame::at_to_number_conversion() const {
 }
 
 int WasmFrame::LookupExceptionHandlerInTable() {
-  wasm::WasmCode* code =
-      wasm::GetWasmEngine()->code_manager()->LookupCode(pc());
+  wasm::WasmCode* code = wasm::GetWasmCodeManager()->LookupCode(pc());
   if (!code->IsAnonymous() && code->handler_table_size() > 0) {
     HandlerTable table(code);
     int pc_offset = static_cast<int>(pc() - code->instruction_start());
@@ -1970,8 +1965,7 @@ int WasmFrame::LookupExceptionHandlerInTable() {
 
 void WasmDebugBreakFrame::Iterate(RootVisitor* v) const {
   DCHECK(caller_pc());
-  wasm::WasmCode* code =
-      wasm::GetWasmEngine()->code_manager()->LookupCode(caller_pc());
+  wasm::WasmCode* code = wasm::GetWasmCodeManager()->LookupCode(caller_pc());
   DCHECK(code);
   SafepointTable table(code);
   SafepointEntry safepoint_entry = table.FindEntry(caller_pc());

src/execution/isolate.cc

@@ -1793,7 +1793,7 @@ Object Isolate::UnwindAndFindHandler() {
         wasm::WasmCodeRefScope code_ref_scope;
         WasmFrame* wasm_frame = static_cast<WasmFrame*>(frame);
         wasm::WasmCode* wasm_code =
-            wasm::GetWasmEngine()->code_manager()->LookupCode(frame->pc());
+            wasm::GetWasmCodeManager()->LookupCode(frame->pc());
         int offset = wasm_frame->LookupExceptionHandlerInTable();
         if (offset < 0) break;
         wasm::GetWasmEngine()->SampleCatchEvent(this);
@@ -1854,8 +1854,7 @@ Object Isolate::UnwindAndFindHandler() {
         StubFrame* stub_frame = static_cast<StubFrame*>(frame);
 #if defined(DEBUG) && V8_ENABLE_WEBASSEMBLY
         wasm::WasmCodeRefScope code_ref_scope;
-        DCHECK_NULL(
-            wasm::GetWasmEngine()->code_manager()->LookupCode(frame->pc()));
+        DCHECK_NULL(wasm::GetWasmCodeManager()->LookupCode(frame->pc()));
 #endif  // defined(DEBUG) && V8_ENABLE_WEBASSEMBLY
         Code code = stub_frame->LookupCode();
         if (!code.IsCode() || code.kind() != CodeKind::BUILTIN ||

src/wasm/wasm-code-manager.cc

@@ -511,16 +511,14 @@ int WasmCode::GetSourcePositionBefore(int offset) {
 // static
 constexpr size_t WasmCodeAllocator::kMaxCodeSpaceSize;
 
-WasmCodeAllocator::WasmCodeAllocator(WasmCodeManager* code_manager,
-                                     std::shared_ptr<Counters> async_counters)
-    : code_manager_(code_manager),
-      async_counters_(std::move(async_counters)) {
+WasmCodeAllocator::WasmCodeAllocator(std::shared_ptr<Counters> async_counters)
+    : async_counters_(std::move(async_counters)) {
   owned_code_space_.reserve(4);
 }
 
 WasmCodeAllocator::~WasmCodeAllocator() {
-  code_manager_->FreeNativeModule(base::VectorOf(owned_code_space_),
-                                  committed_code_space());
+  GetWasmCodeManager()->FreeNativeModule(base::VectorOf(owned_code_space_),
+                                         committed_code_space());
 }
 
 void WasmCodeAllocator::Init(VirtualMemory code_space) {
@@ -632,9 +630,8 @@ base::Vector<byte> WasmCodeAllocator::AllocateForCode(
 
 base::Vector<byte> WasmCodeAllocator::AllocateForCodeInRegion(
     NativeModule* native_module, size_t size, base::AddressRegion region) {
-  DCHECK_EQ(code_manager_, native_module->engine()->code_manager());
   DCHECK_LT(0, size);
-  v8::PageAllocator* page_allocator = GetPlatformPageAllocator();
+  auto* code_manager = GetWasmCodeManager();
   size = RoundUp<kCodeAlignment>(size);
   base::AddressRegion code_space =
       free_code_space_.AllocateInRegion(size, region);
@@ -654,14 +651,14 @@ base::Vector<byte> WasmCodeAllocator::AllocateForCodeInRegion(
     std::tie(min_reservation, reserve_size) = ReservationSize(
         size, native_module->module()->num_declared_functions, total_reserved);
     VirtualMemory new_mem =
-        code_manager_->TryAllocate(reserve_size, reinterpret_cast<void*>(hint));
+        code_manager->TryAllocate(reserve_size, reinterpret_cast<void*>(hint));
     if (!new_mem.IsReserved() || new_mem.size() < min_reservation) {
       V8::FatalProcessOutOfMemory(nullptr, "wasm code reservation");
       UNREACHABLE();
     }
 
     base::AddressRegion new_region = new_mem.region();
-    code_manager_->AssignRange(new_region, native_module);
+    code_manager->AssignRange(new_region, native_module);
     free_code_space_.Merge(new_region);
     owned_code_space_.emplace_back(std::move(new_mem));
     native_module->AddCodeSpaceLocked(new_region);
@@ -671,7 +668,7 @@ base::Vector<byte> WasmCodeAllocator::AllocateForCodeInRegion(
     async_counters_->wasm_module_num_code_spaces()->AddSample(
         static_cast<int>(owned_code_space_.size()));
   }
-  const Address commit_page_size = page_allocator->CommitPageSize();
+  const Address commit_page_size = CommitPageSize();
   Address commit_start = RoundUp(code_space.begin(), commit_page_size);
   Address commit_end = RoundUp(code_space.end(), commit_page_size);
   // {commit_start} will be either code_space.start or the start of the next
@@ -685,7 +682,7 @@ base::Vector<byte> WasmCodeAllocator::AllocateForCodeInRegion(
   if (commit_start < commit_end) {
     for (base::AddressRegion split_range : SplitRangeByReservationsIfNeeded(
              {commit_start, commit_end - commit_start}, owned_code_space_)) {
-      code_manager_->Commit(split_range);
+      code_manager->Commit(split_range);
     }
     committed_code_space_.fetch_add(commit_end - commit_start);
     // Committed code cannot grow bigger than maximum code space size.
@@ -776,7 +773,7 @@ bool WasmCodeAllocator::SetThreadWritable(bool writable) {
   }
   writable = writable_nesting_level > 0;
 
-  int key = code_manager_->memory_protection_key_;
+  int key = GetWasmCodeManager()->memory_protection_key_;
 
   MemoryProtectionKeyPermission permissions =
       writable ? kNoRestrictions : kDisableWrite;
@@ -804,8 +801,7 @@ void WasmCodeAllocator::FreeCode(base::Vector<WasmCode* const> codes) {
   // pages to decommit into {regions_to_decommit} (decommitting is expensive,
   // so try to merge regions before decommitting).
   DisjointAllocationPool regions_to_decommit;
-  PageAllocator* allocator = GetPlatformPageAllocator();
-  size_t commit_page_size = allocator->CommitPageSize();
+  size_t commit_page_size = CommitPageSize();
   for (auto region : freed_regions.regions()) {
     auto merged_region = freed_code_space_.Merge(region);
     Address discard_start =
@@ -818,13 +814,14 @@ void WasmCodeAllocator::FreeCode(base::Vector<WasmCode* const> codes) {
     regions_to_decommit.Merge({discard_start, discard_end - discard_start});
   }
 
+  auto* code_manager = GetWasmCodeManager();
   for (auto region : regions_to_decommit.regions()) {
     size_t old_committed = committed_code_space_.fetch_sub(region.size());
     DCHECK_GE(old_committed, region.size());
     USE(old_committed);
     for (base::AddressRegion split_range :
          SplitRangeByReservationsIfNeeded(region, owned_code_space_)) {
-      code_manager_->Decommit(split_range);
+      code_manager->Decommit(split_range);
     }
   }
 }
@@ -843,7 +840,7 @@ NativeModule::NativeModule(WasmEngine* engine, const WasmFeatures& enabled,
                            std::shared_ptr<NativeModule>* shared_this)
     : engine_(engine),
       engine_scope_(engine->GetBarrierForBackgroundCompile()->TryLock()),
-      code_allocator_(engine->code_manager(), async_counters),
+      code_allocator_(async_counters),
       enabled_features_(enabled),
       module_(std::move(module)),
       import_wrapper_cache_(std::unique_ptr<WasmImportWrapperCache>(
@@ -1710,14 +1707,12 @@ NativeModule::~NativeModule() {
   import_wrapper_cache_.reset();
 }
 
-WasmCodeManager::WasmCodeManager(size_t max_committed)
-    : max_committed_code_space_(max_committed),
-      critical_committed_code_space_(max_committed / 2),
+WasmCodeManager::WasmCodeManager()
+    : max_committed_code_space_(FLAG_wasm_max_code_space * MB),
+      critical_committed_code_space_(max_committed_code_space_ / 2),
       memory_protection_key_(FLAG_wasm_memory_protection_keys
                                  ? AllocateMemoryProtectionKey()
-                                 : kNoMemoryProtectionKey) {
-  DCHECK_LE(max_committed, FLAG_wasm_max_code_space * MB);
-}
+                                 : kNoMemoryProtectionKey) {}
 
 WasmCodeManager::~WasmCodeManager() {
   // No more committed code space.
@@ -1968,7 +1963,6 @@ size_t WasmCodeManager::EstimateNativeModuleMetaDataSize(
 std::shared_ptr<NativeModule> WasmCodeManager::NewNativeModule(
     WasmEngine* engine, Isolate* isolate, const WasmFeatures& enabled,
     size_t code_size_estimate, std::shared_ptr<const WasmModule> module) {
-  DCHECK_EQ(this, GetWasmEngine()->code_manager());
   if (total_committed_code_space_.load() >
       critical_committed_code_space_.load()) {
     (reinterpret_cast<v8::Isolate*>(isolate))

src/wasm/wasm-code-manager.h

@@ -43,7 +43,6 @@ namespace wasm {
 
 class DebugInfo;
 class NativeModule;
-class WasmCodeManager;
 struct WasmCompilationResult;
 class WasmEngine;
 class WasmImportWrapperCache;
@@ -481,7 +480,7 @@ class WasmCodeAllocator {
   static constexpr size_t kMaxCodeSpaceSize = 1024 * MB;
 #endif
 
-  WasmCodeAllocator(WasmCodeManager*, std::shared_ptr<Counters> async_counters);
+  explicit WasmCodeAllocator(std::shared_ptr<Counters> async_counters);
   ~WasmCodeAllocator();
 
   // Call before use, after the {NativeModule} is set up completely.
@@ -532,9 +531,6 @@ class WasmCodeAllocator {
   static constexpr base::AddressRegion kUnrestrictedRegion{
       kNullAddress, std::numeric_limits<size_t>::max()};
 
-  // The engine-wide wasm code manager.
-  WasmCodeManager* const code_manager_;
-
   //////////////////////////////////////////////////////////////////////////////
   // These fields are protected by the mutex in {NativeModule}.
 
@@ -938,7 +934,7 @@ class V8_EXPORT_PRIVATE NativeModule final {
 
 class V8_EXPORT_PRIVATE WasmCodeManager final {
  public:
-  explicit WasmCodeManager(size_t max_committed);
+  WasmCodeManager();
   WasmCodeManager(const WasmCodeManager&) = delete;
   WasmCodeManager& operator=(const WasmCodeManager&) = delete;
 

src/wasm/wasm-debug.cc

@@ -529,8 +529,7 @@ class DebugInfoImpl {
  private:
   struct FrameInspectionScope {
     FrameInspectionScope(DebugInfoImpl* debug_info, Address pc)
-        : code(debug_info->native_module_->engine()->code_manager()->LookupCode(
-              pc)),
+        : code(wasm::GetWasmCodeManager()->LookupCode(pc)),
           pc_offset(static_cast<int>(pc - code->instruction_start())),
           debug_side_table(code->is_inspectable()
                                ? debug_info->GetDebugSideTable(code)

src/wasm/wasm-engine.cc

@@ -438,7 +438,7 @@ struct WasmEngine::NativeModuleInfo {
   int8_t num_code_gcs_triggered = 0;
 };
 
-WasmEngine::WasmEngine() : code_manager_(FLAG_wasm_max_code_space * MB) {}
+WasmEngine::WasmEngine() = default;
 
 WasmEngine::~WasmEngine() {
 #ifdef V8_ENABLE_WASM_GDB_REMOTE_DEBUGGING
@@ -1135,8 +1135,9 @@ std::shared_ptr<NativeModule> WasmEngine::NewNativeModule(
   }
 #endif  // V8_ENABLE_WASM_GDB_REMOTE_DEBUGGING
 
-  std::shared_ptr<NativeModule> native_module = code_manager_.NewNativeModule(
-      this, isolate, enabled, code_size_estimate, std::move(module));
+  std::shared_ptr<NativeModule> native_module =
+      GetWasmCodeManager()->NewNativeModule(
+          this, isolate, enabled, code_size_estimate, std::move(module));
   base::MutexGuard lock(&mutex_);
   auto pair = native_modules_.insert(std::make_pair(
       native_module.get(), std::make_unique<NativeModuleInfo>(native_module)));
@@ -1154,7 +1155,7 @@ std::shared_ptr<NativeModule> WasmEngine::NewNativeModule(
     auto* histogram =
         isolate->counters()->wasm_memory_protection_keys_support();
     bool has_mpk =
-        code_manager_.memory_protection_key_ != kNoMemoryProtectionKey;
+        GetWasmCodeManager()->memory_protection_key_ != kNoMemoryProtectionKey;
     histogram->AddSample(has_mpk ? 1 : 0);
   }
 
@@ -1339,8 +1340,7 @@ void WasmEngine::ReportLiveCodeFromStackForGC(Isolate* isolate) {
       Address osr_target = base::Memory<Address>(WasmFrame::cast(frame)->fp() -
                                                  kOSRTargetOffset);
       if (osr_target) {
-        WasmCode* osr_code =
-            GetWasmEngine()->code_manager()->LookupCode(osr_target);
+        WasmCode* osr_code = GetWasmCodeManager()->LookupCode(osr_target);
         DCHECK_NOT_NULL(osr_code);
         live_wasm_code.insert(osr_code);
       }
@@ -1368,7 +1368,7 @@ bool WasmEngine::AddPotentiallyDeadCode(WasmCode* code) {
     size_t dead_code_limit =
         FLAG_stress_wasm_code_gc
             ? 0
-            : 64 * KB + code_manager_.committed_code_space() / 10;
+            : 64 * KB + GetWasmCodeManager()->committed_code_space() / 10;
     if (new_potentially_dead_code_size_ > dead_code_limit) {
       bool inc_gc_count =
           info->num_code_gcs_triggered < std::numeric_limits<int8_t>::max();
@@ -1579,14 +1579,22 @@ void WasmEngine::PotentiallyFinishCurrentGC() {
 
 namespace {
 
-WasmEngine* global_wasm_engine = nullptr;
+struct GlobalWasmState {
+  // Note: The order of fields is important here, as the WasmEngine's destructor
+  // must run first. It contains a barrier which ensures that background threads
+  // finished, and that has to happen before the WasmCodeManager gets destroyed.
+  WasmCodeManager code_manager;
+  WasmEngine engine;
+};
+
+GlobalWasmState* global_wasm_state = nullptr;
 
 }  // namespace
 
 // static
 void WasmEngine::InitializeOncePerProcess() {
-  DCHECK_NULL(global_wasm_engine);
-  global_wasm_engine = new WasmEngine();
+  DCHECK_NULL(global_wasm_state);
+  global_wasm_state = new GlobalWasmState();
 }
 
 // static
@@ -1594,13 +1602,18 @@ void WasmEngine::GlobalTearDown() {
   // Note: This can be called multiple times in a row (see
   // test-api/InitializeAndDisposeMultiple). This is fine, as
   // {global_wasm_engine} will be nullptr then.
-  delete global_wasm_engine;
-  global_wasm_engine = nullptr;
+  delete global_wasm_state;
+  global_wasm_state = nullptr;
 }
 
 WasmEngine* GetWasmEngine() {
-  DCHECK_NOT_NULL(global_wasm_engine);
-  return global_wasm_engine;
+  DCHECK_NOT_NULL(global_wasm_state);
+  return &global_wasm_state->engine;
+}
+
+WasmCodeManager* GetWasmCodeManager() {
+  DCHECK_NOT_NULL(global_wasm_state);
+  return &global_wasm_state->code_manager;
 }
 
 // {max_mem_pages} is declared in wasm-limits.h.

src/wasm/wasm-engine.h

@@ -216,8 +216,6 @@ class V8_EXPORT_PRIVATE WasmEngine {
       Isolate* isolate, std::shared_ptr<NativeModule> shared_module,
       base::Vector<const char> source_url);
 
-  WasmCodeManager* code_manager() { return &code_manager_; }
-
   AccountingAllocator* allocator() { return &allocator_; }
 
   // Compilation statistics for TurboFan compilations.
@@ -380,7 +378,6 @@ class V8_EXPORT_PRIVATE WasmEngine {
   // calling this method.
   void PotentiallyFinishCurrentGC();
 
-  WasmCodeManager code_manager_;
   AccountingAllocator allocator_;
 
 #ifdef V8_ENABLE_WASM_GDB_REMOTE_DEBUGGING
@@ -432,6 +429,9 @@ class V8_EXPORT_PRIVATE WasmEngine {
 // Returns a reference to the WasmEngine shared by the entire process.
 V8_EXPORT_PRIVATE WasmEngine* GetWasmEngine();
 
+// Returns a reference to the WasmCodeManager shared by the entire process.
+V8_EXPORT_PRIVATE WasmCodeManager* GetWasmCodeManager();
+
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8