[wasm][cleanup] Hold wasm engine in a raw pointer

After the --wasm-shared-engine flag was removed (in
https://crrev.com/c/1864935), there is no point any more in holding the
wasm engine in a shared_ptr. The engine is initialized once for the
whole process, and only deallocated during global tear down.

R=jkummerow@chromium.org

Bug: v8:11384
Change-Id: Id8e96eaecfcab8b44842ec323c94529e9c5a5e25
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2853589Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74253}

src/execution/isolate.cc

@@ -2692,9 +2692,10 @@ void Isolate::UnregisterManagedPtrDestructor(ManagedPtrDestructor* destructor) {
 }
 
 #if V8_ENABLE_WEBASSEMBLY
-void Isolate::SetWasmEngine(std::shared_ptr<wasm::WasmEngine> engine) {
+void Isolate::SetWasmEngine(wasm::WasmEngine* engine) {
   DCHECK_NULL(wasm_engine_);  // Only call once before {Init}.
-  wasm_engine_ = std::move(engine);
+  DCHECK_NOT_NULL(engine);
+  wasm_engine_ = engine;
   wasm_engine_->AddIsolate(this);
 }
 
@@ -3136,10 +3137,7 @@ void Isolate::Deinit() {
   if (logfile != nullptr) base::Fclose(logfile);
 
 #if V8_ENABLE_WEBASSEMBLY
-  if (wasm_engine_) {
   wasm_engine_->RemoveIsolate(this);
-    wasm_engine_.reset();
-  }
 #endif  // V8_ENABLE_WEBASSEMBLY
 
   TearDownEmbeddedBlob();
@@ -3617,11 +3615,7 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
   isolate_data_.external_reference_table()->Init(this);
 
 #if V8_ENABLE_WEBASSEMBLY
-  // Setup the wasm engine.
-  if (wasm_engine_ == nullptr) {
   SetWasmEngine(wasm::WasmEngine::GetWasmEngine());
-  }
-  DCHECK_NOT_NULL(wasm_engine_);
 #endif  // V8_ENABLE_WEBASSEMBLY
 
   if (setup_delegate_ == nullptr) {

src/execution/isolate.h

@@ -1684,8 +1684,9 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   }
 
 #if V8_ENABLE_WEBASSEMBLY
-  wasm::WasmEngine* wasm_engine() const { return wasm_engine_.get(); }
-  void SetWasmEngine(std::shared_ptr<wasm::WasmEngine> engine);
+  // TODO(wasm): Replace all uses by {WasmEngine::GetWasmEngine}?
+  wasm::WasmEngine* wasm_engine() const { return wasm_engine_; }
+  void SetWasmEngine(wasm::WasmEngine* engine);
 
   void AddSharedWasmMemory(Handle<WasmMemoryObject> memory_object);
 #endif  // V8_ENABLE_WEBASSEMBLY
@@ -2135,7 +2136,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   size_t elements_deletion_counter_ = 0;
 
 #if V8_ENABLE_WEBASSEMBLY
-  std::shared_ptr<wasm::WasmEngine> wasm_engine_;
+  wasm::WasmEngine* wasm_engine_ = nullptr;
 #endif  // V8_ENABLE_WEBASSEMBLY
 
   std::unique_ptr<TracingCpuProfilerImpl> tracing_cpu_profiler_;

src/wasm/wasm-engine.cc

@@ -1561,24 +1561,29 @@ void WasmEngine::PotentiallyFinishCurrentGC() {
 
 namespace {
 
-DEFINE_LAZY_LEAKY_OBJECT_GETTER(std::shared_ptr<WasmEngine>,
-                                GetSharedWasmEngine)
+WasmEngine* global_wasm_engine = nullptr;
 
 }  // namespace
 
 // static
 void WasmEngine::InitializeOncePerProcess() {
-  *GetSharedWasmEngine() = std::make_shared<WasmEngine>();
+  DCHECK_NULL(global_wasm_engine);
+  global_wasm_engine = new WasmEngine();
 }
 
 // static
 void WasmEngine::GlobalTearDown() {
-  GetSharedWasmEngine()->reset();
+  // 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;
 }
 
 // static
-std::shared_ptr<WasmEngine> WasmEngine::GetWasmEngine() {
-  return *GetSharedWasmEngine();
+WasmEngine* WasmEngine::GetWasmEngine() {
+  DCHECK_NOT_NULL(global_wasm_engine);
+  return global_wasm_engine;
 }
 
 // {max_mem_pages} is declared in wasm-limits.h.

src/wasm/wasm-engine.h

@@ -357,10 +357,8 @@ class V8_EXPORT_PRIVATE WasmEngine {
   static void InitializeOncePerProcess();
   static void GlobalTearDown();
 
-  // Returns a reference to the WasmEngine shared by the entire process. Try to
-  // use {Isolate::wasm_engine} instead if it is available, which encapsulates
-  // engine lifetime decisions during Isolate bootstrapping.
-  static std::shared_ptr<WasmEngine> GetWasmEngine();
+  // Returns a reference to the WasmEngine shared by the entire process.
+  static WasmEngine* GetWasmEngine();
 
  private:
   struct CurrentGCInfo;

test/cctest/wasm/test-wasm-serialization.cc

@@ -257,7 +257,6 @@ TEST(BlockWasmCodeGenAtDeserialization) {
 }
 
 UNINITIALIZED_TEST(CompiledWasmModulesTransfer) {
-  i::wasm::WasmEngine::InitializeOncePerProcess();
   v8::internal::AccountingAllocator allocator;
   Zone zone(&allocator, ZONE_NAME);
 

test/cctest/wasm/test-wasm-shared-engine.cc

@@ -22,40 +22,15 @@ namespace internal {
 namespace wasm {
 namespace test_wasm_shared_engine {
 
-// Helper class representing a WebAssembly engine that is capable of being
-// shared between multiple Isolates, sharing the underlying generated code.
-class SharedEngine {
- public:
-  ~SharedEngine() {
-    // Ensure no remaining uses exist.
-    CHECK(wasm_engine_.unique());
-  }
-
-  WasmEngine* engine() const { return wasm_engine_.get(); }
-  WasmCodeManager* code_manager() const { return engine()->code_manager(); }
-
-  int NumberOfExportedEngineUses() const {
-    // This class holds one implicit use itself, which we discount.
-    return static_cast<int>(wasm_engine_.use_count()) - 1;
-  }
-
-  std::shared_ptr<WasmEngine> ExportEngineForSharing() { return wasm_engine_; }
-
- private:
-  std::shared_ptr<WasmEngine> wasm_engine_ = std::make_unique<WasmEngine>();
-};
-
 // Helper type definition representing a WebAssembly module shared between
 // multiple Isolates with implicit reference counting.
 using SharedModule = std::shared_ptr<NativeModule>;
 
-// Helper class representing an Isolate based on a given shared WebAssembly
-// engine available at construction time.
+// Helper class representing an Isolate that uses the process-wide (shared) wasm
+// engine.
 class SharedEngineIsolate {
  public:
-  explicit SharedEngineIsolate(SharedEngine* engine)
-      : isolate_(v8::Isolate::Allocate()) {
-    isolate()->SetWasmEngine(engine->ExportEngineForSharing());
+  SharedEngineIsolate() : isolate_(v8::Isolate::Allocate()) {
     v8::Isolate::CreateParams create_params;
     create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
     v8::Isolate::Initialize(isolate_, create_params);
@@ -111,19 +86,16 @@ class SharedEngineIsolate {
 // with a shared WebAssembly engine available at construction time.
 class SharedEngineThread : public v8::base::Thread {
  public:
-  SharedEngineThread(SharedEngine* engine,
+  explicit SharedEngineThread(
       std::function<void(SharedEngineIsolate*)> callback)
-      : Thread(Options("SharedEngineThread")),
-        engine_(engine),
-        callback_(callback) {}
+      : Thread(Options("SharedEngineThread")), callback_(callback) {}
 
   void Run() override {
-    SharedEngineIsolate isolate(engine_);
+    SharedEngineIsolate isolate;
     callback_(&isolate);
   }
 
  private:
-  SharedEngine* engine_;
   std::function<void(SharedEngineIsolate*)> callback_;
 };
 
@@ -201,34 +173,16 @@ Handle<WasmInstanceObject> CompileAndInstantiateAsync(
 
 }  // namespace
 
-TEST(SharedEngineUseCount) {
-  SharedEngine engine;
-  CHECK_EQ(0, engine.NumberOfExportedEngineUses());
-  {
-    SharedEngineIsolate isolate(&engine);
-    CHECK_EQ(1, engine.NumberOfExportedEngineUses());
-  }
-  CHECK_EQ(0, engine.NumberOfExportedEngineUses());
-  {
-    SharedEngineIsolate isolate1(&engine);
-    CHECK_EQ(1, engine.NumberOfExportedEngineUses());
-    SharedEngineIsolate isolate2(&engine);
-    CHECK_EQ(2, engine.NumberOfExportedEngineUses());
-  }
-  CHECK_EQ(0, engine.NumberOfExportedEngineUses());
-}
-
 TEST(SharedEngineRunSeparated) {
-  SharedEngine engine;
   {
-    SharedEngineIsolate isolate(&engine);
+    SharedEngineIsolate isolate;
     HandleScope scope(isolate.isolate());
     ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23);
     Handle<WasmInstanceObject> instance = isolate.CompileAndInstantiate(buffer);
     CHECK_EQ(23, isolate.Run(instance));
   }
   {
-    SharedEngineIsolate isolate(&engine);
+    SharedEngineIsolate isolate;
     HandleScope scope(isolate.isolate());
     ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 42);
     Handle<WasmInstanceObject> instance = isolate.CompileAndInstantiate(buffer);
@@ -237,10 +191,9 @@ TEST(SharedEngineRunSeparated) {
 }
 
 TEST(SharedEngineRunImported) {
-  SharedEngine engine;
   SharedModule module;
   {
-    SharedEngineIsolate isolate(&engine);
+    SharedEngineIsolate isolate;
     HandleScope scope(isolate.isolate());
     ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23);
     Handle<WasmInstanceObject> instance = isolate.CompileAndInstantiate(buffer);
@@ -248,7 +201,7 @@ TEST(SharedEngineRunImported) {
     CHECK_EQ(23, isolate.Run(instance));
   }
   {
-    SharedEngineIsolate isolate(&engine);
+    SharedEngineIsolate isolate;
     HandleScope scope(isolate.isolate());
     Handle<WasmInstanceObject> instance = isolate.ImportInstance(module);
     CHECK_EQ(23, isolate.Run(instance));
@@ -256,15 +209,14 @@ TEST(SharedEngineRunImported) {
 }
 
 TEST(SharedEngineRunThreadedBuildingSync) {
-  SharedEngine engine;
-  SharedEngineThread thread1(&engine, [](SharedEngineIsolate* isolate) {
+  SharedEngineThread thread1([](SharedEngineIsolate* isolate) {
     HandleScope scope(isolate->isolate());
     ZoneBuffer* buffer = BuildReturnConstantModule(isolate->zone(), 23);
     Handle<WasmInstanceObject> instance =
         isolate->CompileAndInstantiate(buffer);
     CHECK_EQ(23, isolate->Run(instance));
   });
-  SharedEngineThread thread2(&engine, [](SharedEngineIsolate* isolate) {
+  SharedEngineThread thread2([](SharedEngineIsolate* isolate) {
     HandleScope scope(isolate->isolate());
     ZoneBuffer* buffer = BuildReturnConstantModule(isolate->zone(), 42);
     Handle<WasmInstanceObject> instance =
@@ -278,15 +230,14 @@ TEST(SharedEngineRunThreadedBuildingSync) {
 }
 
 TEST(SharedEngineRunThreadedBuildingAsync) {
-  SharedEngine engine;
-  SharedEngineThread thread1(&engine, [](SharedEngineIsolate* isolate) {
+  SharedEngineThread thread1([](SharedEngineIsolate* isolate) {
     HandleScope scope(isolate->isolate());
     ZoneBuffer* buffer = BuildReturnConstantModule(isolate->zone(), 23);
     Handle<WasmInstanceObject> instance =
         CompileAndInstantiateAsync(isolate, buffer);
     CHECK_EQ(23, isolate->Run(instance));
   });
-  SharedEngineThread thread2(&engine, [](SharedEngineIsolate* isolate) {
+  SharedEngineThread thread2([](SharedEngineIsolate* isolate) {
     HandleScope scope(isolate->isolate());
     ZoneBuffer* buffer = BuildReturnConstantModule(isolate->zone(), 42);
     Handle<WasmInstanceObject> instance =
@@ -300,21 +251,20 @@ TEST(SharedEngineRunThreadedBuildingAsync) {
 }
 
 TEST(SharedEngineRunThreadedExecution) {
-  SharedEngine engine;
   SharedModule module;
   {
-    SharedEngineIsolate isolate(&engine);
+    SharedEngineIsolate isolate;
     HandleScope scope(isolate.isolate());
     ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23);
     Handle<WasmInstanceObject> instance = isolate.CompileAndInstantiate(buffer);
     module = isolate.ExportInstance(instance);
   }
-  SharedEngineThread thread1(&engine, [module](SharedEngineIsolate* isolate) {
+  SharedEngineThread thread1([module](SharedEngineIsolate* isolate) {
     HandleScope scope(isolate->isolate());
     Handle<WasmInstanceObject> instance = isolate->ImportInstance(module);
     CHECK_EQ(23, isolate->Run(instance));
   });
-  SharedEngineThread thread2(&engine, [module](SharedEngineIsolate* isolate) {
+  SharedEngineThread thread2([module](SharedEngineIsolate* isolate) {
     HandleScope scope(isolate->isolate());
     Handle<WasmInstanceObject> instance = isolate->ImportInstance(module);
     CHECK_EQ(23, isolate->Run(instance));
@@ -326,10 +276,9 @@ TEST(SharedEngineRunThreadedExecution) {
 }
 
 TEST(SharedEngineRunThreadedTierUp) {
-  SharedEngine engine;
   SharedModule module;
   {
-    SharedEngineIsolate isolate(&engine);
+    SharedEngineIsolate isolate;
     HandleScope scope(isolate.isolate());
     ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23);
     Handle<WasmInstanceObject> instance = isolate.CompileAndInstantiate(buffer);
@@ -338,7 +287,7 @@ TEST(SharedEngineRunThreadedTierUp) {
   constexpr int kNumberOfThreads = 5;
   std::list<SharedEngineThread> threads;
   for (int i = 0; i < kNumberOfThreads; ++i) {
-    threads.emplace_back(&engine, [module](SharedEngineIsolate* isolate) {
+    threads.emplace_back([module](SharedEngineIsolate* isolate) {
       constexpr int kNumberOfIterations = 100;
       HandleScope scope(isolate->isolate());
       Handle<WasmInstanceObject> instance = isolate->ImportInstance(module);
@@ -347,7 +296,7 @@ TEST(SharedEngineRunThreadedTierUp) {
       }
     });
   }
-  threads.emplace_back(&engine, [module](SharedEngineIsolate* isolate) {
+  threads.emplace_back([module](SharedEngineIsolate* isolate) {
     HandleScope scope(isolate->isolate());
     Handle<WasmInstanceObject> instance = isolate->ImportInstance(module);
     WasmFeatures detected = WasmFeatures::None();