[wasm] Remove the isolate_ field from WasmCodeManager

The isolate is mainly used for accounting purposes. As such, it
doesn't need a field in the WasmCodeManager, and cannot have one
if it is to be made isolate independent. Instead, pass the isolate
explicitly in the appropriate cases.

R=mstarzinger@chromium.org
BUG=v8:7424

Change-Id: I539c2b33692e57605a280530bd704ef25269ad0f
Reviewed-on: https://chromium-review.googlesource.com/1073412
Commit-Queue: Ben Titzer <titzer@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53676}

src/isolate.cc

@@ -2994,9 +2994,9 @@ bool Isolate::Init(StartupDeserializer* des) {
   heap_.SetUp();
 
   // Setup the wasm engine. Currently, there's one per Isolate.
-  wasm_engine_.reset(new wasm::WasmEngine(
-      std::unique_ptr<wasm::WasmCodeManager>(new wasm::WasmCodeManager(
-          reinterpret_cast<v8::Isolate*>(this), kMaxWasmCodeMemory))));
+  wasm_engine_.reset(
+      new wasm::WasmEngine(std::unique_ptr<wasm::WasmCodeManager>(
+          new wasm::WasmCodeManager(kMaxWasmCodeMemory))));
   wasm_engine_->memory_tracker()->SetAllocationResultHistogram(
       counters()->wasm_memory_allocation_result());
   wasm_engine_->memory_tracker()->SetAddressSpaceUsageHistogram(

src/wasm/wasm-code-manager.cc

@@ -338,14 +338,14 @@ WasmCode::~WasmCode() {
 
 base::AtomicNumber<size_t> NativeModule::next_id_;
 
-NativeModule::NativeModule(uint32_t num_functions, uint32_t num_imports,
-                           bool can_request_more, VirtualMemory* code_space,
+NativeModule::NativeModule(Isolate* isolate, uint32_t num_functions,
+                           uint32_t num_imports, bool can_request_more,
+                           VirtualMemory* code_space,
                            WasmCodeManager* code_manager, ModuleEnv& env)
     : instance_id(next_id_.Increment(1)),
       num_functions_(num_functions),
       num_imported_functions_(num_imports),
-      compilation_state_(NewCompilationState(
-          reinterpret_cast<Isolate*>(code_manager->isolate_), env)),
+      compilation_state_(NewCompilationState(isolate, env)),
       free_code_space_({code_space->address(), code_space->end()}),
       wasm_code_manager_(code_manager),
       can_request_more_memory_(can_request_more),
@@ -842,7 +842,7 @@ void NativeModule::DisableTrapHandler() {
 NativeModule::~NativeModule() {
   TRACE_HEAP("Deleting native module: %p\n", reinterpret_cast<void*>(this));
   // Clear the handle at the beginning of destructor to make it robust against
-  // potential GCs in the rest of the desctructor.
+  // potential GCs in the rest of the destructor.
   if (shared_module_data_ != nullptr) {
     Isolate* isolate = shared_module_data()->GetIsolate();
     isolate->global_handles()->Destroy(
@@ -852,8 +852,7 @@ NativeModule::~NativeModule() {
   wasm_code_manager_->FreeNativeModule(this);
 }
 
-WasmCodeManager::WasmCodeManager(v8::Isolate* isolate, size_t max_committed)
-    : isolate_(isolate) {
+WasmCodeManager::WasmCodeManager(size_t max_committed) {
   DCHECK_LE(max_committed, kMaxWasmCodeMemory);
   remaining_uncommitted_code_space_.store(max_committed);
 }
@@ -885,12 +884,6 @@ bool WasmCodeManager::Commit(Address start, size_t size) {
     remaining_uncommitted_code_space_.fetch_add(size);
     return false;
   }
-  if (WouldGCHelp()) {
-    // This API does not assume main thread, and would schedule
-    // a GC if called from a different thread, instead of synchronously
-    // doing one.
-    isolate_->MemoryPressureNotification(MemoryPressureLevel::kCritical);
-  }
   return ret;
 }
 
@@ -944,15 +937,15 @@ size_t WasmCodeManager::EstimateNativeModuleSize(const WasmModule* module) {
 }
 
 std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
-    const WasmModule& module, ModuleEnv& env) {
+    Isolate* isolate, const WasmModule& module, ModuleEnv& env) {
   size_t memory_estimate = EstimateNativeModuleSize(&module);
   return NewNativeModule(
-      memory_estimate, static_cast<uint32_t>(module.functions.size()),
+      isolate, memory_estimate, static_cast<uint32_t>(module.functions.size()),
       module.num_imported_functions, kModuleCanAllocateMoreMemory, env);
 }
 
 std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
-    size_t memory_estimate, uint32_t num_functions,
+    Isolate* isolate, size_t memory_estimate, uint32_t num_functions,
     uint32_t num_imported_functions, bool can_request_more, ModuleEnv& env) {
   VirtualMemory mem;
   // If the code must be contiguous, reserve enough address space up front.
@@ -963,7 +956,7 @@ std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
     size_t size = mem.size();
     Address end = mem.end();
     std::unique_ptr<NativeModule> ret(
-        new NativeModule(num_functions, num_imported_functions,
+        new NativeModule(isolate, num_functions, num_imported_functions,
                          can_request_more, &mem, this, env));
     TRACE_HEAP("New Module: ID:%zu. Mem: %p,+%zu\n", ret->instance_id,
                reinterpret_cast<void*>(start), size);
@@ -972,8 +965,7 @@ std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
     return ret;
   }
 
-  V8::FatalProcessOutOfMemory(reinterpret_cast<Isolate*>(isolate_),
-                              "WasmCodeManager::NewNativeModule");
+  V8::FatalProcessOutOfMemory(isolate, "WasmCodeManager::NewNativeModule");
   return nullptr;
 }
 
@@ -1040,9 +1032,6 @@ void WasmCodeManager::FreeNativeModule(NativeModule* native_module) {
     module_code_size_mb_->AddSample(static_cast<int>(code_size / MB));
   }
 
-  // No need to tell the GC anything if we're destroying the heap,
-  // which we currently indicate by having the isolate_ as null
-  if (isolate_ == nullptr) return;
   remaining_uncommitted_code_space_.fetch_add(code_size);
 }
 

src/wasm/wasm-code-manager.h

@@ -17,7 +17,6 @@
 #include "src/wasm/module-compiler.h"
 
 namespace v8 {
-class Isolate;
 namespace internal {
 
 struct CodeDesc;
@@ -342,7 +341,7 @@ class V8_EXPORT_PRIVATE NativeModule final {
   friend class NativeModuleModificationScope;
 
   static base::AtomicNumber<size_t> next_id_;
-  NativeModule(uint32_t num_functions, uint32_t num_imports,
+  NativeModule(Isolate* isolate, uint32_t num_functions, uint32_t num_imports,
                bool can_request_more, VirtualMemory* code_space,
                WasmCodeManager* code_manager, ModuleEnv& env);
 
@@ -415,22 +414,20 @@ class V8_EXPORT_PRIVATE NativeModule final {
 
 class V8_EXPORT_PRIVATE WasmCodeManager final {
  public:
-  // The only reason we depend on Isolate is to report native memory used
-  // and held by a GC-ed object. We'll need to mitigate that when we
-  // start sharing wasm heaps.
-  WasmCodeManager(v8::Isolate*, size_t max_committed);
+  explicit WasmCodeManager(size_t max_committed);
   // Create a new NativeModule. The caller is responsible for its
   // lifetime. The native module will be given some memory for code,
   // which will be page size aligned. The size of the initial memory
   // is determined with a heuristic based on the total size of wasm
   // code. The native module may later request more memory.
-  std::unique_ptr<NativeModule> NewNativeModule(const WasmModule& module,
-                                                ModuleEnv& env);
-  std::unique_ptr<NativeModule> NewNativeModule(size_t memory_estimate,
-                                                uint32_t num_functions,
-                                                uint32_t num_imported_functions,
-                                                bool can_request_more,
+  // TODO(titzer): isolate is only required here for CompilationState.
+  std::unique_ptr<NativeModule> NewNativeModule(Isolate* isolate,
+                                                const WasmModule& module,
                                                 ModuleEnv& env);
+  // TODO(titzer): isolate is only required here for CompilationState.
+  std::unique_ptr<NativeModule> NewNativeModule(
+      Isolate* isolate, size_t memory_estimate, uint32_t num_functions,
+      uint32_t num_imported_functions, bool can_request_more, ModuleEnv& env);
 
   WasmCode* LookupCode(Address pc) const;
   WasmCode* GetCodeFromStartAddress(Address pc) const;
@@ -461,9 +458,6 @@ class V8_EXPORT_PRIVATE WasmCodeManager final {
   size_t active_ = 0;
   std::atomic<size_t> remaining_uncommitted_code_space_;
 
-  // TODO(mtrofin): remove the dependency on isolate.
-  v8::Isolate* isolate_;
-
   // Histogram to update with the maximum used code space for each NativeModule.
   Histogram* module_code_size_mb_ = nullptr;
 

src/wasm/wasm-objects.cc

@@ -1401,7 +1401,8 @@ Handle<WasmCompiledModule> WasmCompiledModule::New(Isolate* isolate,
         isolate->wasm_engine()->code_manager()->EstimateNativeModuleSize(
             module);
     auto native_module =
-        isolate->wasm_engine()->code_manager()->NewNativeModule(*module, env);
+        isolate->wasm_engine()->code_manager()->NewNativeModule(isolate,
+                                                                *module, env);
     Handle<Foreign> native_module_wrapper =
         Managed<wasm::NativeModule>::FromUniquePtr(isolate, memory_estimate,
                                                    std::move(native_module));

test/cctest/compiler/test-multiple-return.cc

@@ -130,8 +130,8 @@ std::unique_ptr<wasm::NativeModule> AllocateNativeModule(Isolate* isolate,
   // WasmCallDescriptor assumes that code is on the native heap and not
   // within a code object.
   std::unique_ptr<wasm::NativeModule> module =
-      isolate->wasm_engine()->code_manager()->NewNativeModule(code_size, 1, 0,
-                                                              false, env);
+      isolate->wasm_engine()->code_manager()->NewNativeModule(
+          isolate, code_size, 1, 0, false, env);
   return module;
 }
 

test/cctest/wasm/wasm-run-utils.cc

@@ -270,8 +270,8 @@ void TestBuildingGraph(Zone* zone, compiler::JSGraph* jsgraph,
                        ModuleEnv* module, FunctionSig* sig,
                        compiler::SourcePositionTable* source_position_table,
                        const byte* start, const byte* end) {
-  compiler::WasmGraphBuilder builder(module, zone, jsgraph,
-                                     sig, source_position_table);
+  compiler::WasmGraphBuilder builder(module, zone, jsgraph, sig,
+                                     source_position_table);
   TestBuildingGraphWithBuilder(&builder, zone, sig, start, end);
 }
 

test/fuzzer/multi-return.cc

@@ -158,8 +158,8 @@ std::unique_ptr<wasm::NativeModule> AllocateNativeModule(i::Isolate* isolate,
   // WasmCallDescriptor assumes that code is on the native heap and not
   // within a code object.
   std::unique_ptr<wasm::NativeModule> module =
-      isolate->wasm_engine()->code_manager()->NewNativeModule(code_size, 1, 0,
-                                                              false, env);
+      isolate->wasm_engine()->code_manager()->NewNativeModule(
+          isolate, code_size, 1, 0, false, env);
   return module;
 }
 

test/unittests/wasm/wasm-code-manager-unittest.cc

@@ -145,30 +145,13 @@ class WasmCodeManagerTest : public TestWithContext,
  public:
   using NativeModulePtr = std::unique_ptr<NativeModule>;
 
-  // We pretend all our modules have 10 functions and no imports, just so
-  // we can size up the code_table.
-  NativeModulePtr AllocFixedModule(WasmCodeManager* manager, size_t size) {
-    wasm::ModuleEnv env(nullptr, UseTrapHandler::kNoTrapHandler,
-                        RuntimeExceptionSupport::kNoRuntimeExceptionSupport);
-    return manager->NewNativeModule(size, 10, 0, false, env);
-  }
-
-  NativeModulePtr AllocGrowableModule(WasmCodeManager* manager, size_t size) {
-    wasm::ModuleEnv env(nullptr, UseTrapHandler::kNoTrapHandler,
-                        RuntimeExceptionSupport::kNoRuntimeExceptionSupport);
-    return manager->NewNativeModule(size, 10, 0, true, env);
-  }
-
   NativeModulePtr AllocModule(WasmCodeManager* manager, size_t size,
                               ModuleStyle style) {
-    switch (style) {
-      case Fixed:
-        return AllocFixedModule(manager, size);
-      case Growable:
-        return AllocGrowableModule(manager, size);
-      default:
-        UNREACHABLE();
-    }
+    bool can_request_more = style == Growable;
+    wasm::ModuleEnv env(nullptr, UseTrapHandler::kNoTrapHandler,
+                        RuntimeExceptionSupport::kNoRuntimeExceptionSupport);
+    return manager->NewNativeModule(i_isolate(), size, 10, 0, can_request_more,
+                                    env);
   }
 
   WasmCode* AddCode(NativeModule* native_module, uint32_t index, size_t size) {
@@ -183,16 +166,13 @@ class WasmCodeManagerTest : public TestWithContext,
   }
 
   size_t page() const { return AllocatePageSize(); }
-  v8::Isolate* v8_isolate() const {
-    return reinterpret_cast<v8::Isolate*>(isolate());
-  }
 };
 
 INSTANTIATE_TEST_CASE_P(Parameterized, WasmCodeManagerTest,
                         ::testing::Values(Fixed, Growable));
 
 TEST_P(WasmCodeManagerTest, EmptyCase) {
-  WasmCodeManager manager(v8_isolate(), 0 * page());
+  WasmCodeManager manager(0 * page());
   CHECK_EQ(0, manager.remaining_uncommitted_code_space());
 
   NativeModulePtr native_module = AllocModule(&manager, 1 * page(), GetParam());
@@ -202,7 +182,7 @@ TEST_P(WasmCodeManagerTest, EmptyCase) {
 }
 
 TEST_P(WasmCodeManagerTest, AllocateAndGoOverLimit) {
-  WasmCodeManager manager(v8_isolate(), 1 * page());
+  WasmCodeManager manager(1 * page());
   CHECK_EQ(1 * page(), manager.remaining_uncommitted_code_space());
   NativeModulePtr native_module = AllocModule(&manager, 1 * page(), GetParam());
   CHECK(native_module);
@@ -226,7 +206,7 @@ TEST_P(WasmCodeManagerTest, AllocateAndGoOverLimit) {
 }
 
 TEST_P(WasmCodeManagerTest, TotalLimitIrrespectiveOfModuleCount) {
-  WasmCodeManager manager(v8_isolate(), 1 * page());
+  WasmCodeManager manager(1 * page());
   NativeModulePtr nm1 = AllocModule(&manager, 1 * page(), GetParam());
   NativeModulePtr nm2 = AllocModule(&manager, 1 * page(), GetParam());
   CHECK(nm1);
@@ -238,8 +218,8 @@ TEST_P(WasmCodeManagerTest, TotalLimitIrrespectiveOfModuleCount) {
 }
 
 TEST_P(WasmCodeManagerTest, DifferentHeapsApplyLimitsIndependently) {
-  WasmCodeManager manager1(v8_isolate(), 1 * page());
-  WasmCodeManager manager2(v8_isolate(), 2 * page());
+  WasmCodeManager manager1(1 * page());
+  WasmCodeManager manager2(2 * page());
   NativeModulePtr nm1 = AllocModule(&manager1, 1 * page(), GetParam());
   NativeModulePtr nm2 = AllocModule(&manager2, 1 * page(), GetParam());
   CHECK(nm1);
@@ -252,7 +232,7 @@ TEST_P(WasmCodeManagerTest, DifferentHeapsApplyLimitsIndependently) {
 }
 
 TEST_P(WasmCodeManagerTest, GrowingVsFixedModule) {
-  WasmCodeManager manager(v8_isolate(), 3 * page());
+  WasmCodeManager manager(3 * page());
   NativeModulePtr nm = AllocModule(&manager, 1 * page(), GetParam());
   if (GetParam() == Fixed) {
     ASSERT_DEATH_IF_SUPPORTED(AddCode(nm.get(), 0, kMaxWasmCodeMemory + 1),
@@ -264,7 +244,7 @@ TEST_P(WasmCodeManagerTest, GrowingVsFixedModule) {
 }
 
 TEST_P(WasmCodeManagerTest, CommitIncrements) {
-  WasmCodeManager manager(v8_isolate(), 10 * page());
+  WasmCodeManager manager(10 * page());
   NativeModulePtr nm = AllocModule(&manager, 3 * page(), GetParam());
   WasmCode* code = AddCode(nm.get(), 0, kCodeAlignment);
   CHECK_NOT_NULL(code);
@@ -278,7 +258,7 @@ TEST_P(WasmCodeManagerTest, CommitIncrements) {
 }
 
 TEST_P(WasmCodeManagerTest, Lookup) {
-  WasmCodeManager manager(v8_isolate(), 2 * page());
+  WasmCodeManager manager(2 * page());
 
   NativeModulePtr nm1 = AllocModule(&manager, 1 * page(), GetParam());
   NativeModulePtr nm2 = AllocModule(&manager, 1 * page(), GetParam());
@@ -317,8 +297,8 @@ TEST_P(WasmCodeManagerTest, Lookup) {
 }
 
 TEST_P(WasmCodeManagerTest, MultiManagerLookup) {
-  WasmCodeManager manager1(v8_isolate(), 2 * page());
-  WasmCodeManager manager2(v8_isolate(), 2 * page());
+  WasmCodeManager manager1(2 * page());
+  WasmCodeManager manager2(2 * page());
 
   NativeModulePtr nm1 = AllocModule(&manager1, 1 * page(), GetParam());
   NativeModulePtr nm2 = AllocModule(&manager2, 1 * page(), GetParam());
@@ -340,7 +320,7 @@ TEST_P(WasmCodeManagerTest, MultiManagerLookup) {
 }
 
 TEST_P(WasmCodeManagerTest, LookupWorksAfterRewrite) {
-  WasmCodeManager manager(v8_isolate(), 2 * page());
+  WasmCodeManager manager(2 * page());
 
   NativeModulePtr nm1 = AllocModule(&manager, 1 * page(), GetParam());