Create a thread safe version of StatsCounters and use.

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 10 matching lines @@
 namespace v8 {
 namespace internal {
 
 // StatsCounters is an interface for plugging into external
 // counters for monitoring.  Counters can be looked up and
 // manipulated by name.
 
 class StatsTable {
  public:
   // Register an application-defined function where
-  // counters can be looked up.
-  void SetCounterFunction(CounterLookupCallback f) {
-    lookup_function_ = f;
-  }
+  // counters can be looked up. Note: Must be called on main thread,
+  // so that threaded stats counters can be created now.
+  void SetCounterFunction(CounterLookupCallback f, Isolate* isolate);
 
   // Register an application-defined function to create
   // a histogram for passing to the AddHistogramSample function
   void SetCreateHistogramFunction(CreateHistogramCallback f) {
     create_histogram_function_ = f;
   }
 
   // Register an application-defined function to add a sample
   // to a histogram created with CreateHistogram function
   void SetAddHistogramSampleFunction(AddHistogramSampleCallback f) {
@@ skipping 40 matching lines @@
 
   CounterLookupCallback lookup_function_;
   CreateHistogramCallback create_histogram_function_;
   AddHistogramSampleCallback add_histogram_sample_function_;
 
   friend class Isolate;
 
   DISALLOW_COPY_AND_ASSIGN(StatsTable);
 };
 
+// Base class for stats counters.
+class StatsCounterBase {
+ public:
+  StatsCounterBase() {}
+  StatsCounterBase(Isolate* isolate, const char* name)
+      : isolate_(isolate), name_(name), ptr_(nullptr) {}
+
+ protected:
+  Isolate* isolate_;
+  const char* name_;
+  int* ptr_;
+
+  void SetLoc(int* loc, int value) { *loc = value; }
+  void IncrementLoc(int* loc) { (*loc)++; }

[Clemens Hammacher, 2017/06/02 13:08:49]

Is there any implementation where the semantics of {IncrementLoc(loc)} is
different from {IncrementLoc(loc, 1)}? If no, we should just set a default value
of 1 for the {int value} parameter.
(Also on all other occurences of this pattern)

[kschimpf, 2017/06/02 15:01:02]

I did this to follow the same pattern as the code it replaced. However, I'll
look to see if it is unnecessary.

[kschimpf, 2017/06/02 17:06:27]

There are calls on "size" counters that increment counts by the size value.
Hence they are needed.

+  void IncrementLoc(int* loc, int value) { (*loc) += value; }
+  void DecrementLoc(int* loc) { (*loc)--; }
+  void DecrementLoc(int* loc, int value) { (*loc) -= value; }
+
+  int* FindLocationInStatsTable() const;
+};
+
 // StatsCounters are dynamically created values which can be tracked in
 // the StatsTable.  They are designed to be lightweight to create and
 // easy to use.
 //
 // Internally, a counter represents a value in a row of a StatsTable.
 // The row has a 32bit value for each process/thread in the table and also
 // a name (stored in the table metadata).  Since the storage location can be
 // thread-specific, this class cannot be shared across threads.
-class StatsCounter {
+class StatsCounter : public StatsCounterBase {
  public:
   StatsCounter() { }
-  explicit StatsCounter(Isolate* isolate, const char* name)
-      : isolate_(isolate), name_(name), ptr_(NULL), lookup_done_(false) { }
+  StatsCounter(Isolate* isolate, const char* name)
+      : StatsCounterBase(isolate, name), lookup_done_(false) {}
 
   // Sets the counter to a specific value.
   void Set(int value) {
-    int* loc = GetPtr();
-    if (loc) *loc = value;
+    if (int* loc = GetPtr()) SetLoc(loc, value);
   }
 
   // Increments the counter.
   void Increment() {
-    int* loc = GetPtr();
-    if (loc) (*loc)++;
+    if (int* loc = GetPtr()) IncrementLoc(loc);
   }
 
   void Increment(int value) {

[kschimpf, 2017/06/02 15:01:02]

Here was the previous definition of increment.

-    int* loc = GetPtr();
-    if (loc)
-      (*loc) += value;
+    if (int* loc = GetPtr()) IncrementLoc(loc, value);
   }
 
   // Decrements the counter.
   void Decrement() {
-    int* loc = GetPtr();
-    if (loc) (*loc)--;
+    if (int* loc = GetPtr()) DecrementLoc(loc);
   }
 
   void Decrement(int value) {
-    int* loc = GetPtr();
-    if (loc) (*loc) -= value;
+    if (int* loc = GetPtr()) DecrementLoc(loc, value);
   }
 
   // Is this counter enabled?
   // Returns false if table is full.
   bool Enabled() {
     return GetPtr() != NULL;
   }
 
   // Get the internal pointer to the counter. This is used
   // by the code generator to emit code that manipulates a
   // given counter without calling the runtime system.
   int* GetInternalPointer() {
     int* loc = GetPtr();
     DCHECK(loc != NULL);
     return loc;
   }
 
   // Reset the cached internal pointer.
   void Reset() { lookup_done_ = false; }
 
- protected:
+ private:
   // Returns the cached address of this counter location.
   int* GetPtr() {
     if (lookup_done_) return ptr_;
     lookup_done_ = true;
     ptr_ = FindLocationInStatsTable();
     return ptr_;
   }
 
- private:
-  int* FindLocationInStatsTable() const;
-
-  Isolate* isolate_;
-  const char* name_;
-  int* ptr_;
   bool lookup_done_;
 };
 
+// Thread safe version of StatsCounter. WARNING: Unlike StatsCounter,
+// StatsCounterThreadSafe's constructor and method Reset() actually do
+// the table lookup, and should be called from the main thread
+// (i.e. not workers).
+class StatsCounterThreadSafe : public StatsCounterBase {
+ public:
+  StatsCounterThreadSafe(Isolate* isolate, const char* name);
+
+  void Set(int Value);
+  void Increment();
+  void Increment(int value);
+  void Decrement();
+  void Decrement(int value);
+  bool Enabled() { return ptr_ != NULL; }
+  int* GetInternalPointer() {
+    DCHECK(ptr_ != NULL);
+    return ptr_;
+  }
+  void Reset() { GetPtr(); }
+
+ private:
+  int* GetPtr();
+
+  base::Mutex mutex_;
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(StatsCounterThreadSafe);
+};
+
 // A Histogram represents a dynamically created histogram in the StatsTable.
 // It will be registered with the histogram system on first use.
 class Histogram {
  public:
   Histogram() { }
   Histogram(const char* name,
             int min,
             int max,
             int num_buckets,
             Isolate* isolate)
@@ skipping 976 matching lines @@
   SC(code_space_bytes_used, V8.MemoryCodeSpaceBytesUsed)                       \
   SC(map_space_bytes_available, V8.MemoryMapSpaceBytesAvailable)               \
   SC(map_space_bytes_committed, V8.MemoryMapSpaceBytesCommitted)               \
   SC(map_space_bytes_used, V8.MemoryMapSpaceBytesUsed)                         \
   SC(lo_space_bytes_available, V8.MemoryLoSpaceBytesAvailable)                 \
   SC(lo_space_bytes_committed, V8.MemoryLoSpaceBytesCommitted)                 \
   SC(lo_space_bytes_used, V8.MemoryLoSpaceBytesUsed)                           \
   /* Total code size (including metadata) of baseline code or bytecode. */     \
   SC(total_baseline_code_size, V8.TotalBaselineCodeSize)                       \
   /* Total count of functions compiled using the baseline compiler. */         \
-  SC(total_baseline_compile_count, V8.TotalBaselineCompileCount)               \
-  SC(wasm_generated_code_size, V8.WasmGeneratedCodeBytes)                      \
-  SC(wasm_reloc_size, V8.WasmRelocBytes)                                       \
+  SC(total_baseline_compile_count, V8.TotalBaselineCompileCount)
+
+#define STATS_COUNTER_TS_LIST(SC)                         \
+  SC(wasm_generated_code_size, V8.WasmGeneratedCodeBytes) \
+  SC(wasm_reloc_size, V8.WasmRelocBytes)                  \
   SC(wasm_lazily_compiled_functions, V8.WasmLazilyCompiledFunctions)
 
 // This file contains all the v8 counters that are in use.
-class Counters {
+class Counters : public std::enable_shared_from_this<Counters> {
  public:
+  explicit Counters(Isolate* isolate);
+
 #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
 
@@ skipping 19 matching lines @@
   }
   HISTOGRAM_MEMORY_LIST(HM)
 #undef HM
 
 #define SC(name, caption) \
   StatsCounter* name() { return &name##_; }
   STATS_COUNTER_LIST_1(SC)
   STATS_COUNTER_LIST_2(SC)
 #undef SC
 
+#define SC(name, caption) \
+  StatsCounterThreadSafe* name() { return &name##_; }
+  STATS_COUNTER_TS_LIST(SC)
+#undef SC
+
 #define SC(name) \
   StatsCounter* count_of_##name() { return &count_of_##name##_; } \
   StatsCounter* size_of_##name() { return &size_of_##name##_; }
   INSTANCE_TYPE_LIST(SC)
 #undef SC
 
 #define SC(name) \
   StatsCounter* count_of_CODE_TYPE_##name() \
     { return &count_of_CODE_TYPE_##name##_; } \
   StatsCounter* size_of_CODE_TYPE_##name() \
@@ skipping 10 matching lines @@
 #undef SC
 
 #define SC(name) \
   StatsCounter* count_of_CODE_AGE_##name() \
     { 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)
 #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)
 #undef MEMORY_ID
 #define COUNTER_ID(name, caption) k_##name,
     STATS_COUNTER_LIST_1(COUNTER_ID)
     STATS_COUNTER_LIST_2(COUNTER_ID)
+    STATS_COUNTER_TS_LIST(COUNTER_ID)
 #undef COUNTER_ID
 #define COUNTER_ID(name) kCountOf##name, kSizeOf##name,
     INSTANCE_TYPE_LIST(COUNTER_ID)
 #undef COUNTER_ID
 #define COUNTER_ID(name) kCountOfCODE_TYPE_##name, \
     kSizeOfCODE_TYPE_##name,
     CODE_KIND_LIST(COUNTER_ID)
 #undef COUNTER_ID
 #define COUNTER_ID(name) kCountOfFIXED_ARRAY__##name, \
     kSizeOfFIXED_ARRAY__##name,
     FIXED_ARRAY_SUB_INSTANCE_TYPE_LIST(COUNTER_ID)
 #undef COUNTER_ID
 #define COUNTER_ID(name) kCountOfCODE_AGE__##name, \
     kSizeOfCODE_AGE__##name,
     CODE_AGE_LIST_COMPLETE(COUNTER_ID)
 #undef COUNTER_ID
     stats_counter_count
   };
+  // clang-format on
 
   void ResetCounters();
   void ResetHistograms();
   void InitializeHistograms();
 
   RuntimeCallStats* runtime_call_stats() { return &runtime_call_stats_; }
 
  private:
 #define HR(name, caption, min, max, num_buckets) Histogram name##_;
   HISTOGRAM_RANGE_LIST(HR)
@@ skipping 23 matching lines @@
   AggregatedMemoryHistogram<Histogram> aggregated_##name##_;
   HISTOGRAM_MEMORY_LIST(HM)
 #undef HM
 
 #define SC(name, caption) \
   StatsCounter name##_;
   STATS_COUNTER_LIST_1(SC)
   STATS_COUNTER_LIST_2(SC)
 #undef SC
 
+#define SC(name, caption) StatsCounterThreadSafe name##_;
+  STATS_COUNTER_TS_LIST(SC)
+#undef SC
+
 #define SC(name) \
   StatsCounter size_of_##name##_; \
   StatsCounter count_of_##name##_;
   INSTANCE_TYPE_LIST(SC)
 #undef SC
 
 #define SC(name) \
   StatsCounter size_of_CODE_TYPE_##name##_; \
   StatsCounter count_of_CODE_TYPE_##name##_;
   CODE_KIND_LIST(SC)
 #undef SC
 
 #define SC(name) \
   StatsCounter size_of_FIXED_ARRAY_##name##_; \
   StatsCounter count_of_FIXED_ARRAY_##name##_;
   FIXED_ARRAY_SUB_INSTANCE_TYPE_LIST(SC)
 #undef SC
 
 #define SC(name) \
   StatsCounter size_of_CODE_AGE_##name##_; \
   StatsCounter count_of_CODE_AGE_##name##_;
   CODE_AGE_LIST_COMPLETE(SC)
 #undef SC
 
   RuntimeCallStats runtime_call_stats_;
 
   friend class Isolate;
 
-  explicit Counters(Isolate* isolate);
-
   DISALLOW_IMPLICIT_CONSTRUCTORS(Counters);
 };
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_COUNTERS_H_