Fix use of history timers in background threads.

src/counters.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8_COUNTERS_H_
 #define V8_COUNTERS_H_
 
 #include "include/v8.h"
 #include "src/allocation.h"
 #include "src/base/atomic-utils.h"
@@ skipping 230 matching lines @@
   void* CreateHistogram() const;
 
   const char* name_;
   int min_;
   int max_;
   int num_buckets_;
   void* histogram_;
   Counters* counters_;
 };
 
-// A HistogramTimer allows distributions of results to be created.
-class HistogramTimer : public Histogram {
+enum class HistogramTimerResolution { MILLISECOND, MICROSECOND };
+
+// A thread safe histogram timer, allowing distributions of
+// (non-nested) timed results.

[Mircea Trofin, 2017/06/22 18:42:24]

why is non-nested in brackets?

[kschimpf, 2017/06/22 20:38:31]

Removing parenthesis and fixing to say nested. This class is intended to be the
timed histogram of choice.

+class TimedHistogram : public Histogram {
  public:
-  enum Resolution {
-    MILLISECOND,
-    MICROSECOND
-  };
+  // Start the timer. Log if isolate non-null.
+  void Start(base::ElapsedTimer* timer, Isolate* isolate);
 
-  // Note: public for testing purposes only.
-  HistogramTimer(const char* name, int min, int max, Resolution resolution,
-                 int num_buckets, Counters* counters)
+  // Stop the timer and record the results. Log if isolate non-null.
+  void Stop(base::ElapsedTimer* timer, Isolate* isolate);
+
+ protected:
+  friend class Counters;
+  HistogramTimerResolution resolution_;
+
+  TimedHistogram() {}
+  TimedHistogram(const char* name, int min, int max,
+                 HistogramTimerResolution resolution, int num_buckets,
+                 Counters* counters)
       : Histogram(name, min, max, num_buckets, counters),
         resolution_(resolution) {}
+  void AddTimeSample();
+};
 
-  // Start the timer.
-  void Start();
+// Helper class for scoping a TimedHistogram.
+class TimedHistogramScope {
+ public:
+  explicit TimedHistogramScope(TimedHistogram* histogram,
+                               Isolate* isolate = nullptr)
+      : histogram_(histogram), isolate_(isolate) {
+    histogram_->Start(&timer_, isolate);
+  }
+  ~TimedHistogramScope() { histogram_->Stop(&timer_, isolate_); }
 
-  // Stop the timer and record the results.
-  void Stop();
+ private:
+  base::ElapsedTimer timer_;
+  TimedHistogram* histogram_;
+  Isolate* isolate_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(TimedHistogramScope);
+};
+
+// A HistogramTimer allows distributions of (non-nested) timed results

[Mircea Trofin, 2017/06/22 18:42:23]

non-nested in brackets

[kschimpf, 2017/06/22 20:38:31]

Done.

+// to be created. WARNING: This class is not thread safe and can only
+// be run on the foreground thread.
+class HistogramTimer : public TimedHistogram {

[Mircea Trofin, 2017/06/22 18:42:23]

Is the difference between the two their thread safety? Should the names capture
that, otherwise it's a bit tricky to tell them apart.

Also, is the inheritance relationship necessary?

[kschimpf, 2017/06/22 20:38:31]

The base class is the more general solution, as it allows both thread safety and
nesting.

There are a lot of current user of a HistogramTimer, and would like to remove
this in a separate CL.

+ public:
+  // Note: public for testing purposes only.
+  HistogramTimer(const char* name, int min, int max,
+                 HistogramTimerResolution resolution, int num_buckets,
+                 Counters* counters)
+      : TimedHistogram(name, min, max, resolution, num_buckets, counters) {}
+
+  inline void Start();
+  inline void Stop();
 
   // Returns true if the timer is running.
   bool Running() {
     return Enabled() && timer_.IsStarted();
   }
 
   // TODO(bmeurer): Remove this when HistogramTimerScope is fixed.
 #ifdef DEBUG
   base::ElapsedTimer* timer() { return &timer_; }
 #endif
 
  private:
   friend class Counters;
 
   base::ElapsedTimer timer_;
-  Resolution resolution_;
 
   HistogramTimer() {}
 };
 
 // Helper class for scoping a HistogramTimer.
 // TODO(bmeurer): The ifdeffery is an ugly hack around the fact that the
 // Parser is currently reentrant (when it throws an error, we call back
 // into JavaScript and all bets are off), but ElapsedTimer is not
 // reentry-safe. Fix this properly and remove |allow_nesting|.
 class HistogramTimerScope BASE_EMBEDDED {
@@ skipping 24 matching lines @@
 #endif
   }
 
  private:
   HistogramTimer* timer_;
 #ifdef DEBUG
   bool skipped_timer_start_;
 #endif
 };
 
-
 // A histogram timer that can aggregate events within a larger scope.
 //
 // Intended use of this timer is to have an outer (aggregating) and an inner
 // (to be aggregated) scope, where the inner scope measure the time of events,
 // and all those inner scope measurements will be summed up by the outer scope.
 // An example use might be to aggregate the time spent in lazy compilation
 // while running a script.
 //
 // Helpers:
 // - AggregatingHistogramTimerScope, the "outer" scope within which
@@ skipping 659 matching lines @@
      MICROSECOND)                                                              \
   /* Total compilation time incl. caching/parsing */                           \
   HT(compile_script, V8.CompileScriptMicroSeconds, 1000000, MICROSECOND)       \
   /* 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_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(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 AGGREGATABLE_HISTOGRAM_TIMER_LIST(AHT) \
   AHT(compile_lazy, V8.CompileLazyMicroSeconds)
 
 #define HISTOGRAM_PERCENTAGE_LIST(HP)                                          \
   /* Heap fragmentation. */                                                    \
   HP(external_fragmentation_total, V8.MemoryExternalFragmentationTotal)        \
   HP(external_fragmentation_old_space, V8.MemoryExternalFragmentationOldSpace) \
   HP(external_fragmentation_code_space,                                        \
      V8.MemoryExternalFragmentationCodeSpace)                                  \
   HP(external_fragmentation_map_space, V8.MemoryExternalFragmentationMapSpace) \
@@ skipping 190 matching lines @@
 #define HR(name, caption, min, max, num_buckets) \
   Histogram* name() { return &name##_; }
   HISTOGRAM_RANGE_LIST(HR)
 #undef HR
 
 #define HT(name, caption, max, res) \
   HistogramTimer* name() { return &name##_; }
   HISTOGRAM_TIMER_LIST(HT)
 #undef HT
 
+#define HT(name, caption, max, res) \
+  TimedHistogram* name() { return &name##_; }
+  TIMED_HISTOGRAM_LIST(HT)
+#undef HT
+
 #define AHT(name, caption) \
   AggregatableHistogramTimer* name() { return &name##_; }
   AGGREGATABLE_HISTOGRAM_TIMER_LIST(AHT)
 #undef AHT
 
 #define HP(name, caption) \
   Histogram* name() { return &name##_; }
   HISTOGRAM_PERCENTAGE_LIST(HP)
 #undef HP
 
@@ skipping 48 matching lines @@
     { return &count_of_CODE_AGE_##name##_; } \
   StatsCounter* size_of_CODE_AGE_##name() \
     { return &size_of_CODE_AGE_##name##_; }
   CODE_AGE_LIST_COMPLETE(SC)
 #undef SC
 
   // clang-format off
   enum Id {
 #define RATE_ID(name, caption, max, res) k_##name,
     HISTOGRAM_TIMER_LIST(RATE_ID)
+    TIMED_HISTOGRAM_LIST(RATE_ID)
 #undef RATE_ID
 #define AGGREGATABLE_ID(name, caption) k_##name,
     AGGREGATABLE_HISTOGRAM_TIMER_LIST(AGGREGATABLE_ID)
 #undef AGGREGATABLE_ID
 #define PERCENTAGE_ID(name, caption) k_##name,
     HISTOGRAM_PERCENTAGE_LIST(PERCENTAGE_ID)
 #undef PERCENTAGE_ID
 #define MEMORY_ID(name, caption) k_##name,
     HISTOGRAM_LEGACY_MEMORY_LIST(MEMORY_ID)
     HISTOGRAM_MEMORY_LIST(MEMORY_ID)
@@ skipping 48 matching lines @@
   Isolate* isolate() { return isolate_; }
 
 #define HR(name, caption, min, max, num_buckets) Histogram name##_;
   HISTOGRAM_RANGE_LIST(HR)
 #undef HR
 
 #define HT(name, caption, max, res) HistogramTimer name##_;
   HISTOGRAM_TIMER_LIST(HT)
 #undef HT
 
+#define HT(name, caption, max, res) TimedHistogram name##_;
+  TIMED_HISTOGRAM_LIST(HT)
+#undef HT
+
 #define AHT(name, caption) \
   AggregatableHistogramTimer name##_;
   AGGREGATABLE_HISTOGRAM_TIMER_LIST(AHT)
 #undef AHT
 
 #define HP(name, caption) \
   Histogram name##_;
   HISTOGRAM_PERCENTAGE_LIST(HP)
 #undef HP
 
@@ skipping 40 matching lines @@
   StatsCounter size_of_CODE_AGE_##name##_; \
   StatsCounter count_of_CODE_AGE_##name##_;
   CODE_AGE_LIST_COMPLETE(SC)
 #undef SC
 
   RuntimeCallStats runtime_call_stats_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(Counters);
 };
 
+void HistogramTimer::Start() {
+  TimedHistogram::Start(&timer_, counters()->isolate());
+}
+
+void HistogramTimer::Stop() {
+  TimedHistogram::Stop(&timer_, counters()->isolate());
+}
+
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_COUNTERS_H_