[wasm][debug] Make recompilation isolate-independent

Passing an isolate to {RecompileNativeModule} feels wrong, since
compilation and the generated code are totally isolate-independent. In
fact, the isolate is only used for updating counters.
Instead of passing the counters instead, this CL just refactors the code
to support a nullptr for the counters everywhere (some code paths
already supported that). The few recompilation would not make a
significant difference in the histograms anyway, and even have the risk
of skewing the data.

Drive-by 1: Rename {TierUp} to {StartTierUp} and update comments.
Drive-by 2: Remove non-actionable TODO.

R=thibaudm@chromium.org

Bug: v8:10359
Change-Id: Ic027f939bbc55398b90784922130fe1fe5573b0c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2187638
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67708}

src/compiler/wasm-compiler.cc

@@ -6788,9 +6788,10 @@ wasm::WasmCompilationResult ExecuteTurbofanWasmCompilation(
       &info, wasm_engine, mcgraph, call_descriptor, source_positions,
       node_origins, func_body, env->module, func_index);
 
-  // TODO(bradnelson): Improve histogram handling of size_t.
+  if (counters) {
-  counters->wasm_compile_function_peak_memory_bytes()->AddSample(
+    counters->wasm_compile_function_peak_memory_bytes()->AddSample(
-      static_cast<int>(mcgraph->graph()->zone()->allocation_size()));
+        static_cast<int>(mcgraph->graph()->zone()->allocation_size()));
+  }
   auto result = info.ReleaseWasmCompilationResult();
   CHECK_NOT_NULL(result);  // Compilation expected to succeed.
   DCHECK_EQ(wasm::ExecutionTier::kTurbofan, result->result_tier);

src/runtime/runtime-test.cc

@@ -1383,7 +1383,7 @@ RUNTIME_FUNCTION(Runtime_WasmTierDownModule) {
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_HANDLE_CHECKED(WasmInstanceObject, instance, 0);
   auto* native_module = instance->module_object().native_module();
-  native_module->TierDown(isolate);
+  native_module->TierDown();
   CHECK(!native_module->compilation_state()->failed());
   return ReadOnlyRoots(isolate).undefined_value();
 }
@@ -1393,7 +1393,7 @@ RUNTIME_FUNCTION(Runtime_WasmTierUpModule) {
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_HANDLE_CHECKED(WasmInstanceObject, instance, 0);
   auto* native_module = instance->module_object().native_module();
-  native_module->TierUp(isolate);
+  native_module->StartTierUp();
   CHECK(!native_module->compilation_state()->failed());
   return ReadOnlyRoots(isolate).undefined_value();
 }

src/wasm/function-compiler.cc

@@ -130,7 +130,7 @@ WasmCompilationResult WasmCompilationUnit::ExecuteCompilation(
                                         counters, detected);
   }
 
-  if (result.succeeded()) {
+  if (result.succeeded() && counters) {
     counters->wasm_generated_code_size()->Increment(
         result.code_desc.instr_size);
     counters->wasm_reloc_size()->Increment(result.code_desc.reloc_size);
@@ -163,12 +163,16 @@ WasmCompilationResult WasmCompilationUnit::ExecuteFunctionCompilation(
   wasm::FunctionBody func_body{func->sig, func->code.offset(), code.begin(),
                                code.end()};
 
-  auto size_histogram = SELECT_WASM_COUNTER(counters, env->module->origin, wasm,
+  base::Optional<TimedHistogramScope> wasm_compile_function_time_scope;
-                                            function_size_bytes);
+  if (counters) {
-  size_histogram->AddSample(static_cast<int>(func_body.end - func_body.start));
+    auto size_histogram = SELECT_WASM_COUNTER(counters, env->module->origin,
-  auto timed_histogram = SELECT_WASM_COUNTER(counters, env->module->origin,
+                                              wasm, function_size_bytes);
-                                             wasm_compile, function_time);
+    size_histogram->AddSample(
-  TimedHistogramScope wasm_compile_function_time_scope(timed_histogram);
+        static_cast<int>(func_body.end - func_body.start));
+    auto timed_histogram = SELECT_WASM_COUNTER(counters, env->module->origin,
+                                               wasm_compile, function_time);
+    wasm_compile_function_time_scope.emplace(timed_histogram);
+  }
 
   if (FLAG_trace_wasm_compiler) {
     PrintF("Compiling wasm function %d with %s\n", func_index_,

src/wasm/module-compiler.cc

@@ -1466,7 +1466,7 @@ std::shared_ptr<NativeModule> CompileToNativeModule(
   return native_module;
 }
 
-void RecompileNativeModule(Isolate* isolate, NativeModule* native_module,
+void RecompileNativeModule(NativeModule* native_module,
                            TieringState tiering_state) {
   // Install a callback to notify us once background recompilation finished.
   auto recompilation_finished_semaphore = std::make_shared<base::Semaphore>(0);
@@ -1484,8 +1484,9 @@ void RecompileNativeModule(Isolate* isolate, NativeModule* native_module,
   // We only wait for tier down. Tier up can happen in the background.
   if (tiering_state == kTieredDown) {
     // The main thread contributes to the compilation.
+    constexpr Counters* kNoCounters = nullptr;
     while (ExecuteCompilationUnits(
-        compilation_state->background_compile_token(), isolate->counters(),
+        compilation_state->background_compile_token(), kNoCounters,
         kMainThreadTaskId, kBaselineOnly)) {
       // Continue executing compilation units.
     }

src/wasm/module-compiler.h

@@ -44,7 +44,7 @@ std::shared_ptr<NativeModule> CompileToNativeModule(
     std::shared_ptr<const WasmModule> module, const ModuleWireBytes& wire_bytes,
     Handle<FixedArray>* export_wrappers_out);
 
-void RecompileNativeModule(Isolate* isolate, NativeModule* native_module,
+void RecompileNativeModule(NativeModule* native_module,
                            TieringState new_tiering_state);
 
 V8_EXPORT_PRIVATE

src/wasm/wasm-code-manager.cc

@@ -1853,17 +1853,17 @@ bool NativeModule::IsTieredDown() {
   return tiering_state_ == kTieredDown;
 }
 
-void NativeModule::TierDown(Isolate* isolate) {
+void NativeModule::TierDown() {
   // Do not tier down asm.js.
   if (module()->origin != kWasmOrigin) return;
 
   // Set the module to tiered down state; return if it is already in that state.
   if (!SetTieredDown()) return;
 
-  RecompileNativeModule(isolate, this, kTieredDown);
+  RecompileNativeModule(this, kTieredDown);
 }
 
-void NativeModule::TierUp(Isolate* isolate) {
+void NativeModule::StartTierUp() {
   // Do not tier up asm.js.
   if (module()->origin != kWasmOrigin) return;
 
@@ -1873,7 +1873,7 @@ void NativeModule::TierUp(Isolate* isolate) {
     tiering_state_ = kTieredUp;
   }
 
-  RecompileNativeModule(isolate, this, kTieredUp);
+  RecompileNativeModule(this, kTieredUp);
 }
 
 void NativeModule::FreeCode(Vector<WasmCode* const> codes) {

src/wasm/wasm-code-manager.h

@@ -600,10 +600,13 @@ class V8_EXPORT_PRIVATE NativeModule final {
   bool SetTieredDown();
   bool IsTieredDown();
 
-  // Sets the flag, triggers recompilation of all methods to tier down or up,
+  // Set the flag to keep this module tiered down, trigger recompilation of all
-  // waits for that to complete.
+  // functions, and wait for recompilation to complete.
-  void TierDown(Isolate* isolate);
+  void TierDown();
-  void TierUp(Isolate* isolate);
+
+  // Clear the flag to keep this module tiered down and trigger recompilation
+  // of all functions. Does not wait for completion of recompilation.
+  void StartTierUp();
 
   // Free a set of functions of this module. Uncommits whole pages if possible.
   // The given vector must be ordered by the instruction start address, and all

src/wasm/wasm-engine.cc

@@ -626,7 +626,7 @@ void WasmEngine::TierDownAllModulesPerIsolate(Isolate* isolate) {
     }
   }
   for (auto* native_module : native_modules) {
-    native_module->TierDown(isolate);
+    native_module->TierDown();
   }
 }
 
@@ -640,7 +640,7 @@ void WasmEngine::TierUpAllModulesPerIsolate(Isolate* isolate) {
     }
   }
   for (auto* native_module : native_modules) {
-    native_module->TierUp(isolate);
+    native_module->StartTierUp();
   }
 }
 

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

@@ -221,7 +221,7 @@ class TestingModuleBuilder {
   void SetExecutable() { native_module_->SetExecutable(true); }
 
   void TierDown() {
-    native_module_->TierDown(isolate_);
+    native_module_->TierDown();
     execution_tier_ = ExecutionTier::kLiftoff;
   }