Fix remaining cases of HistogramTimer that may run in background.

That is, change to use TimedHistogram (which functions properly on
background threads).

Bug: v8:6361
Change-Id: I821fb0afea97be422786778d576683f67667c31b
Reviewed-on: https://chromium-review.googlesource.com/559769
Commit-Queue: Karl Schimpf <kschimpf@chromium.org>
Reviewed-by: Brad Nelson <bradnelson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46529}

src/compiler/wasm-compiler.cc

@@ -3924,7 +3924,7 @@ Vector<const char> GetDebugName(Zone* zone, wasm::WasmName name, int index) {
 WasmCompilationUnit::WasmCompilationUnit(Isolate* isolate,
                                          wasm::ModuleBytesEnv* module_env,
                                          const wasm::WasmFunction* function,
-                                         Handle<Code> centry_stub, bool is_sync)
+                                         Handle<Code> centry_stub)
     : WasmCompilationUnit(
           isolate, &module_env->module_env,
           wasm::FunctionBody{
@@ -3932,29 +3932,51 @@ WasmCompilationUnit::WasmCompilationUnit(Isolate* isolate,
               module_env->wire_bytes.start() + function->code.offset(),
               module_env->wire_bytes.start() + function->code.end_offset()},
           module_env->wire_bytes.GetNameOrNull(function), function->func_index,
-          centry_stub, is_sync) {}
+          centry_stub) {}
 
 WasmCompilationUnit::WasmCompilationUnit(Isolate* isolate,
                                          wasm::ModuleEnv* module_env,
                                          wasm::FunctionBody body,
                                          wasm::WasmName name, int index,
-                                         Handle<Code> centry_stub, bool is_sync)
+                                         Handle<Code> centry_stub)
     : isolate_(isolate),
       module_env_(module_env),
       func_body_(body),
       func_name_(name),
-      is_sync_(is_sync),
+      counters_(isolate->counters()),
+      is_sync_(true),
+      centry_stub_(centry_stub),
+      func_index_(index) {}
+
+WasmCompilationUnit::WasmCompilationUnit(
+    Isolate* isolate, wasm::ModuleBytesEnv* module_env,
+    const wasm::WasmFunction* function, Handle<Code> centry_stub,
+    const std::shared_ptr<Counters>& async_counters)
+    : WasmCompilationUnit(
+          isolate, &module_env->module_env,
+          wasm::FunctionBody{
+              function->sig, function->code.offset(),
+              module_env->wire_bytes.start() + function->code.offset(),
+              module_env->wire_bytes.start() + function->code.end_offset()},
+          module_env->wire_bytes.GetNameOrNull(function), function->func_index,
+          centry_stub, async_counters) {}
+
+WasmCompilationUnit::WasmCompilationUnit(
+    Isolate* isolate, wasm::ModuleEnv* module_env, wasm::FunctionBody body,
+    wasm::WasmName name, int index, Handle<Code> centry_stub,
+    const std::shared_ptr<Counters>& async_counters)
+    : isolate_(isolate),
+      module_env_(module_env),
+      func_body_(body),
+      func_name_(name),
+      counters_(async_counters.get()),
+      is_sync_(false),
       centry_stub_(centry_stub),
       func_index_(index) {}
 
 void WasmCompilationUnit::ExecuteCompilation() {
-  // TODO(karlschimpf): Make this work when asynchronous.
-  // https://bugs.chromium.org/p/v8/issues/detail?id=6361
-  base::Optional<HistogramTimerScope> wasm_compile_function_time_scope;
-  if (is_sync_) {
-    wasm_compile_function_time_scope.emplace(
-        isolate_->counters()->wasm_compile_function_time());
-  }
+  TimedHistogramScope wasm_compile_function_time_scope(
+      counters()->wasm_compile_function_time());
 
   if (FLAG_trace_wasm_compiler) {
     if (func_name_.start() != nullptr) {
@@ -4016,10 +4038,8 @@ void WasmCompilationUnit::ExecuteCompilation() {
     if (is_sync_)
       // TODO(karlschimpf): Make this work when asynchronous.
       // https://bugs.chromium.org/p/v8/issues/detail?id=6361
-      isolate_->counters()
-          ->wasm_compile_function_peak_memory_bytes()
-          ->AddSample(
-              static_cast<int>(jsgraph_->graph()->zone()->allocation_size()));
+      counters()->wasm_compile_function_peak_memory_bytes()->AddSample(
+          static_cast<int>(jsgraph_->graph()->zone()->allocation_size()));
 
     if (FLAG_trace_wasm_decode_time) {
       double pipeline_ms = pipeline_timer.Elapsed().InMillisecondsF();

src/compiler/wasm-compiler.h

@@ -47,12 +47,24 @@ typedef compiler::JSGraph TFGraph;
 namespace compiler {
 class WasmCompilationUnit final {
  public:
+  // Use the following constructors if you know you are running on the
+  // foreground thread.
   WasmCompilationUnit(Isolate* isolate, wasm::ModuleBytesEnv* module_env,
                       const wasm::WasmFunction* function,
-                      Handle<Code> centry_stub, bool is_sync = true);
+                      Handle<Code> centry_stub);
   WasmCompilationUnit(Isolate* isolate, wasm::ModuleEnv* module_env,
                       wasm::FunctionBody body, wasm::WasmName name, int index,
-                      Handle<Code> centry_stub, bool is_sync = true);
+                      Handle<Code> centry_stub);
+  // Use the following constructors if the compilation may run on a background
+  // thread.
+  WasmCompilationUnit(Isolate* isolate, wasm::ModuleBytesEnv* module_env,
+                      const wasm::WasmFunction* function,
+                      Handle<Code> centry_stub,
+                      const std::shared_ptr<Counters>& async_counters);
+  WasmCompilationUnit(Isolate* isolate, wasm::ModuleEnv* module_env,
+                      wasm::FunctionBody body, wasm::WasmName name, int index,
+                      Handle<Code> centry_stub,
+                      const std::shared_ptr<Counters>& async_counters);
 
   int func_index() const { return func_index_; }
 
@@ -74,6 +86,7 @@ class WasmCompilationUnit final {
   wasm::ModuleEnv* module_env_;
   wasm::FunctionBody func_body_;
   wasm::WasmName func_name_;
+  Counters* counters_;
   bool is_sync_;
   // The graph zone is deallocated at the end of ExecuteCompilation by virtue of
   // it being zone allocated.
@@ -90,6 +103,8 @@ class WasmCompilationUnit final {
   bool ok_ = true;
   size_t memory_cost_ = 0;
 
+  Counters* counters() { return counters_; }
+
   DISALLOW_COPY_AND_ASSIGN(WasmCompilationUnit);
 };
 

src/counters.h

@@ -1037,30 +1037,30 @@ class RuntimeCallTimerScope {
   /* Total JavaScript execution time (including callbacks and runtime calls */ \
   HT(execute, V8.Execute, 1000000, MICROSECOND)                                \
   /* Asm/Wasm */                                                               \
-  HT(wasm_instantiate_asm_module_time,                                         \
-     V8.WasmInstantiateModuleMicroSeconds.asm, 10000000, MICROSECOND)          \
-  HT(wasm_instantiate_wasm_module_time,                                        \
-     V8.WasmInstantiateModuleMicroSeconds.wasm, 10000000, MICROSECOND)         \
-  HT(wasm_compile_function_time, V8.WasmCompileFunctionMicroSeconds, 1000000,  \
-     MICROSECOND)                                                              \
   HT(asm_wasm_translation_time, V8.AsmWasmTranslationMicroSeconds, 1000000,    \
      MICROSECOND)                                                              \
   HT(wasm_lazy_compilation_time, V8.WasmLazyCompilationMicroSeconds, 1000000,  \
      MICROSECOND)
 
-#define TIMED_HISTOGRAM_LIST(HT)                                             \
-  HT(wasm_decode_asm_module_time, V8.WasmDecodeModuleMicroSeconds.asm,       \
-     1000000, MICROSECOND)                                                   \
-  HT(wasm_decode_wasm_module_time, V8.WasmDecodeModuleMicroSeconds.wasm,     \
-     1000000, MICROSECOND)                                                   \
-  HT(wasm_decode_asm_function_time, V8.WasmDecodeFunctionMicroSeconds.asm,   \
-     1000000, MICROSECOND)                                                   \
-  HT(wasm_decode_wasm_function_time, V8.WasmDecodeFunctionMicroSeconds.wasm, \
-     1000000, MICROSECOND)                                                   \
-  HT(wasm_compile_asm_module_time, V8.WasmCompileModuleMicroSeconds.asm,     \
-     10000000, MICROSECOND)                                                  \
-  HT(wasm_compile_wasm_module_time, V8.WasmCompileModuleMicroSeconds.wasm,   \
-     10000000, MICROSECOND)
+#define TIMED_HISTOGRAM_LIST(HT)                                              \
+  HT(wasm_decode_asm_module_time, V8.WasmDecodeModuleMicroSeconds.asm,        \
+     1000000, MICROSECOND)                                                    \
+  HT(wasm_decode_wasm_module_time, V8.WasmDecodeModuleMicroSeconds.wasm,      \
+     1000000, MICROSECOND)                                                    \
+  HT(wasm_decode_asm_function_time, V8.WasmDecodeFunctionMicroSeconds.asm,    \
+     1000000, MICROSECOND)                                                    \
+  HT(wasm_decode_wasm_function_time, V8.WasmDecodeFunctionMicroSeconds.wasm,  \
+     1000000, MICROSECOND)                                                    \
+  HT(wasm_compile_asm_module_time, V8.WasmCompileModuleMicroSeconds.asm,      \
+     10000000, MICROSECOND)                                                   \
+  HT(wasm_compile_wasm_module_time, V8.WasmCompileModuleMicroSeconds.wasm,    \
+     10000000, MICROSECOND)                                                   \
+  HT(wasm_compile_function_time, V8.WasmCompileFunctionMicroSeconds, 1000000, \
+     MICROSECOND)                                                             \
+  HT(wasm_instantiate_wasm_module_time,                                       \
+     V8.WasmInstantiateModuleMicroSeconds.wasm, 10000000, MICROSECOND)        \
+  HT(wasm_instantiate_asm_module_time,                                        \
+     V8.WasmInstantiateModuleMicroSeconds.asm, 10000000, MICROSECOND)
 
 #define AGGREGATABLE_HISTOGRAM_TIMER_LIST(AHT) \
   AHT(compile_lazy, V8.CompileLazyMicroSeconds)

src/wasm/module-compiler.cc

@@ -760,7 +760,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
   }
 
   // Record build time into correct bucket, then build instance.
-  HistogramTimerScope wasm_instantiate_module_time_scope(
+  TimedHistogramScope wasm_instantiate_module_time_scope(
       module_->is_wasm() ? counters()->wasm_instantiate_wasm_module_time()
                          : counters()->wasm_instantiate_asm_module_time());
   Factory* factory = isolate_->factory();

src/wasm/module-compiler.h

@@ -52,12 +52,20 @@ class ModuleCompiler {
     void AddUnit(ModuleEnv* module_env, const WasmFunction* function,
                  uint32_t buffer_offset, Vector<const uint8_t> bytes,
                  WasmName name) {
-      constexpr bool is_sync = true;
-      units_.emplace_back(new compiler::WasmCompilationUnit(
-          compiler_->isolate_, module_env,
-          wasm::FunctionBody{function->sig, buffer_offset, bytes.begin(),
-                             bytes.end()},
-          name, function->func_index, compiler_->centry_stub_, is_sync));
+      if (compiler_->is_sync_) {
+        units_.emplace_back(new compiler::WasmCompilationUnit(
+            compiler_->isolate_, module_env,
+            wasm::FunctionBody{function->sig, buffer_offset, bytes.begin(),
+                               bytes.end()},
+            name, function->func_index, compiler_->centry_stub_));
+      } else {
+        units_.emplace_back(new compiler::WasmCompilationUnit(
+            compiler_->isolate_, module_env,
+            wasm::FunctionBody{function->sig, buffer_offset, bytes.begin(),
+                               bytes.end()},
+            name, function->func_index, compiler_->centry_stub_,
+            compiler_->async_counters()));
+      }
     }
 
     void Commit() {