Make wasm memory histograms simple histograms.

This Cl fixes a fundamental misunderstanding when Wasm memory
histograms were added. They were added using
HISTOGRAM_MEMORY_LIST(). This macro implements aggregating memory
histograms that handle cases memory cases that are not module
specific.

The fixed memory histograms are all module specific, and are simple
histograms.

In addition, it removes field is_sync from ModuleCompiler and
WasmCompilationUnit, since the field is no longer needed to make the
fixed memory histograms synchronous.

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

src/compiler/wasm-compiler.cc

@@ -3944,7 +3944,6 @@ WasmCompilationUnit::WasmCompilationUnit(Isolate* isolate,
       func_body_(body),
       func_name_(name),
       counters_(isolate->counters()),
-      is_sync_(true),
       centry_stub_(centry_stub),
       func_index_(index) {}
 
@@ -3970,7 +3969,6 @@ WasmCompilationUnit::WasmCompilationUnit(
       func_body_(body),
       func_name_(name),
       counters_(async_counters.get()),
-      is_sync_(false),
       centry_stub_(centry_stub),
       func_index_(index) {}
 
@@ -4035,11 +4033,8 @@ void WasmCompilationUnit::ExecuteCompilation() {
         &protected_instructions, module_env_->module->origin()));
     ok_ = job_->ExecuteJob() == CompilationJob::SUCCEEDED;
     // TODO(bradnelson): Improve histogram handling of size_t.
-    if (is_sync_)
-      // TODO(karlschimpf): Make this work when asynchronous.
-      // https://bugs.chromium.org/p/v8/issues/detail?id=6361
-      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

@@ -87,7 +87,6 @@ class WasmCompilationUnit final {
   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.
   JSGraph* jsgraph_ = nullptr;

src/counters.h

@@ -1006,7 +1006,25 @@ class RuntimeCallTimerScope {
      13)                                                                      \
   HR(array_buffer_new_size_failures, V8.ArrayBufferNewSizeFailures, 0, 4096,  \
      13)                                                                      \
-  HR(shared_array_allocations, V8.SharedArrayAllocationSizes, 0, 4096, 13)
+  HR(shared_array_allocations, V8.SharedArrayAllocationSizes, 0, 4096, 13)    \
+  HR(wasm_asm_function_size_bytes, V8.WasmFunctionSizeBytes.asm, 1, GB, 51)   \
+  HR(wasm_wasm_function_size_bytes, V8.WasmFunctionSizeBytes.wasm, 1, GB, 51) \
+  HR(wasm_asm_module_size_bytes, V8.WasmModuleSizeBytes.asm, 1, GB, 51)       \
+  HR(wasm_wasm_module_size_bytes, V8.WasmModuleSizeBytes.wasm, 1, GB, 51)     \
+  HR(wasm_asm_min_mem_pages_count, V8.WasmMinMemPagesCount.asm, 1, 2 << 16,   \
+     51)                                                                      \
+  HR(wasm_wasm_min_mem_pages_count, V8.WasmMinMemPagesCount.wasm, 1, 2 << 16, \
+     51)                                                                      \
+  HR(wasm_wasm_max_mem_pages_count, V8.WasmMaxMemPagesCount.wasm, 1, 2 << 16, \
+     51)                                                                      \
+  HR(wasm_decode_asm_module_peak_memory_bytes,                                \
+     V8.WasmDecodeModulePeakMemoryBytes.asm, 1, GB, 51)                       \
+  HR(wasm_decode_wasm_module_peak_memory_bytes,                               \
+     V8.WasmDecodeModulePeakMemoryBytes.wasm, 1, GB, 51)                      \
+  HR(asm_wasm_translation_peak_memory_bytes,                                  \
+     V8.AsmWasmTranslationPeakMemoryBytes, 1, GB, 51)                         \
+  HR(wasm_compile_function_peak_memory_bytes,                                 \
+     V8.WasmCompileFunctionPeakMemoryBytes, 1, GB, 51)
 
 #define HISTOGRAM_TIMER_LIST(HT)                                               \
   /* Garbage collection timers. */                                             \
@@ -1080,6 +1098,7 @@ class RuntimeCallTimerScope {
   HP(heap_fraction_map_space, V8.MemoryHeapFractionMapSpace)                   \
   HP(heap_fraction_lo_space, V8.MemoryHeapFractionLoSpace)
 
+// Note: These use Histogram with options (min=1000, max=500000, buckets=50).
 #define HISTOGRAM_LEGACY_MEMORY_LIST(HM)                                      \
   HM(heap_sample_total_committed, V8.MemoryHeapSampleTotalCommitted)          \
   HM(heap_sample_total_used, V8.MemoryHeapSampleTotalUsed)                    \
@@ -1087,25 +1106,11 @@ class RuntimeCallTimerScope {
   HM(heap_sample_code_space_committed, V8.MemoryHeapSampleCodeSpaceCommitted) \
   HM(heap_sample_maximum_committed, V8.MemoryHeapSampleMaximumCommitted)
 
-#define HISTOGRAM_MEMORY_LIST(HM)                                  \
-  HM(memory_heap_committed, V8.MemoryHeapCommitted)                \
-  HM(memory_heap_used, V8.MemoryHeapUsed)                          \
-  /* Asm/Wasm */                                                   \
-  HM(wasm_decode_asm_module_peak_memory_bytes,                     \
-     V8.WasmDecodeModulePeakMemoryBytes.asm)                       \
-  HM(wasm_decode_wasm_module_peak_memory_bytes,                    \
-     V8.WasmDecodeModulePeakMemoryBytes.wasm)                      \
-  HM(wasm_compile_function_peak_memory_bytes,                      \
-     V8.WasmCompileFunctionPeakMemoryBytes)                        \
-  HM(wasm_asm_min_mem_pages_count, V8.WasmMinMemPagesCount.asm)    \
-  HM(wasm_wasm_min_mem_pages_count, V8.WasmMinMemPagesCount.wasm)  \
-  HM(wasm_wasm_max_mem_pages_count, V8.WasmMaxMemPagesCount.wasm)  \
-  HM(wasm_asm_function_size_bytes, V8.WasmFunctionSizeBytes.asm)   \
-  HM(wasm_wasm_function_size_bytes, V8.WasmFunctionSizeBytes.wasm) \
-  HM(wasm_asm_module_size_bytes, V8.WasmModuleSizeBytes.asm)       \
-  HM(wasm_wasm_module_size_bytes, V8.WasmModuleSizeBytes.wasm)     \
-  HM(asm_wasm_translation_peak_memory_bytes,                       \
-     V8.AsmWasmTranslationPeakMemoryBytes)
+// Note: These define both Histogram and AggregatedMemoryHistogram<Histogram>
+// histograms with options (min=4000, max=2000000, buckets=100).
+#define HISTOGRAM_MEMORY_LIST(HM)                   \
+  HM(memory_heap_committed, V8.MemoryHeapCommitted) \
+  HM(memory_heap_used, V8.MemoryHeapUsed)
 
 // WARNING: STATS_COUNTER_LIST_* is a very large macro that is causing MSVC
 // Intellisense to crash.  It was broken into two macros (each of length 40

src/wasm/module-compiler.cc

@@ -83,11 +83,10 @@ size_t ModuleCompiler::CodeGenerationSchedule::GetRandomIndexInSchedule() {
 }
 
 ModuleCompiler::ModuleCompiler(Isolate* isolate,
-                               std::unique_ptr<WasmModule> module, bool is_sync)
+                               std::unique_ptr<WasmModule> module)
     : isolate_(isolate),
       module_(std::move(module)),
       async_counters_(isolate->async_counters()),
-      is_sync_(is_sync),
       executed_units_(
           isolate->random_number_generator(),
           (isolate->heap()->memory_allocator()->code_range()->valid()
@@ -347,16 +346,9 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObject(
     signature_tables->set(i, *temp_instance.signature_tables[i]);
   }
 
-  if (is_sync_) {
-    // TODO(karlschimpf): Make this work when asynchronous.
-    // https://bugs.chromium.org/p/v8/issues/detail?id=6361
-    TimedHistogramScope wasm_compile_module_time_scope(
-        module_->is_wasm() ? counters()->wasm_compile_wasm_module_time()
-                           : counters()->wasm_compile_asm_module_time());
-    return CompileToModuleObjectInternal(
-        thrower, wire_bytes, asm_js_script, asm_js_offset_table_bytes, factory,
-        &temp_instance, &function_tables, &signature_tables);
-  }
+  TimedHistogramScope wasm_compile_module_time_scope(
+      module_->is_wasm() ? counters()->wasm_compile_wasm_module_time()
+                         : counters()->wasm_compile_asm_module_time());
   return CompileToModuleObjectInternal(
       thrower, wire_bytes, asm_js_script, asm_js_offset_table_bytes, factory,
       &temp_instance, &function_tables, &signature_tables);
@@ -591,12 +583,9 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(
     temp_instance->function_code[i] = init_builtin;
   }
 
-  if (is_sync_)
-    // TODO(karlschimpf): Make this work when asynchronous.
-    // https://bugs.chromium.org/p/v8/issues/detail?id=6361
-    (module_->is_wasm() ? counters()->wasm_functions_per_wasm_module()
-                        : counters()->wasm_functions_per_asm_module())
-        ->AddSample(static_cast<int>(module_->functions.size()));
+  (module_->is_wasm() ? counters()->wasm_functions_per_wasm_module()
+                      : counters()->wasm_functions_per_asm_module())
+      ->AddSample(static_cast<int>(module_->functions.size()));
 
   if (!lazy_compile) {
     size_t funcs_to_compile =
@@ -2110,9 +2099,8 @@ class AsyncCompileJob::PrepareAndStartCompile : public CompileStep {
     // Transfer ownership of the {WasmModule} to the {ModuleCompiler}, but
     // keep a pointer.
     WasmModule* module = module_.get();
-    constexpr bool is_sync = true;
     job_->compiler_.reset(
-        new ModuleCompiler(job_->isolate_, std::move(module_), !is_sync));
+        new ModuleCompiler(job_->isolate_, std::move(module_)));
     job_->compiler_->EnableThrottling();
 
     // Reopen all handles which should survive in the DeferredHandleScope.

src/wasm/module-compiler.h

@@ -27,8 +27,7 @@ class ModuleCompiler {
   // In {CompileToModuleObject}, it will transfer ownership to the generated
   // {WasmModuleWrapper}. If this method is not called, ownership may be
   // reclaimed by explicitely releasing the {module_} field.
-  ModuleCompiler(Isolate* isolate, std::unique_ptr<WasmModule> module,
-                 bool is_sync);
+  ModuleCompiler(Isolate* isolate, std::unique_ptr<WasmModule> module);
 
   // The actual runnable task that performs compilations in the background.
   class CompilationTask : public CancelableTask {
@@ -52,20 +51,12 @@ class ModuleCompiler {
     void AddUnit(ModuleEnv* module_env, const WasmFunction* function,
                  uint32_t buffer_offset, Vector<const uint8_t> bytes,
                  WasmName name) {
-      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()));
-      }
+      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() {
@@ -177,7 +168,6 @@ class ModuleCompiler {
   Isolate* isolate_;
   std::unique_ptr<WasmModule> module_;
   const std::shared_ptr<Counters> async_counters_;
-  bool is_sync_;
   std::vector<std::unique_ptr<compiler::WasmCompilationUnit>>
       compilation_units_;
   base::Mutex compilation_units_mutex_;

src/wasm/module-decoder.cc

@@ -1336,9 +1336,9 @@ FunctionResult DecodeWasmFunction(Isolate* isolate, Zone* zone,
                                   bool is_sync) {
   size_t size = function_end - function_start;
   bool is_wasm = module_env->module_env.is_wasm();
-  auto size_counter = is_wasm ? counters->wasm_wasm_function_size_bytes()
-                              : counters->wasm_asm_function_size_bytes();
-  size_counter->AddSample(static_cast<int>(size));
+  auto size_histogram = is_wasm ? counters->wasm_wasm_function_size_bytes()
+                                : counters->wasm_asm_function_size_bytes();
+  size_histogram->AddSample(static_cast<int>(size));
   auto time_counter = is_wasm ? counters->wasm_decode_wasm_function_time()
                               : counters->wasm_decode_asm_function_time();
   TimedHistogramScope wasm_decode_function_time_scope(time_counter);

src/wasm/wasm-module.cc

@@ -760,8 +760,7 @@ MaybeHandle<WasmModuleObject> wasm::SyncCompileTranslatedAsmJs(
 
   // Transfer ownership to the {WasmModuleWrapper} generated in
   // {CompileToModuleObject}.
-  constexpr bool is_sync = true;
-  ModuleCompiler helper(isolate, std::move(result.val), is_sync);
+  ModuleCompiler helper(isolate, std::move(result.val));
   return helper.CompileToModuleObject(thrower, bytes, asm_js_script,
                                       asm_js_offset_table_bytes);
 }
@@ -783,8 +782,7 @@ MaybeHandle<WasmModuleObject> wasm::SyncCompile(Isolate* isolate,
 
   // Transfer ownership to the {WasmModuleWrapper} generated in
   // {CompileToModuleObject}.
-  constexpr bool is_sync = true;
-  ModuleCompiler helper(isolate, std::move(result.val), is_sync);
+  ModuleCompiler helper(isolate, std::move(result.val));
   return helper.CompileToModuleObject(thrower, bytes, Handle<Script>(),
                                       Vector<const byte>());
 }