[wasm] Remove redundant weak link in CompilationState.

This removes the redundant weak link to the {WasmCompiledModule} from
the {CompilationState} class and instead replaces it with a normal C++
pointer back to the associated {NativeModule}. This reduces the number
of weak links back into the garbage collected heap, such links are
costly for the GC, hard to understand and maintain, and should be kept
at a minimum. Simply explicitly encoding the one-to-one relationship
between the {NativeModule} and the {CompilationState} is easier.

R=clemensh@chromium.org

Change-Id: Ib7f79bc3a89fe463f548615a918f3fa4c9feea59
Reviewed-on: https://chromium-review.googlesource.com/995274Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52348}

src/wasm/module-compiler.cc

@@ -99,7 +99,7 @@ class CompilationState {
     base::AtomicNumber<size_t> allocated_memory_{0};
   };
 
-  explicit CompilationState(internal::Isolate* isolate);
+  CompilationState(internal::Isolate* isolate, NativeModule* native_module);
   ~CompilationState();
 
   // Needs to be set before {AddCompilationUnits} is run, which triggers
@@ -142,17 +142,7 @@ class CompilationState {
   void Abort();
 
   Isolate* isolate() { return isolate_; }
-
-  WasmCompiledModule* compiled_module() const {
-    DCHECK_NOT_NULL(compiled_module_);
-    return *compiled_module_;
-  }
-
-  void SetCompiledModule(Handle<WasmCompiledModule> compiled_module) {
-    compiled_module_ =
-        isolate_->global_handles()->Create(*compiled_module).location();
-    GlobalHandles::MakeWeak(reinterpret_cast<Object***>(&compiled_module_));
-  }
+  NativeModule* native_module() const { return native_module_; }
 
   bool failed() { return failed_; }
 
@@ -162,11 +152,7 @@ class CompilationState {
   void NotifyOnEvent(CompilationEvent event, Handle<Object> error);
 
   Isolate* isolate_;
-
-  // A phantom reference to the {WasmCompiledModule}. It is intentionally not
-  // typed {Handle<WasmCompiledModule>} because this location will be cleared
-  // when the phantom reference is cleared.
-  WasmCompiledModule** compiled_module_ = nullptr;
+  NativeModule* native_module_;
 
   std::vector<std::unique_ptr<compiler::WasmCompilationUnit>>
       compilation_units_;
@@ -1479,8 +1465,9 @@ class FinishCompileTask : public CancelableTask {
     Isolate* isolate = compilation_state_->isolate();
     HandleScope scope(isolate);
     SaveContext saved_context(isolate);
-    isolate->set_context(
-        compilation_state_->compiled_module()->native_context());
+    isolate->set_context(compilation_state_->native_module()
+                             ->compiled_module()
+                             ->native_context());
 
     TRACE_COMPILE("(4a) Finishing compilation units...\n");
     if (compilation_state_->failed()) {
@@ -3427,18 +3414,15 @@ void CompilationStateDeleter::operator()(
 }
 
 std::unique_ptr<CompilationState, CompilationStateDeleter> NewCompilationState(
-    Isolate* isolate) {
+    Isolate* isolate, NativeModule* native_module) {
   return std::unique_ptr<CompilationState, CompilationStateDeleter>(
-      new CompilationState(isolate));
-}
-
-void SetCompiledModule(CompilationState* compilation_state,
-                       Handle<WasmCompiledModule> compiled_module) {
-  compilation_state->SetCompiledModule(compiled_module);
+      new CompilationState(isolate, native_module));
 }
 
-CompilationState::CompilationState(internal::Isolate* isolate)
+CompilationState::CompilationState(internal::Isolate* isolate,
+                                   NativeModule* native_module)
     : isolate_(isolate),
+      native_module_(native_module),
       executed_units_(isolate->random_number_generator(),
                       GetMaxUsableMemorySize(isolate) / 2) {
   v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate_);
@@ -3451,13 +3435,6 @@ CompilationState::CompilationState(internal::Isolate* isolate)
 }
 
 CompilationState::~CompilationState() {
-  // Clear the handle at the beginning of destructor to make it robust against
-  // potential GCs in the rest of the desctructor.
-  if (compiled_module_ != nullptr) {
-    isolate_->global_handles()->Destroy(
-        reinterpret_cast<Object**>(compiled_module_));
-    compiled_module_ = nullptr;
-  }
   CancelAndWait();
   foreground_task_manager_.CancelAndWait();
 }

src/wasm/module-compiler.h

@@ -31,10 +31,7 @@ struct CompilationStateDeleter {
 // Wrapper to create a CompilationState exists in order to avoid having
 // the the CompilationState in the header file.
 std::unique_ptr<CompilationState, CompilationStateDeleter> NewCompilationState(
-    Isolate* isolate);
-
-void SetCompiledModule(CompilationState* compilation_state,
-                       Handle<WasmCompiledModule> compiled_module);
+    Isolate* isolate, NativeModule* native_module);
 
 MaybeHandle<WasmModuleObject> CompileToModuleObject(
     Isolate* isolate, ErrorThrower* thrower, std::unique_ptr<WasmModule> module,

src/wasm/wasm-code-manager.cc

@@ -430,7 +430,7 @@ NativeModule::NativeModule(uint32_t num_functions, uint32_t num_imports,
       code_table_(num_functions),
       num_imported_functions_(num_imports),
       compilation_state_(NewCompilationState(
-          reinterpret_cast<Isolate*>(code_manager->isolate_))),
+          reinterpret_cast<Isolate*>(code_manager->isolate_), this)),
       free_memory_(reinterpret_cast<Address>(mem->address()),
                    reinterpret_cast<Address>(mem->end())),
       wasm_code_manager_(code_manager),
@@ -532,7 +532,6 @@ void NativeModule::SetCompiledModule(
                          ->Create(*compiled_module)
                          .location();
   GlobalHandles::MakeWeak(reinterpret_cast<Object***>(&compiled_module_));
-  wasm::SetCompiledModule(compilation_state_.get(), compiled_module);
 }
 
 WasmCode* NativeModule::AddAnonymousCode(Handle<Code> code,