Fix use of history timers in background threads.

src/counters.h

Index: src/counters.h
diff --git a/src/counters.h b/src/counters.h
index afa62dab3d980579566bbcd7ee809f51ce7e3999..5587eebbb74d3bcfe3f61fa9271ca7f967fcb23b 100644
--- a/src/counters.h
+++ b/src/counters.h
@@ -248,25 +248,62 @@ class 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();
+};
+
+// 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_); }
+
+ private:
+  base::ElapsedTimer timer_;
+  TimedHistogram* histogram_;
+  Isolate* isolate_;
 
-  // Start the timer.
-  void Start();
+  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) {}
 
-  // Stop the timer and record the results.
-  void Stop();
+  inline void Start();
+  inline void Stop();
 
   // Returns true if the timer is running.
   bool Running() {
@@ -282,7 +319,6 @@ class HistogramTimer : public Histogram {
   friend class Counters;
 
   base::ElapsedTimer timer_;
-  Resolution resolution_;
 
   HistogramTimer() {}
 };
@@ -327,7 +363,6 @@ class HistogramTimerScope BASE_EMBEDDED {
 #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
@@ -1007,18 +1042,6 @@ class RuntimeCallTimerScope {
      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,    \
@@ -1026,6 +1049,20 @@ class RuntimeCallTimerScope {
   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)
 
@@ -1236,6 +1273,11 @@ class Counters : public std::enable_shared_from_this<Counters> {
   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)
@@ -1304,6 +1346,7 @@ class Counters : public std::enable_shared_from_this<Counters> {
   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)
@@ -1372,6 +1415,10 @@ class Counters : public std::enable_shared_from_this<Counters> {
   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)
@@ -1432,6 +1479,14 @@ class Counters : public std::enable_shared_from_this<Counters> {
   DISALLOW_IMPLICIT_CONSTRUCTORS(Counters);
 };
 
+void HistogramTimer::Start() {
+  TimedHistogram::Start(&timer_, counters()->isolate());
+}
+
+void HistogramTimer::Stop() {
+  TimedHistogram::Stop(&timer_, counters()->isolate());
+}
+
 }  // namespace internal
 }  // namespace v8