heap: Create counter histograms lazily

Until now, histograms associated with isolate counters were created
at the time of isolate initialization. This is too early because it
happens before persistent memory is configured. Histograms created
before persistent memory is set up are retrieved periodically but
infrequently and are not "flushed" on process termination. As a result,
a lot of samples from V8 are lost.

This CL implements lazy creation of counter histograms, the first time
that they are used.

Bug: chromium:1270428
Change-Id: I8540b50b6c3dde1f477853a011b6c3f2c2c6ef9d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3284888
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78125}

src/logging/counters.cc

@@ -73,7 +73,7 @@ void Histogram::AddSample(int sample) {
   }
 }
 
-void* Histogram::CreateHistogram() const {
+V8_EXPORT_PRIVATE void* Histogram::CreateHistogram() const {
   return counters_->CreateHistogram(name_, min_, max_, num_buckets_);
 }
 
@@ -143,9 +143,9 @@ Counters::Counters(Isolate* isolate)
 #undef HR
   };
   for (const auto& histogram : kHistograms) {
-    this->*histogram.member =
-        Histogram(histogram.caption, histogram.min, histogram.max,
-                  histogram.num_buckets, this);
+    (this->*histogram.member)
+        .Initialize(histogram.caption, histogram.min, histogram.max,
+                    histogram.num_buckets, this);
   }
 
   const int DefaultTimedHistogramNumBuckets = 50;
@@ -162,9 +162,9 @@ Counters::Counters(Isolate* isolate)
 #undef HT
   };
   for (const auto& timer : kNestedTimedHistograms) {
-    this->*timer.member =
-        NestedTimedHistogram(timer.caption, 0, timer.max, timer.res,
-                             DefaultTimedHistogramNumBuckets, this);
+    (this->*timer.member)
+        .Initialize(timer.caption, 0, timer.max, timer.res,
+                    DefaultTimedHistogramNumBuckets, this);
   }
 
   static const struct {
@@ -179,8 +179,9 @@ Counters::Counters(Isolate* isolate)
 #undef HT
   };
   for (const auto& timer : kTimedHistograms) {
-    this->*timer.member = TimedHistogram(timer.caption, 0, timer.max, timer.res,
-                                         DefaultTimedHistogramNumBuckets, this);
+    (this->*timer.member)
+        .Initialize(timer.caption, 0, timer.max, timer.res,
+                    DefaultTimedHistogramNumBuckets, this);
   }
 
   static const struct {
@@ -192,8 +193,9 @@ Counters::Counters(Isolate* isolate)
 #undef AHT
   };
   for (const auto& aht : kAggregatableHistogramTimers) {
-    this->*aht.member = AggregatableHistogramTimer(
-        aht.caption, 0, 10000000, DefaultTimedHistogramNumBuckets, this);
+    (this->*aht.member)
+        .Initialize(aht.caption, 0, 10000000, DefaultTimedHistogramNumBuckets,
+                    this);
   }
 
   static const struct {
@@ -205,7 +207,8 @@ Counters::Counters(Isolate* isolate)
 #undef HP
   };
   for (const auto& percentage : kHistogramPercentages) {
-    this->*percentage.member = Histogram(percentage.caption, 0, 101, 100, this);
+    (this->*percentage.member)
+        .Initialize(percentage.caption, 0, 101, 100, this);
   }
 
   // Exponential histogram assigns bucket limits to points
@@ -223,8 +226,8 @@ Counters::Counters(Isolate* isolate)
 #undef HM
   };
   for (const auto& histogram : kLegacyMemoryHistograms) {
-    this->*histogram.member =
-        Histogram(histogram.caption, 1000, 500000, 50, this);
+    (this->*histogram.member)
+        .Initialize(histogram.caption, 1000, 500000, 50, this);
   }
 
   // clang-format off
@@ -303,7 +306,7 @@ void Counters::ResetCreateHistogramFunction(CreateHistogramCallback f) {
   NESTED_TIMED_HISTOGRAM_LIST(HT)
 #undef HT
 
-#define HT(name, caption, max, res) name##_.Reset(FLAG_slow_histograms);
+#define HT(name, caption, max, res) name##_.Reset();
   NESTED_TIMED_HISTOGRAM_LIST_SLOW(HT)
 #undef HT
 

src/logging/counters.h

@@ -228,35 +228,47 @@ class Histogram {
 
  protected:
   Histogram() = default;
-  Histogram(const char* name, int min, int max, int num_buckets,
-            Counters* counters)
-      : name_(name),
-        min_(min),
-        max_(max),
-        num_buckets_(num_buckets),
-        histogram_(nullptr),
-        counters_(counters) {
-    DCHECK(counters_);
+  Histogram(const Histogram&) = delete;
+  Histogram& operator=(const Histogram&) = delete;
+
+  void Initialize(const char* name, int min, int max, int num_buckets,
+                  Counters* counters) {
+    name_ = name;
+    min_ = min;
+    max_ = max;
+    num_buckets_ = num_buckets;
+    histogram_ = nullptr;
+    counters_ = counters;
+    DCHECK_NOT_NULL(counters_);
   }
 
   Counters* counters() const { return counters_; }
 
-  // Reset the cached internal pointer.
-  void Reset(bool create_new = true) {
-    histogram_ = create_new ? CreateHistogram() : nullptr;
+  // Reset the cached internal pointer to nullptr; the histogram will be
+  // created lazily, the first time it is needed.
+  void Reset() { histogram_ = nullptr; }
+
+  // Lazily create the histogram, if it has not been created yet.
+  void EnsureCreated(bool create_new = true) {
+    if (create_new && histogram_.load(std::memory_order_acquire) == nullptr) {
+      base::MutexGuard Guard(&mutex_);
+      if (histogram_.load(std::memory_order_relaxed) == nullptr)
+        histogram_.store(CreateHistogram(), std::memory_order_release);
+    }
   }
 
  private:
   friend class Counters;
 
-  void* CreateHistogram() const;
+  V8_EXPORT_PRIVATE void* CreateHistogram() const;
 
   const char* name_;
   int min_;
   int max_;
   int num_buckets_;
-  void* histogram_;
+  std::atomic<void*> histogram_;
   Counters* counters_;
+  base::Mutex mutex_;
 };
 
 enum class TimedHistogramResolution { MILLISECOND, MICROSECOND };
@@ -290,11 +302,15 @@ class TimedHistogram : public Histogram {
   TimedHistogramResolution resolution_;
 
   TimedHistogram() = default;
-  TimedHistogram(const char* name, int min, int max,
-                 TimedHistogramResolution resolution, int num_buckets,
-                 Counters* counters)
-      : Histogram(name, min, max, num_buckets, counters),
-        resolution_(resolution) {}
+  TimedHistogram(const TimedHistogram&) = delete;
+  TimedHistogram& operator=(const TimedHistogram&) = delete;
+
+  void Initialize(const char* name, int min, int max,
+                  TimedHistogramResolution resolution, int num_buckets,
+                  Counters* counters) {
+    Histogram::Initialize(name, min, max, num_buckets, counters);
+    resolution_ = resolution;
+  }
 };
 
 class NestedTimedHistogramScope;
@@ -307,7 +323,9 @@ class NestedTimedHistogram : public TimedHistogram {
   NestedTimedHistogram(const char* name, int min, int max,
                        TimedHistogramResolution resolution, int num_buckets,
                        Counters* counters)
-      : TimedHistogram(name, min, max, resolution, num_buckets, counters) {}
+      : NestedTimedHistogram() {
+    Initialize(name, min, max, resolution, num_buckets, counters);
+  }
 
  private:
   friend class Counters;
@@ -327,6 +345,8 @@ class NestedTimedHistogram : public TimedHistogram {
   NestedTimedHistogramScope* current_ = nullptr;
 
   NestedTimedHistogram() = default;
+  NestedTimedHistogram(const NestedTimedHistogram&) = delete;
+  NestedTimedHistogram& operator=(const NestedTimedHistogram&) = delete;
 };
 
 // A histogram timer that can aggregate events within a larger scope.
@@ -361,9 +381,9 @@ class AggregatableHistogramTimer : public Histogram {
   friend class Counters;
 
   AggregatableHistogramTimer() = default;
-  AggregatableHistogramTimer(const char* name, int min, int max,
-                             int num_buckets, Counters* counters)
-      : Histogram(name, min, max, num_buckets, counters) {}
+  AggregatableHistogramTimer(const AggregatableHistogramTimer&) = delete;
+  AggregatableHistogramTimer& operator=(const AggregatableHistogramTimer&) =
+      delete;
 
   base::TimeDelta time_;
 };
@@ -539,33 +559,58 @@ class Counters : public std::enable_shared_from_this<Counters> {
   }
 
 #define HR(name, caption, min, max, num_buckets) \
-  Histogram* name() { return &name##_; }
+  Histogram* name() {                            \
+    name##_.EnsureCreated();                     \
+    return &name##_;                             \
+  }
   HISTOGRAM_RANGE_LIST(HR)
 #undef HR
 
 #define HT(name, caption, max, res) \
-  NestedTimedHistogram* name() { return &name##_; }
+  NestedTimedHistogram* name() {    \
+    name##_.EnsureCreated();        \
+    return &name##_;                \
+  }
   NESTED_TIMED_HISTOGRAM_LIST(HT)
+#undef HT
+
+#define HT(name, caption, max, res)              \
+  NestedTimedHistogram* name() {                 \
+    name##_.EnsureCreated(FLAG_slow_histograms); \
+    return &name##_;                             \
+  }
   NESTED_TIMED_HISTOGRAM_LIST_SLOW(HT)
 #undef HT
 
 #define HT(name, caption, max, res) \
-  TimedHistogram* name() { return &name##_; }
+  TimedHistogram* name() {          \
+    name##_.EnsureCreated();        \
+    return &name##_;                \
+  }
   TIMED_HISTOGRAM_LIST(HT)
 #undef HT
 
-#define AHT(name, caption) \
-  AggregatableHistogramTimer* name() { return &name##_; }
+#define AHT(name, caption)             \
+  AggregatableHistogramTimer* name() { \
+    name##_.EnsureCreated();           \
+    return &name##_;                   \
+  }
   AGGREGATABLE_HISTOGRAM_TIMER_LIST(AHT)
 #undef AHT
 
-#define HP(name, caption) \
-  Histogram* name() { return &name##_; }
+#define HP(name, caption)    \
+  Histogram* name() {        \
+    name##_.EnsureCreated(); \
+    return &name##_;         \
+  }
   HISTOGRAM_PERCENTAGE_LIST(HP)
 #undef HP
 
-#define HM(name, caption) \
-  Histogram* name() { return &name##_; }
+#define HM(name, caption)    \
+  Histogram* name() {        \
+    name##_.EnsureCreated(); \
+    return &name##_;         \
+  }
   HISTOGRAM_LEGACY_MEMORY_LIST(HM)
 #undef HM