Exponential decay on sample rate for Rendering Core UKM

Switch the Render Core UKM sample rate, the number of frames between
samples, to be an exponentially decaying value designed to get more
early frame samples and exponentially fewer samples as time moves
on.

This is designed to limit the total sample count while prioritizing
updates early in the page lifetime.

The parameters have been tuned to give an effectively maximum sample
count of 40-50, which is the value indicated as suitable by previous
analysis of UKM sample counts.

Bug: 1014195
Change-Id: Ic2004030be69514f6499ff6df18dcaac0588785b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918159
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720505}

third_party/blink/renderer/core/frame/local_frame_ukm_aggregator.cc

@@ -422,13 +422,9 @@ unsigned LocalFrameUkmAggregator::SampleFramesToNextEvent() {
     return frames_to_next_event_for_test_;
 
   // Sample from an exponential distribution to give a poisson distribution
-  // of samples per time unit. In this case, a mean of one sample per
-  // mean_milliseconds_between_samples_. The exponential distribution tends
-  // to give more samples in the range (0, mean) than in the range
-  // [mean, infinity). Intuitively, the (0, mean) is bounded and can only
-  // influence the mean so much, while the [mean, infinity) range is infinite
-  // and can generate some long interval times (though there is less than 1%
-  // chance of an interval mofre than 5 * mean).
+  // of samples per time unit, then weigh it with an exponential multiplier to
+  // give a few samples in rapid succession (for frames early in the page's
+  // life) then exponentially fewer as the page lives longer.
   // RandDouble() returns [0,1), but we need (0,1]. If RandDouble() is
   // uniformly random, so is 1-RandDouble(), so use it to adjust the range.
   // When RandDouble returns 0.0, as it could, we will get a float_sample of
@@ -436,14 +432,20 @@ unsigned LocalFrameUkmAggregator::SampleFramesToNextEvent() {
   // sample until we get a positive count.
   double float_sample = 0;
   do {
-    float_sample =
-        -(mean_frames_between_samples_ * std::log(1.0 - base::RandDouble()));
+    float_sample = -(sample_rate_multiplier_ *
+                     std::exp(samples_so_far_ / sample_decay_rate_) *
+                     std::log(1.0 - base::RandDouble()));
   } while (float_sample == 0);
   // float_sample is positive, so we don't need to worry about underflow.
-  // But with extremely low probability we might end up with a super high
+  // After around 100 samples we will end up with a super high
   // sample. That's OK because it just means we'll stop reporting metrics
-  // for that session.
-  return (unsigned)std::ceil(float_sample);
+  // for that session, but we do need to be careful about overflow and NaN.
+  samples_so_far_++;
+  unsigned unsigned_sample =
+      std::isnan(float_sample)
+          ? UINT_MAX
+          : base::saturated_cast<unsigned>(std::ceil(float_sample));
+  return unsigned_sample;
 }
 
 bool LocalFrameUkmAggregator::AllMetricsAreZero() {

third_party/blink/renderer/core/frame/local_frame_ukm_aggregator.h

@@ -253,10 +253,6 @@ class CORE_EXPORT LocalFrameUkmAggregator
   // RecordEndOfFrameMetrics.
   std::unique_ptr<cc::BeginMainFrameMetrics> GetBeginMainFrameMetrics();
 
-  // The caller is the owner of the |clock|. The |clock| must outlive the
-  // LocalFrameUkmAggregator.
-  void SetTickClockForTesting(const base::TickClock* clock);
-
  private:
   struct AbsoluteMetricRecord {
     std::unique_ptr<CustomCountHistogram> uma_counter;
@@ -299,6 +295,10 @@ class CORE_EXPORT LocalFrameUkmAggregator
   // Used to check that we only for the MainFrame of a document.
   bool AllMetricsAreZero();
 
+  // The caller is the owner of the |clock|. The |clock| must outlive the
+  // LocalFrameUkmAggregator.
+  void SetTickClockForTesting(const base::TickClock* clock);
+
   // UKM system data
   const int64_t source_id_;
   ukm::UkmRecorder* const recorder_;
@@ -310,10 +310,17 @@ class CORE_EXPORT LocalFrameUkmAggregator
   Vector<AbsoluteMetricRecord> absolute_metric_records_;
   Vector<MainFramePercentageRecord> main_frame_percentage_records_;
 
-  // Sampling control. Currently we sample a bit more than every 30s assuming we
-  // are achieving 60fps. Better to sample less rather than more given our data
-  // is already beyond the throtting threshold.
-  unsigned mean_frames_between_samples_ = 2000;
+  // Sampling control. We use a Poisson process with an exponential decay
+  // multiplier. The goal is to get many randomly distributed samples early
+  // during page load and initial interaction, then samples at an exponentially
+  // decreasing rate to effectively cap the number of samples. The particular
+  // parameters chosen here give roughly 10-15 samples in the first 100 frames,
+  // decaying to several hours between samples by the 40th sample. The
+  // multiplier value should be tuned to achieve a total sample count that
+  // avoids throttling by the UKM system.
+  double sample_decay_rate_ = 3;
+  double sample_rate_multiplier_ = 1;
+  unsigned samples_so_far_ = 0;
   unsigned frames_to_next_event_ = 0;
 
   // Control for the ForcedStyleAndUpdate UMA metric sampling

third_party/blink/renderer/core/frame/local_frame_ukm_aggregator_test.cc

@@ -53,6 +53,12 @@ class LocalFrameUkmAggregatorTest : public testing::Test {
     aggregator().FramesToNextEventForTest(delta);
   }
 
+  unsigned FramesToNextEvent() { return aggregator().frames_to_next_event_; }
+  unsigned SamplesSoFar() { return aggregator().samples_so_far_; }
+  unsigned SampleFramesToNextEvent() {
+    return aggregator().SampleFramesToNextEvent();
+  }
+
   base::TimeTicks Now() { return test_task_runner_->NowTicks(); }
 
  protected:
@@ -369,4 +375,20 @@ TEST_F(LocalFrameUkmAggregatorTest, LatencyDataIsPopulated) {
   // is not set by the aggregator.
 }
 
+TEST_F(LocalFrameUkmAggregatorTest, SampleFramesGoesToMaxUnsigned) {
+  // This will time out if the exponential decay in sample rate does not
+  // happen. It should not take too many iterations to reach maximum time
+  // between samples.
+  unsigned initial_sample_count = SamplesSoFar();
+  unsigned last_sample_count = initial_sample_count;
+  unsigned frames_to_next_event = FramesToNextEvent();
+  while (frames_to_next_event < UINT_MAX) {
+    frames_to_next_event = SampleFramesToNextEvent();
+    EXPECT_GT(frames_to_next_event, 0u);
+    EXPECT_EQ(last_sample_count + 1, SamplesSoFar());
+    last_sample_count++;
+  }
+  EXPECT_NE(initial_sample_count, last_sample_count);
+}
+
 }  // namespace blink