Reland "[wasm] Refactor callback triggering"

This is a reland of 0dc598df

Original change's description:
> [wasm] Refactor callback triggering
> 
> 1) Instead of passing three boolean values to |TriggerCallbacks|, pass
>    one EnumSet which contains all events to trigger.
> 2) Remember which events already happened, to avoid triggering them
>    again.
> 3) Compute triggered events once after the loop in |OnFinishedUnits|,
>    instead of checking for every finished unit.
> 4) When a new callback is registered, trigger all previous events
>    immediately. This solves issue v8:10217.
> 5) Replace |NotifyTopTierReady| by |AddCallback| which is identical now.
> 6) Do not call |OnFinishedJSToWasmWrapperUnits| if no wrappers were
>    compiled (this is a minor performance optimization; we save taking
>    and releasing a lock).
> 7) Drive-by: Make the |EnumSet| constructor really constexpr (by making
>    |Mask| constexpr).
> 
> R=ahaas@chromium.org
> 
> Bug: v8:10217
> Change-Id: Ib3688a1687ad7b523e90efd73f4073e9f1193016
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2072737
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#66459}

Bug: v8:10217
Change-Id: I9f7ca424fa7f1d0379dabe230bebf62522dfc857
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2074501Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66468}

src/base/enum-set.h

@@ -62,8 +62,10 @@ class EnumSet {
  private:
   explicit constexpr EnumSet(T bits) : bits_(bits) {}
 
-  static T Mask(E element) {
+  static constexpr T Mask(E element) {
+#if V8_HAS_CXX14_CONSTEXPR
     DCHECK_GT(sizeof(T) * 8, static_cast<int>(element));
+#endif
     return T{1} << static_cast<typename std::underlying_type<E>::type>(element);
   }
 

src/wasm/compilation-environment.h

@@ -119,7 +119,6 @@ class CompilationState {
       const;
 
   void AddCallback(callback_t);
-  void NotifyTopTierReady(callback_t);
 
   bool failed() const;
   V8_EXPORT_PRIVATE bool baseline_compilation_finished() const;

src/wasm/module-compiler.cc

@@ -394,10 +394,6 @@ class CompilationStateImpl {
   // events. The callback object must support being deleted from any thread.
   void AddCallback(CompilationState::callback_t);
 
-  // If the module is already tiered up, trigger the callback immediately with a
-  // {kFinishedTopTierCompilation} event. Otherwise, behave as {AddCallback}.
-  void NotifyTopTierReady(CompilationState::callback_t);
-
   // Inserts new functions to compile and kicks off compilation.
   void AddCompilationUnits(
       Vector<WasmCompilationUnit> baseline_units,
@@ -415,9 +411,6 @@ class CompilationStateImpl {
 
   void OnFinishedUnits(Vector<WasmCode*>, Vector<WasmCompilationResult>);
   void OnFinishedJSToWasmWrapperUnits(int num);
-  void TriggerCallbacks(bool completes_baseline_compilation,
-                        bool completes_top_tier_compilation,
-                        bool completes_recompilation = false);
 
   void OnBackgroundTaskStopped(int task_id, const WasmFeatures& detected);
   void UpdateDetectedFeatures(const WasmFeatures& detected);
@@ -490,6 +483,11 @@ class CompilationStateImpl {
   }
 
  private:
+  // Trigger callbacks according to the internal counters below
+  // (outstanding_...), plus the given events.
+  // Hold the {callbacks_mutex_} when calling this method.
+  void TriggerCallbacks(base::EnumSet<CompilationEvent> additional_events = {});
+
   NativeModule* const native_module_;
   const std::shared_ptr<BackgroundCompileToken> background_compile_token_;
   const CompileMode compile_mode_;
@@ -550,6 +548,9 @@ class CompilationStateImpl {
   // Callback functions to be called on compilation events.
   std::vector<CompilationState::callback_t> callbacks_;
 
+  // Events that already happened.
+  base::EnumSet<CompilationEvent> finished_events_;
+
   int outstanding_baseline_units_ = 0;
   int outstanding_top_tier_functions_ = 0;
   std::vector<uint8_t> compilation_progress_;
@@ -607,11 +608,6 @@ void CompilationState::AddCallback(CompilationState::callback_t callback) {
   return Impl(this)->AddCallback(std::move(callback));
 }
 
-void CompilationState::NotifyTopTierReady(
-    CompilationState::callback_t callback) {
-  return Impl(this)->NotifyTopTierReady(std::move(callback));
-}
-
 bool CompilationState::failed() const { return Impl(this)->failed(); }
 
 bool CompilationState::baseline_compilation_finished() const {
@@ -985,7 +981,7 @@ bool ExecuteJSToWasmWrapperCompilationUnits(
       ++num_processed_wrappers;
     }
   } while (wrapper_unit);
-  {
+  if (num_processed_wrappers > 0) {
     BackgroundCompileScope compile_scope(token);
     if (compile_scope.cancelled()) return false;
     compile_scope.compilation_state()->OnFinishedJSToWasmWrapperUnits(
@@ -2054,10 +2050,7 @@ class SampleTopTierCodeSizeCallback {
       : native_module_(std::move(native_module)) {}
 
   void operator()(CompilationEvent event) {
-    // This callback is registered after baseline compilation finished, so the
-    // only possible event to follow is {kFinishedTopTierCompilation}.
-    if (event == CompilationEvent::kFinishedRecompilation) return;
-    DCHECK_EQ(CompilationEvent::kFinishedTopTierCompilation, event);
+    if (event != CompilationEvent::kFinishedTopTierCompilation) return;
     if (std::shared_ptr<NativeModule> native_module = native_module_.lock()) {
       native_module->engine()->SampleTopTierCodeSizeInAllIsolates(
           native_module);
@@ -2582,17 +2575,20 @@ void CompilationStateImpl::InitializeRecompilation(
 
 void CompilationStateImpl::AddCallback(CompilationState::callback_t callback) {
   base::MutexGuard callbacks_guard(&callbacks_mutex_);
-  callbacks_.emplace_back(std::move(callback));
-}
-
-void CompilationStateImpl::NotifyTopTierReady(
-    CompilationState::callback_t callback) {
-  base::MutexGuard callbacks_guard(&callbacks_mutex_);
-  if (!compilation_progress_.empty() && outstanding_top_tier_functions_ == 0) {
-    callback(CompilationEvent::kFinishedTopTierCompilation);
-    return;
+  // Immediately trigger events that already happened.
+  for (auto event : {CompilationEvent::kFinishedBaselineCompilation,
+                     CompilationEvent::kFinishedTopTierCompilation,
+                     CompilationEvent::kFailedCompilation}) {
+    if (finished_events_.contains(event)) {
+      callback(event);
+    }
+  }
+  constexpr base::EnumSet<CompilationEvent> kFinalEvents{
+      CompilationEvent::kFinishedTopTierCompilation,
+      CompilationEvent::kFailedCompilation};
+  if (!finished_events_.contains_any(kFinalEvents)) {
+    callbacks_.emplace_back(std::move(callback));
   }
-  callbacks_.emplace_back(std::move(callback));
 }
 
 void CompilationStateImpl::AddCompilationUnits(
@@ -2677,9 +2673,7 @@ void CompilationStateImpl::OnFinishedUnits(
   DCHECK_EQ(compilation_progress_.size(),
             native_module_->module()->num_declared_functions);
 
-  bool completes_baseline_compilation = false;
-  bool completes_top_tier_compilation = false;
-  bool completes_recompilation = false;
+  base::EnumSet<CompilationEvent> triggered_events;
 
   for (size_t i = 0; i < code_vector.size(); i++) {
     WasmCode* code = code_vector[i];
@@ -2690,9 +2684,6 @@ void CompilationStateImpl::OnFinishedUnits(
       // Import wrapper.
       DCHECK_EQ(code->tier(), ExecutionTier::kTurbofan);
       outstanding_baseline_units_--;
-      if (outstanding_baseline_units_ == 0) {
-        completes_baseline_compilation = true;
-      }
     } else {
       // Function.
       DCHECK_NE(code->tier(), ExecutionTier::kNone);
@@ -2715,17 +2706,11 @@ void CompilationStateImpl::OnFinishedUnits(
           required_baseline_tier <= code->tier()) {
         DCHECK_GT(outstanding_baseline_units_, 0);
         outstanding_baseline_units_--;
-        if (outstanding_baseline_units_ == 0) {
-          completes_baseline_compilation = true;
-        }
       }
       if (reached_tier < required_top_tier &&
           required_top_tier <= code->tier()) {
         DCHECK_GT(outstanding_top_tier_functions_, 0);
         outstanding_top_tier_functions_--;
-        if (outstanding_top_tier_functions_ == 0) {
-          completes_top_tier_compilation = true;
-        }
       }
 
       // If there is recompilation in progress, we would only count the
@@ -2744,7 +2729,7 @@ void CompilationStateImpl::OnFinishedUnits(
               ReachedRecompilationTierField::update(
                   compilation_progress_[slot_index], code->tier());
           if (outstanding_recompilation_functions_ == 0) {
-            completes_recompilation = true;
+            triggered_events.Add(CompilationEvent::kFinishedRecompilation);
           }
         }
       }
@@ -2758,43 +2743,50 @@ void CompilationStateImpl::OnFinishedUnits(
     }
   }
 
-  TriggerCallbacks(completes_baseline_compilation,
-                   completes_top_tier_compilation, completes_recompilation);
+  TriggerCallbacks(triggered_events);
 }
 
 void CompilationStateImpl::OnFinishedJSToWasmWrapperUnits(int num) {
   if (num == 0) return;
   base::MutexGuard guard(&callbacks_mutex_);
+  DCHECK_GE(outstanding_baseline_units_, num);
   outstanding_baseline_units_ -= num;
-  bool completes_baseline_compilation = outstanding_baseline_units_ == 0;
-  TriggerCallbacks(completes_baseline_compilation, false);
+  TriggerCallbacks();
 }
 
-void CompilationStateImpl::TriggerCallbacks(bool completes_baseline_compilation,
-                                            bool completes_top_tier_compilation,
-                                            bool completes_recompilation) {
+void CompilationStateImpl::TriggerCallbacks(
+    base::EnumSet<CompilationEvent> triggered_events) {
   DCHECK(!callbacks_mutex_.TryLock());
-  if (completes_recompilation) {
-    DCHECK(!callbacks_.empty());
-    for (auto& callback : callbacks_) {
-      callback(CompilationEvent::kFinishedRecompilation);
-    }
-  }
-  if (completes_baseline_compilation) {
-    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), "BaselineFinished");
-    for (auto& callback : callbacks_) {
-      callback(CompilationEvent::kFinishedBaselineCompilation);
-    }
+
+  if (outstanding_baseline_units_ == 0) {
+    triggered_events.Add(CompilationEvent::kFinishedBaselineCompilation);
     if (outstanding_top_tier_functions_ == 0) {
-      completes_top_tier_compilation = true;
+      triggered_events.Add(CompilationEvent::kFinishedTopTierCompilation);
     }
   }
-  if (outstanding_baseline_units_ == 0 && completes_top_tier_compilation) {
-    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), "TopTierFinished");
+
+  if (triggered_events.empty()) return;
+
+  // Don't trigger past events again.
+  triggered_events -= finished_events_;
+  // Recompilation can happen multiple times, thus do not store this.
+  finished_events_ |=
+      triggered_events - CompilationEvent::kFinishedRecompilation;
+
+  for (auto event :
+       {std::make_pair(CompilationEvent::kFinishedBaselineCompilation,
+                       "BaselineFinished"),
+        std::make_pair(CompilationEvent::kFinishedTopTierCompilation,
+                       "TopTierFinished"),
+        std::make_pair(CompilationEvent::kFinishedRecompilation,
+                       "RecompilationFinished")}) {
+    if (!triggered_events.contains(event.first)) continue;
+    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), event.second);
     for (auto& callback : callbacks_) {
-      callback(CompilationEvent::kFinishedTopTierCompilation);
+      callback(event.first);
     }
   }
+
   if (outstanding_baseline_units_ == 0 &&
       outstanding_top_tier_functions_ == 0 &&
       outstanding_recompilation_functions_ == 0) {

src/wasm/streaming-decoder.cc

@@ -154,7 +154,7 @@ void StreamingDecoder::NotifyNativeModuleCreated(
     const std::shared_ptr<NativeModule>& native_module) {
   if (!module_compiled_callback_) return;
   auto* comp_state = native_module->compilation_state();
-  comp_state->NotifyTopTierReady(TopTierCompiledCallback{
+  comp_state->AddCallback(TopTierCompiledCallback{
       std::move(native_module), std::move(module_compiled_callback_)});
   module_compiled_callback_ = {};
 }