Only turn on UMA WASM metric when synchronous.

The code for UMA stats (in counters.h) is not thread safe, and can
lead to using pointers with uninitialized values.

Therefore, this CL turns them off when compiling asynchronously.

It also turns back on several UMA stats that were previously turned
off, but no longer need to because the code now knows if it is
running synchronously.

BUG=v8:6361

Review-Url: https://codereview.chromium.org/2864583004
Cr-Commit-Position: refs/heads/master@{#45168}

src/compiler/wasm-compiler.cc

@@ -3937,24 +3937,27 @@ Vector<const char> GetDebugName(Zone* zone, wasm::WasmName name, int index) {
 
 WasmCompilationUnit::WasmCompilationUnit(Isolate* isolate,
                                          wasm::ModuleBytesEnv* module_env,
-                                         const wasm::WasmFunction* function)
+                                         const wasm::WasmFunction* function,
+                                         bool is_sync)
     : WasmCompilationUnit(
           isolate, &module_env->module_env,
           wasm::FunctionBody{
               function->sig, module_env->wire_bytes.start(),
               module_env->wire_bytes.start() + function->code_start_offset,
               module_env->wire_bytes.start() + function->code_end_offset},
-          module_env->wire_bytes.GetNameOrNull(function),
-          function->func_index) {}
+          module_env->wire_bytes.GetNameOrNull(function), function->func_index,
+          is_sync) {}
 
 WasmCompilationUnit::WasmCompilationUnit(Isolate* isolate,
                                          wasm::ModuleEnv* module_env,
                                          wasm::FunctionBody body,
-                                         wasm::WasmName name, int index)
+                                         wasm::WasmName name, int index,
+                                         bool is_sync)
     : isolate_(isolate),
       module_env_(module_env),
       func_body_(body),
       func_name_(name),
+      is_sync_(is_sync),
       graph_zone_(new Zone(isolate->allocator(), ZONE_NAME)),
       jsgraph_(new (graph_zone()) JSGraph(
           isolate, new (graph_zone()) Graph(graph_zone()),
@@ -3982,9 +3985,18 @@ void WasmCompilationUnit::InitializeHandles() {
 
 void WasmCompilationUnit::ExecuteCompilation() {
   DCHECK(handles_initialized_);
-  // TODO(ahaas): The counters are not thread-safe at the moment.
-  //    HistogramTimerScope wasm_compile_function_time_scope(
-  //        isolate_->counters()->wasm_compile_function_time());
+  if (is_sync_) {
+    // TODO(karlschimpf): Make this work when asynchronous.
+    // https://bugs.chromium.org/p/v8/issues/detail?id=6361
+    HistogramTimerScope wasm_compile_function_time_scope(
+        isolate_->counters()->wasm_compile_function_time());
+    ExecuteCompilationInternal();
+    return;
+  }
+  ExecuteCompilationInternal();
+}
+
+void WasmCompilationUnit::ExecuteCompilationInternal() {
   if (FLAG_trace_wasm_compiler) {
     if (func_name_.start() != nullptr) {
       PrintF("Compiling WASM function %d:'%.*s'\n\n", func_index(),
@@ -4023,10 +4035,11 @@ void WasmCompilationUnit::ExecuteCompilation() {
       !module_env_->module->is_wasm()));
   ok_ = job_->ExecuteJob() == CompilationJob::SUCCEEDED;
   // TODO(bradnelson): Improve histogram handling of size_t.
-  // TODO(ahaas): The counters are not thread-safe at the moment.
-  //    isolate_->counters()->wasm_compile_function_peak_memory_bytes()
-  // ->AddSample(
-  //        static_cast<int>(jsgraph->graph()->zone()->allocation_size()));
+  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()));
 
   if (FLAG_trace_wasm_decode_time) {
     double pipeline_ms = pipeline_timer.Elapsed().InMillisecondsF();

src/compiler/wasm-compiler.h

@@ -48,9 +48,10 @@ namespace compiler {
 class WasmCompilationUnit final {
  public:
   WasmCompilationUnit(Isolate* isolate, wasm::ModuleBytesEnv* module_env,
-                      const wasm::WasmFunction* function);
+                      const wasm::WasmFunction* function, bool is_sync = true);
   WasmCompilationUnit(Isolate* isolate, wasm::ModuleEnv* module_env,
-                      wasm::FunctionBody body, wasm::WasmName name, int index);
+                      wasm::FunctionBody body, wasm::WasmName name, int index,
+                      bool is_sync = true);
 
   Zone* graph_zone() { return graph_zone_.get(); }
   int func_index() const { return func_index_; }
@@ -71,6 +72,7 @@ class WasmCompilationUnit final {
   wasm::ModuleEnv* module_env_;
   wasm::FunctionBody func_body_;
   wasm::WasmName func_name_;
+  bool is_sync_;
   // The graph zone is deallocated at the end of ExecuteCompilation.
   std::unique_ptr<Zone> graph_zone_;
   JSGraph* jsgraph_;
@@ -87,6 +89,8 @@ class WasmCompilationUnit final {
       protected_instructions_;  // Instructions that are protected by the signal
                                 // handler.
 
+  void ExecuteCompilationInternal();
+
   DISALLOW_COPY_AND_ASSIGN(WasmCompilationUnit);
 };
 

src/wasm/module-decoder.cc

@@ -1137,22 +1137,22 @@ class ModuleDecoder : public Decoder {
   }
 };
 
-}  // namespace
-
-ModuleResult DecodeWasmModule(Isolate* isolate, const byte* module_start,
-                              const byte* module_end, bool verify_functions,
-                              ModuleOrigin origin) {
-  HistogramTimerScope wasm_decode_module_time_scope(
-      IsWasm(origin) ? isolate->counters()->wasm_decode_wasm_module_time()
-                     : isolate->counters()->wasm_decode_asm_module_time());
+ModuleResult DecodeWasmModuleInternal(Isolate* isolate,
+                                      const byte* module_start,
+                                      const byte* module_end,
+                                      bool verify_functions,
+                                      ModuleOrigin origin, bool is_sync) {
   size_t size = module_end - module_start;
   if (module_start > module_end) return ModuleResult::Error("start > end");
   if (size >= kV8MaxWasmModuleSize)
     return ModuleResult::Error("size > maximum module size: %zu", size);
   // TODO(bradnelson): Improve histogram handling of size_t.
-  (IsWasm(origin) ? isolate->counters()->wasm_wasm_module_size_bytes()
-                  : isolate->counters()->wasm_asm_module_size_bytes())
-      ->AddSample(static_cast<int>(size));
+  if (is_sync)
+    // TODO(karlschimpf): Make this work when asynchronous.
+    // https://bugs.chromium.org/p/v8/issues/detail?id=6361
+    (IsWasm(origin) ? isolate->counters()->wasm_wasm_module_size_bytes()
+                    : isolate->counters()->wasm_asm_module_size_bytes())
+        ->AddSample(static_cast<int>(size));
   // Signatures are stored in zone memory, which have the same lifetime
   // as the {module}.
   Zone* zone = new Zone(isolate->allocator(), ZONE_NAME);
@@ -1162,13 +1162,34 @@ ModuleResult DecodeWasmModule(Isolate* isolate, const byte* module_start,
   // TODO(titzer): this isn't accurate, since it doesn't count the data
   // allocated on the C++ heap.
   // https://bugs.chromium.org/p/chromium/issues/detail?id=657320
-  (IsWasm(origin)
-       ? isolate->counters()->wasm_decode_wasm_module_peak_memory_bytes()
-       : isolate->counters()->wasm_decode_asm_module_peak_memory_bytes())
-      ->AddSample(static_cast<int>(zone->allocation_size()));
+  if (is_sync)
+    // TODO(karlschimpf): Make this work when asynchronous.
+    // https://bugs.chromium.org/p/v8/issues/detail?id=6361
+    (IsWasm(origin)
+         ? isolate->counters()->wasm_decode_wasm_module_peak_memory_bytes()
+         : isolate->counters()->wasm_decode_asm_module_peak_memory_bytes())
+        ->AddSample(static_cast<int>(zone->allocation_size()));
   return result;
 }
 
+}  // namespace
+
+ModuleResult DecodeWasmModule(Isolate* isolate, const byte* module_start,
+                              const byte* module_end, bool verify_functions,
+                              ModuleOrigin origin, bool is_sync) {
+  if (is_sync) {
+    // TODO(karlschimpf): Make this work when asynchronous.
+    // https://bugs.chromium.org/p/v8/issues/detail?id=6361
+    HistogramTimerScope wasm_decode_module_time_scope(
+        IsWasm(origin) ? isolate->counters()->wasm_decode_wasm_module_time()
+                       : isolate->counters()->wasm_decode_asm_module_time());
+    return DecodeWasmModuleInternal(isolate, module_start, module_end,
+                                    verify_functions, origin, true);
+  }
+  return DecodeWasmModuleInternal(isolate, module_start, module_end,
+                                  verify_functions, origin, false);
+}
+
 FunctionSig* DecodeWasmSignatureForTesting(Zone* zone, const byte* start,
                                            const byte* end) {
   ModuleDecoder decoder(zone, start, end, kWasmOrigin);
@@ -1182,27 +1203,55 @@ WasmInitExpr DecodeWasmInitExprForTesting(const byte* start, const byte* end) {
   return decoder.DecodeInitExpr(start);
 }
 
-FunctionResult DecodeWasmFunction(Isolate* isolate, Zone* zone,
-                                  ModuleBytesEnv* module_env,
-                                  const byte* function_start,
-                                  const byte* function_end) {
-  bool is_wasm = module_env->module_env.is_wasm();
-  HistogramTimerScope wasm_decode_function_time_scope(
-      is_wasm ? isolate->counters()->wasm_decode_wasm_function_time()
-              : isolate->counters()->wasm_decode_asm_function_time());
+namespace {
+
+FunctionResult DecodeWasmFunctionInternal(Isolate* isolate, Zone* zone,
+                                          ModuleBytesEnv* module_env,
+                                          const byte* function_start,
+                                          const byte* function_end,
+                                          bool is_sync) {
   size_t size = function_end - function_start;
   if (function_start > function_end)
     return FunctionResult::Error("start > end");
   if (size > kV8MaxWasmFunctionSize)
     return FunctionResult::Error("size > maximum function size: %zu", size);
-  (is_wasm ? isolate->counters()->wasm_wasm_function_size_bytes()
-           : isolate->counters()->wasm_asm_function_size_bytes())
-      ->AddSample(static_cast<int>(size));
+  if (is_sync) {
+    // TODO(karlschimpf): Make this work when asynchronous.
+    // https://bugs.chromium.org/p/v8/issues/detail?id=6361
+    bool is_wasm = module_env->module_env.is_wasm();
+    (is_wasm ? isolate->counters()->wasm_wasm_function_size_bytes()
+             : isolate->counters()->wasm_asm_function_size_bytes())
+        ->AddSample(static_cast<int>(size));
+  }
   ModuleDecoder decoder(zone, function_start, function_end, kWasmOrigin);
   return decoder.DecodeSingleFunction(
       module_env, std::unique_ptr<WasmFunction>(new WasmFunction()));
 }
 
+}  // namespace
+
+FunctionResult DecodeWasmFunction(Isolate* isolate, Zone* zone,
+                                  ModuleBytesEnv* module_env,
+                                  const byte* function_start,
+                                  const byte* function_end, bool is_sync) {
+  if (is_sync) {
+    // TODO(karlschimpf): Make this work when asynchronous.
+    // https://bugs.chromium.org/p/v8/issues/detail?id=6361
+    size_t size = function_end - function_start;
+    bool is_wasm = module_env->module_env.is_wasm();
+    (is_wasm ? isolate->counters()->wasm_wasm_function_size_bytes()
+             : isolate->counters()->wasm_asm_function_size_bytes())
+        ->AddSample(static_cast<int>(size));
+    HistogramTimerScope wasm_decode_function_time_scope(
+        is_wasm ? isolate->counters()->wasm_decode_wasm_function_time()
+                : isolate->counters()->wasm_decode_asm_function_time());
+    return DecodeWasmFunctionInternal(isolate, zone, module_env, function_start,
+                                      function_end, true);
+  }
+  return DecodeWasmFunctionInternal(isolate, zone, module_env, function_start,
+                                    function_end, false);
+}
+
 AsmJsOffsetsResult DecodeAsmJsOffsets(const byte* tables_start,
                                       const byte* tables_end) {
   AsmJsOffsets table;

src/wasm/module-decoder.h

@@ -57,11 +57,9 @@ typedef std::vector<std::vector<AsmJsOffsetEntry>> AsmJsOffsets;
 typedef Result<AsmJsOffsets> AsmJsOffsetsResult;
 
 // Decodes the bytes of a WASM module between {module_start} and {module_end}.
-V8_EXPORT_PRIVATE ModuleResult DecodeWasmModule(Isolate* isolate,
-                                                const byte* module_start,
-                                                const byte* module_end,
-                                                bool verify_functions,
-                                                ModuleOrigin origin);
+V8_EXPORT_PRIVATE ModuleResult DecodeWasmModule(
+    Isolate* isolate, const byte* module_start, const byte* module_end,
+    bool verify_functions, ModuleOrigin origin, bool is_sync = true);
 
 // Exposed for testing. Decodes a single function signature, allocating it
 // in the given zone. Returns {nullptr} upon failure.
@@ -71,11 +69,9 @@ V8_EXPORT_PRIVATE FunctionSig* DecodeWasmSignatureForTesting(Zone* zone,
 
 // Decodes the bytes of a WASM function between
 // {function_start} and {function_end}.
-V8_EXPORT_PRIVATE FunctionResult DecodeWasmFunction(Isolate* isolate,
-                                                    Zone* zone,
-                                                    ModuleBytesEnv* env,
-                                                    const byte* function_start,
-                                                    const byte* function_end);
+V8_EXPORT_PRIVATE FunctionResult DecodeWasmFunction(
+    Isolate* isolate, Zone* zone, ModuleBytesEnv* env,
+    const byte* function_start, const byte* function_end, bool is_sync = true);
 
 V8_EXPORT_PRIVATE WasmInitExpr DecodeWasmInitExprForTesting(const byte* start,
                                                             const byte* end);