Reland "[wasm] Decouple background compile jobs from NativeModule"

This is a reland of 92d9b09c.
Patch unchanged, errors fixed by https://crrev.com/c/1430059.

Original change's description:
> [wasm] Decouple background compile jobs from NativeModule
>
> Background compile jobs should not keep the NativeModule alive, for two
> reasons:
> 1) We sometimes have to wait for background compilation to finish (from
>    a foreground task!). This introduces unnecessary latency.
> 2) Giving the background compile tasks shared ownership of the
>    NativeModule causes the NativeModule (and the CompilationState) to
>    be freed from background tasks, which is error-prone (see
>    https://crrev.com/c/1400420).
>
> Instead, this CL introduces a BackgroundCompileToken which is held
> alive by the NativeModule and all background compile jobs. The initial
> and the final phase of compilation (getting and submitting work)
> synchronize on this token to check and ensure that the NativeModule is
> and stays alive. During compilation itself, the mutex is released, such
> that the NativeModule can die.
> The destructor of the NativeModule cancels the BackgroundCompileToken.
> Immediately afterwards, the NativeModule and the CompilationState can
> die.
>
> This change allows to remove two hacks introduced previously: The atomic
> {aborted_} flag and the {FreeCallbacksTask}.
>
> R=mstarzinger@chromium.org
> CC=titzer@chromium.org
>
> Bug: v8:8689, v8:7921
> Change-Id: I42e06eab3c944b0988286f2ce18e3c294535dfb6
> Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel
> Reviewed-on: https://chromium-review.googlesource.com/c/1421364
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#59020}

TBR=mstarzinger@chromium.org

Bug: v8:8689, v8:7921
Change-Id: Iead972ef77c8503da7246cab48e7693b176d8f02
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel
Reviewed-on: https://chromium-review.googlesource.com/c/1429862Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59035}

src/v8.cc

@@ -48,10 +48,10 @@ bool V8::Initialize() {
 
 
 void V8::TearDown() {
+  wasm::WasmEngine::GlobalTearDown();
 #if defined(USE_SIMULATOR)
   Simulator::GlobalTearDown();
 #endif
-  wasm::WasmEngine::GlobalTearDown();
   CallDescriptors::TearDown();
   Bootstrapper::TearDownExtensions();
   ElementsAccessor::TearDown();

src/wasm/compilation-environment.h

@@ -15,6 +15,7 @@ namespace internal {
 namespace wasm {
 
 class NativeModule;
+class WasmCode;
 class WasmError;
 
 enum RuntimeExceptionSupport : bool {
@@ -112,6 +113,8 @@ class CompilationState {
 
   bool failed() const;
 
+  void OnFinishedUnit(ExecutionTier, WasmCode*);
+
  private:
   friend class NativeModule;
   friend class WasmCompilationUnit;

src/wasm/wasm-code-manager.h

@@ -30,7 +30,6 @@ namespace wasm {
 
 class NativeModule;
 class WasmCodeManager;
-class WasmEngine;
 class WasmMemoryTracker;
 class WasmImportWrapperCache;
 struct WasmModule;
@@ -326,8 +325,9 @@ class V8_EXPORT_PRIVATE NativeModule final {
 
   CompilationState* compilation_state() { return compilation_state_.get(); }
 
-  // Create a {CompilationEnv} object for compilation. Only valid as long as
-  // this {NativeModule} is alive.
+  // Create a {CompilationEnv} object for compilation. The caller has to ensure
+  // that the {WasmModule} pointer stays valid while the {CompilationEnv} is
+  // being used.
   CompilationEnv CreateCompilationEnv() const;
 
   uint32_t num_functions() const {
@@ -341,6 +341,7 @@ class V8_EXPORT_PRIVATE NativeModule final {
   bool lazy_compile_frozen() const { return lazy_compile_frozen_; }
   Vector<const uint8_t> wire_bytes() const { return wire_bytes_->as_vector(); }
   const WasmModule* module() const { return module_.get(); }
+  std::shared_ptr<const WasmModule> shared_module() const { return module_; }
   size_t committed_code_space() const { return committed_code_space_.load(); }
 
   void SetWireBytes(OwnedVector<const uint8_t> wire_bytes);
@@ -423,8 +424,8 @@ class V8_EXPORT_PRIVATE NativeModule final {
   // to be consistent across asynchronous compilations later.
   const WasmFeatures enabled_features_;
 
-  // TODO(clemensh): Make this a unique_ptr (requires refactoring
-  // AsyncCompileJob).
+  // The decoded module, stored in a shared_ptr such that background compile
+  // tasks can keep this alive.
   std::shared_ptr<const WasmModule> module_;
 
   // Wire bytes, held in a shared_ptr so they can be kept alive by the

src/wasm/wasm-engine.cc

@@ -25,6 +25,8 @@ WasmEngine::WasmEngine()
     : code_manager_(&memory_tracker_, FLAG_wasm_max_code_space * MB) {}
 
 WasmEngine::~WasmEngine() {
+  // Synchronize on all background compile tasks.
+  background_compile_task_manager_.CancelAndWait();
   // All AsyncCompileJobs have been canceled.
   DCHECK(jobs_.empty());
   // All Isolates have been deregistered.

src/wasm/wasm-engine.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <unordered_set>
 
+#include "src/cancelable-task.h"
 #include "src/wasm/wasm-code-manager.h"
 #include "src/wasm/wasm-memory.h"
 #include "src/wasm/wasm-tier.h"
@@ -154,6 +155,12 @@ class V8_EXPORT_PRIVATE WasmEngine {
   // engines this might be a pointer to a new instance or to a shared one.
   static std::shared_ptr<WasmEngine> GetWasmEngine();
 
+  template <typename T, typename... Args>
+  std::unique_ptr<T> NewBackgroundCompileTask(Args&&... args) {
+    return base::make_unique<T>(&background_compile_task_manager_,
+                                std::forward<Args>(args)...);
+  }
+
  private:
   AsyncCompileJob* CreateAsyncCompileJob(
       Isolate* isolate, const WasmFeatures& enabled,
@@ -165,6 +172,10 @@ class V8_EXPORT_PRIVATE WasmEngine {
   WasmCodeManager code_manager_;
   AccountingAllocator allocator_;
 
+  // Task manager managing all background compile jobs. Before shut down of the
+  // engine, they must all be finished because they access the allocator.
+  CancelableTaskManager background_compile_task_manager_;
+
   // This mutex protects all information which is mutated concurrently or
   // fields that are initialized lazily on the first access.
   base::Mutex mutex_;