Localize counter class member functions.

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 18 matching lines @@
 
 class StatsTable {
  public:
   // Register an application-defined function where
   // 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);
 
   // Register an application-defined function to create
   // a histogram for passing to the AddHistogramSample function
-  void SetCreateHistogramFunction(CreateHistogramCallback f) {
-    create_histogram_function_ = f;
-  }
+  void SetCreateHistogramFunction(CreateHistogramCallback f);
 
   // Register an application-defined function to add a sample
   // to a histogram created with CreateHistogram function
   void SetAddHistogramSampleFunction(AddHistogramSampleCallback f) {
     add_histogram_sample_function_ = f;
   }
 
   bool HasCounterFunction() const {
     return lookup_function_ != NULL;
   }
@@ skipping 62 matching lines @@
   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.
+// thread-specific, this class cannot be shared across threads. Note: This
+// class is not thread safe.
 class StatsCounter : public StatsCounterBase {
  public:
-  StatsCounter() { }
-  StatsCounter(Counters* counters, const char* name)
-      : StatsCounterBase(counters, name), lookup_done_(false) {}
-
   // Sets the counter to a specific value.
   void Set(int value) {
     if (int* loc = GetPtr()) SetLoc(loc, value);
   }
 
   // Increments the counter.
   void Increment() {
     if (int* loc = GetPtr()) IncrementLoc(loc);
   }
 
@@ skipping 18 matching lines @@
 
   // 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;
   }
 
+ private:
+  StatsCounter() {}
+  StatsCounter(Counters* counters, const char* name)
+      : StatsCounterBase(counters, name), lookup_done_(false) {}
   // Reset the cached internal pointer.
   void Reset() { lookup_done_ = false; }
 
- private:
   // Returns the cached address of this counter location.
   int* GetPtr() {
     if (lookup_done_) return ptr_;
     lookup_done_ = true;
     ptr_ = FindLocationInStatsTable();
     return ptr_;
   }
 
   bool lookup_done_;
+
+  friend class Counters;
 };
 
-// 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).
+// Thread safe version of StatsCounter.
 class StatsCounterThreadSafe : public StatsCounterBase {
  public:
-  StatsCounterThreadSafe(Counters* counters, 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:
+  StatsCounterThreadSafe(Counters* counters, const char* name);
   int* GetPtr();

[Mircea Trofin, 2017/06/05 15:57:49]

Why not delete GetPtr(), since it's only used in Reset()?

[kschimpf, 2017/06/05 17:38:52]

Your right. It used to have multiple uses, but is no longer needed. Fixing.

+  void Reset() { GetPtr(); }
 
   base::Mutex mutex_;
 
  private:
+  friend class Counters;
   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.
+// A Histogram represents a dynamically created histogram in the
+// StatsTable.  Note: This class is thread safe.
 class Histogram {
  public:
-  Histogram() { }
+  // Add a single sample to this histogram.
+  void AddSample(int sample);
+
+  // Returns true if this histogram is enabled.
+  bool Enabled() { return histogram_ != nullptr; }
+
+  const char* name() { return name_; }
+
+ protected:
+  Counters* counters() const { return counters_; }
+
+ protected:
+  Histogram() {}
   Histogram(const char* name, int min, int max, int num_buckets,
             Counters* counters)
       : name_(name),
         min_(min),
         max_(max),
         num_buckets_(num_buckets),
-        histogram_(NULL),
-        lookup_done_(false),
+        histogram_(nullptr),
         counters_(counters) {}
 
-  // Add a single sample to this histogram.
-  void AddSample(int sample);
-
-  // Returns true if this histogram is enabled.
-  bool Enabled() {
-    return GetHistogram() != NULL;
-  }
-
   // Reset the cached internal pointer.
-  void Reset() {
-    lookup_done_ = false;
-  }
-
-  const char* name() { return name_; }
-
- protected:
-  // Returns the handle to the histogram.
-  void* GetHistogram() {
-    if (!lookup_done_) {
-      lookup_done_ = true;
-      histogram_ = CreateHistogram();
-    }
-    return histogram_;
-  }
-
-  Counters* counters() const { return counters_; }
+  void Reset() { histogram_ = CreateHistogram(); }
 
  private:
   void* CreateHistogram() const;
 
   const char* name_;
   int min_;
   int max_;
   int num_buckets_;
   void* histogram_;
-  bool lookup_done_;
   Counters* counters_;
+
+  friend class Counters;
 };
 
 // A HistogramTimer allows distributions of results to be created.
 class HistogramTimer : public Histogram {
  public:
   enum Resolution {
     MILLISECOND,
     MICROSECOND
   };
 
-  HistogramTimer() {}
+  // Note: public for testing purposes only.
   HistogramTimer(const char* name, int min, int max, Resolution resolution,
                  int num_buckets, Counters* counters)
       : Histogram(name, min, max, num_buckets, counters),
         resolution_(resolution) {}
 
   // Start the timer.
   void Start();
 
   // Stop the timer and record the results.
   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:
+  HistogramTimer() {}
+
   base::ElapsedTimer timer_;
   Resolution resolution_;
+
+  friend class Counters;
 };
 
 // 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 {
  public:
   explicit HistogramTimerScope(HistogramTimer* timer,
@@ skipping 38 matching lines @@
 // An example use might be to aggregate the time spent in lazy compilation
 // while running a script.
 //
 // Helpers:
 // - AggregatingHistogramTimerScope, the "outer" scope within which
 //     times will be summed up.
 // - AggregatedHistogramTimerScope, the "inner" scope which defines the
 //     events to be timed.
 class AggregatableHistogramTimer : public Histogram {
  public:
-  AggregatableHistogramTimer() {}
-  AggregatableHistogramTimer(const char* name, int min, int max,
-                             int num_buckets, Counters* counters)
-      : Histogram(name, min, max, num_buckets, counters) {}
-
   // Start/stop the "outer" scope.
   void Start() { time_ = base::TimeDelta(); }
   void Stop() { AddSample(static_cast<int>(time_.InMicroseconds())); }
 
   // Add a time value ("inner" scope).
   void Add(base::TimeDelta other) { time_ += other; }
 
  private:
+  AggregatableHistogramTimer() {}
+  AggregatableHistogramTimer(const char* name, int min, int max,
+                             int num_buckets, Counters* counters)
+      : Histogram(name, min, max, num_buckets, counters) {}
+
   base::TimeDelta time_;
+  friend class Counters;
 };
 
 // A helper class for use with AggregatableHistogramTimer. This is the
 // // outer-most timer scope used with an AggregatableHistogramTimer. It will
 // // aggregate the information from the inner AggregatedHistogramTimerScope.
 class AggregatingHistogramTimerScope {
  public:
   explicit AggregatingHistogramTimerScope(AggregatableHistogramTimer* histogram)
       : histogram_(histogram) {
     histogram_->Start();
@@ skipping 25 matching lines @@
 // backing histogram receives one sample every T ms, where the T is controlled
 // by the FLAG_histogram_interval.
 //
 // More formally: let F be a real-valued function that maps time to sample
 // values. We define F as a linear interpolation between adjacent samples. For
 // each time interval [x; x + T) the backing histogram gets one sample value
 // that is the average of F(t) in the interval.
 template <typename Histogram>
 class AggregatedMemoryHistogram {
  public:
-  AggregatedMemoryHistogram()
-      : is_initialized_(false),
-        start_ms_(0.0),
-        last_ms_(0.0),
-        aggregate_value_(0.0),
-        last_value_(0.0),
-        backing_histogram_(NULL) {}
-
+  // Note: public for testing purposes only.
   explicit AggregatedMemoryHistogram(Histogram* backing_histogram)
       : AggregatedMemoryHistogram() {
     backing_histogram_ = backing_histogram;
   }
 
   // Invariants that hold before and after AddSample if
   // is_initialized_ is true:
   //
   // 1) For we processed samples that came in before start_ms_ and sent the
   // corresponding aggregated samples to backing histogram.
   // 2) (last_ms_, last_value_) is the last received sample.
   // 3) last_ms_ < start_ms_ + FLAG_histogram_interval.
   // 4) aggregate_value_ is the average of the function that is constructed by
   // linearly interpolating samples received between start_ms_ and last_ms_.
   void AddSample(double current_ms, double current_value);
 
  private:
+  AggregatedMemoryHistogram()
+      : is_initialized_(false),
+        start_ms_(0.0),
+        last_ms_(0.0),
+        aggregate_value_(0.0),
+        last_value_(0.0),
+        backing_histogram_(NULL) {}
+
   double Aggregate(double current_ms, double current_value);
   bool is_initialized_;
   double start_ms_;
   double last_ms_;
   double aggregate_value_;
   double last_value_;
   Histogram* backing_histogram_;
+
+  friend class Counters;
 };
 
 
 template <typename Histogram>
 void AggregatedMemoryHistogram<Histogram>::AddSample(double current_ms,
                                                      double current_value) {
   if (!is_initialized_) {
     aggregate_value_ = current_value;
     start_ms_ = current_ms;
     last_value_ = current_value;
@@ skipping 864 matching lines @@
     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_; }
 
   StatsTable* stats_table() { return &stats_table_; }
 
   Isolate* isolate() { return isolate_; }
 
  private:
   Isolate* isolate_;
   StatsTable stats_table_;
 
@@ skipping 55 matching lines @@
 #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 StatsTable;
+  void ResetCounters();
+  void ResetHistograms();
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(Counters);
 };
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_COUNTERS_H_