Localize counter class member functions.

src/counters.cc

 // 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.
 
 #include "src/counters.h"
 
 #include <iomanip>
 
 #include "src/base/platform/platform.h"
 #include "src/builtins/builtins-definitions.h"
 #include "src/isolate.h"
 #include "src/log-inl.h"
 #include "src/log.h"
 
 namespace v8 {
 namespace internal {
 
 StatsTable::StatsTable(Counters* counters)
     : counters_(counters),
       lookup_function_(NULL),
       create_histogram_function_(NULL),
       add_histogram_sample_function_(NULL) {}
 
 void StatsTable::SetCounterFunction(CounterLookupCallback f) {
   lookup_function_ = f;
   counters_->ResetCounters();
 }
 
+void StatsTable::SetCreateHistogramFunction(CreateHistogramCallback f) {

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

It's a bit surprising setting the callback (which sounds like an accessor) has a
reset as a side-effect. Would it be possible to not have a
SetCreateHistogramFunction, and instead have it passed to the constructor?

Alternatively, I'd add the "AndResetHistograms" to the name - yes, it'd be long;
but that'd be the only shortcoming, and it'd reduce maintenance cost (one less
surprise)

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

Ok. However, the current name matches the "include/v8.h" API for class "Isolate"
(which then did the same thing). I was really trying to move that logic into
this counters.{h,cc} files to localize the internal implementation of the API.

Changed the comments for these functions instead to intentionally not specify
implementation details.

+  create_histogram_function_ = f;
+  counters_->ResetHistograms();
+}
+
 int* StatsCounterBase::FindLocationInStatsTable() const {
   return counters_->stats_table()->FindLocation(name_);
 }
 
 StatsCounterThreadSafe::StatsCounterThreadSafe(Counters* counters,
                                                const char* name)
-    : StatsCounterBase(counters, name) {
-  GetPtr();
-}
+    : StatsCounterBase(counters, name) {}
 
 void StatsCounterThreadSafe::Set(int Value) {
   if (ptr_) {
     base::LockGuard<base::Mutex> Guard(&mutex_);
     SetLoc(ptr_, Value);
   }
 }
 
 void StatsCounterThreadSafe::Increment() {
   if (ptr_) {
@@ skipping 17 matching lines @@
 }
 
 void StatsCounterThreadSafe::Decrement(int value) {
   if (ptr_) {
     base::LockGuard<base::Mutex> Guard(&mutex_);
     DecrementLoc(ptr_, value);
   }
 }
 
 int* StatsCounterThreadSafe::GetPtr() {
-  base::LockGuard<base::Mutex> Guard(&mutex_);
   ptr_ = FindLocationInStatsTable();
   return ptr_;
 }
 
 void Histogram::AddSample(int sample) {
   if (Enabled()) {
     counters_->stats_table()->AddHistogramSample(histogram_, sample);
   }
 }
 
@@ skipping 223 matching lines @@
 #define HP(name, caption) name##_.Reset();
     HISTOGRAM_PERCENTAGE_LIST(HP)
 #undef HP
 
 #define HM(name, caption) name##_.Reset();
     HISTOGRAM_LEGACY_MEMORY_LIST(HM)
     HISTOGRAM_MEMORY_LIST(HM)
 #undef HM
 }
 
-void Counters::InitializeHistograms() {
-#define HR(name, caption, min, max, num_buckets) name##_.Enabled();
-  HISTOGRAM_RANGE_LIST(HR)
-#undef HR
-
-#define HT(name, caption, max, res) name##_.Enabled();
-  HISTOGRAM_TIMER_LIST(HT)
-#undef HT
-
-#define AHT(name, caption) name##_.Enabled();
-  AGGREGATABLE_HISTOGRAM_TIMER_LIST(AHT)
-#undef AHT
-
-#define HP(name, caption) name##_.Enabled();
-  HISTOGRAM_PERCENTAGE_LIST(HP)
-#undef HP
-
-#define HM(name, caption) name##_.Enabled();
-  HISTOGRAM_LEGACY_MEMORY_LIST(HM)
-  HISTOGRAM_MEMORY_LIST(HM)
-#undef HM
-}
-
 class RuntimeCallStatEntries {
  public:
   void Print(std::ostream& os) {
     if (total_call_count == 0) return;
     std::sort(entries.rbegin(), entries.rend());
     os << std::setw(50) << "Runtime Function/C++ Builtin" << std::setw(12)
        << "Time" << std::setw(18) << "Count" << std::endl
        << std::string(88, '=') << std::endl;
     for (Entry& entry : entries) {
       entry.SetTotal(total_time, total_call_count);
@@ skipping 233 matching lines @@
        RuntimeCallStats::counters) {
     RuntimeCallCounter* counter = &(this->*counter_id);
     if (counter->count() > 0) counter->Dump(value);
   }
 
   in_use_ = false;
 }
 
 }  // namespace internal
 }  // namespace v8