Record sampling stride in cord profiling to facilitate unsampling.

The sampling rate may change over time, so this allows us to weight samples by
the value observed when we made the sampling decision.

PiperOrigin-RevId: 616900100
Change-Id: I9b1affdba93f5f48367cb7503916296b2d84709a

absl/strings/internal/cordz_functions.cc

@@ -40,13 +40,15 @@ std::atomic<int> g_cordz_mean_interval(50000);
 // Special negative 'not initialized' per thread value for cordz_next_sample.
 static constexpr int64_t kInitCordzNextSample = -1;
 
-ABSL_CONST_INIT thread_local int64_t cordz_next_sample = kInitCordzNextSample;
+ABSL_CONST_INIT thread_local SamplingState cordz_next_sample = {
+    kInitCordzNextSample, 1};
 
 // kIntervalIfDisabled is the number of profile-eligible events need to occur
 // before the code will confirm that cordz is still disabled.
 constexpr int64_t kIntervalIfDisabled = 1 << 16;
 
-ABSL_ATTRIBUTE_NOINLINE bool cordz_should_profile_slow() {
+ABSL_ATTRIBUTE_NOINLINE int64_t
+cordz_should_profile_slow(SamplingState& state) {
 
   thread_local absl::profiling_internal::ExponentialBiased
       exponential_biased_generator;
@@ -55,30 +57,34 @@ ABSL_ATTRIBUTE_NOINLINE bool cordz_should_profile_slow() {
   // Check if we disabled profiling. If so, set the next sample to a "large"
   // number to minimize the overhead of the should_profile codepath.
   if (mean_interval <= 0) {
-    cordz_next_sample = kIntervalIfDisabled;
-    return false;
+    state = {kIntervalIfDisabled, kIntervalIfDisabled};
+    return 0;
   }
 
   // Check if we're always sampling.
   if (mean_interval == 1) {
-    cordz_next_sample = 1;
-    return true;
+    state = {1, 1};
+    return 1;
   }
 
-  if (cordz_next_sample <= 0) {
+  if (cordz_next_sample.next_sample <= 0) {
     // If first check on current thread, check cordz_should_profile()
     // again using the created (initial) stride in cordz_next_sample.
-    const bool initialized = cordz_next_sample != kInitCordzNextSample;
-    cordz_next_sample = exponential_biased_generator.GetStride(mean_interval);
-    return initialized || cordz_should_profile();
+    const bool initialized =
+        cordz_next_sample.next_sample != kInitCordzNextSample;
+    auto old_stride = state.sample_stride;
+    auto stride = exponential_biased_generator.GetStride(mean_interval);
+    state = {stride, stride};
+    bool should_sample = initialized || cordz_should_profile() > 0;
+    return should_sample ? old_stride : 0;
   }
 
-  --cordz_next_sample;
-  return false;
+  --state.next_sample;
+  return 0;
 }
 
 void cordz_set_next_sample_for_testing(int64_t next_sample) {
-  cordz_next_sample = next_sample;
+  cordz_next_sample = {next_sample, next_sample};
 }
 
 #endif  // ABSL_INTERNAL_CORDZ_ENABLED

absl/strings/internal/cordz_functions.h

@@ -41,31 +41,41 @@ void set_cordz_mean_interval(int32_t mean_interval);
 
 #ifdef ABSL_INTERNAL_CORDZ_ENABLED
 
+struct SamplingState {
+  int64_t next_sample;
+  int64_t sample_stride;
+};
+
 // cordz_next_sample is the number of events until the next sample event. If
 // the value is 1 or less, the code will check on the next event if cordz is
 // enabled, and if so, will sample the Cord. cordz is only enabled when we can
 // use thread locals.
-ABSL_CONST_INIT extern thread_local int64_t cordz_next_sample;
-
-// Determines if the next sample should be profiled. If it is, the value pointed
-// at by next_sample will be set with the interval until the next sample.
-bool cordz_should_profile_slow();
-
-// Returns true if the next cord should be sampled.
-inline bool cordz_should_profile() {
-  if (ABSL_PREDICT_TRUE(cordz_next_sample > 1)) {
-    cordz_next_sample--;
-    return false;
+ABSL_CONST_INIT extern thread_local SamplingState cordz_next_sample;
+
+// Determines if the next sample should be profiled.
+// Returns:
+//   0: Do not sample
+//  >0: Sample with the stride of the last sampling period
+int64_t cordz_should_profile_slow(SamplingState& state);
+
+// Determines if the next sample should be profiled.
+// Returns:
+//   0: Do not sample
+//  >0: Sample with the stride of the last sampling period
+inline int64_t cordz_should_profile() {
+  if (ABSL_PREDICT_TRUE(cordz_next_sample.next_sample > 1)) {
+    cordz_next_sample.next_sample--;
+    return 0;
   }
-  return cordz_should_profile_slow();
+  return cordz_should_profile_slow(cordz_next_sample);
 }
 
 // Sets the interval until the next sample (for testing only)
 void cordz_set_next_sample_for_testing(int64_t next_sample);
 
 #else  // ABSL_INTERNAL_CORDZ_ENABLED
 
-inline bool cordz_should_profile() { return false; }
+inline int64_t cordz_should_profile() { return 0; }
 inline void cordz_set_next_sample_for_testing(int64_t) {}
 
 #endif  // ABSL_INTERNAL_CORDZ_ENABLED

absl/strings/internal/cordz_functions_test.cc

@@ -47,9 +47,9 @@ TEST(CordzFunctionsTest, ShouldProfileDisable) {
 
   set_cordz_mean_interval(0);
   cordz_set_next_sample_for_testing(0);
-  EXPECT_FALSE(cordz_should_profile());
+  EXPECT_EQ(cordz_should_profile(), 0);
   // 1 << 16 is from kIntervalIfDisabled in cordz_functions.cc.
-  EXPECT_THAT(cordz_next_sample, Eq(1 << 16));
+  EXPECT_THAT(cordz_next_sample.next_sample, Eq(1 << 16));
 
   set_cordz_mean_interval(orig_sample_rate);
 }
@@ -59,8 +59,8 @@ TEST(CordzFunctionsTest, ShouldProfileAlways) {
 
   set_cordz_mean_interval(1);
   cordz_set_next_sample_for_testing(1);
-  EXPECT_TRUE(cordz_should_profile());
-  EXPECT_THAT(cordz_next_sample, Le(1));
+  EXPECT_GT(cordz_should_profile(), 0);
+  EXPECT_THAT(cordz_next_sample.next_sample, Le(1));
 
   set_cordz_mean_interval(orig_sample_rate);
 }
@@ -74,9 +74,7 @@ TEST(CordzFunctionsTest, DoesNotAlwaysSampleFirstCord) {
   do {
     ++tries;
     ASSERT_THAT(tries, Le(1000));
-    std::thread thread([&sampled] {
-      sampled = cordz_should_profile();
-    });
+    std::thread thread([&sampled] { sampled = cordz_should_profile() > 0; });
     thread.join();
   } while (sampled);
 }
@@ -94,7 +92,7 @@ TEST(CordzFunctionsTest, ShouldProfileRate) {
     // new value for next_sample each iteration.
     cordz_set_next_sample_for_testing(0);
     cordz_should_profile();
-    sum_of_intervals += cordz_next_sample;
+    sum_of_intervals += cordz_next_sample.next_sample;
   }
 
   // The sum of independent exponential variables is an Erlang distribution,

absl/strings/internal/cordz_info.cc

@@ -14,6 +14,8 @@
 
 #include "absl/strings/internal/cordz_info.h"
 
+#include <cstdint>
+
 #include "absl/base/config.h"
 #include "absl/base/internal/spinlock.h"
 #include "absl/container/inlined_vector.h"
@@ -247,10 +249,12 @@ CordzInfo* CordzInfo::Next(const CordzSnapshot& snapshot) const {
   return next;
 }
 
-void CordzInfo::TrackCord(InlineData& cord, MethodIdentifier method) {
+void CordzInfo::TrackCord(InlineData& cord, MethodIdentifier method,
+                          int64_t sampling_stride) {
   assert(cord.is_tree());
   assert(!cord.is_profiled());
-  CordzInfo* cordz_info = new CordzInfo(cord.as_tree(), nullptr, method);
+  CordzInfo* cordz_info =
+      new CordzInfo(cord.as_tree(), nullptr, method, sampling_stride);
   cord.set_cordz_info(cordz_info);
   cordz_info->Track();
 }
@@ -266,7 +270,8 @@ void CordzInfo::TrackCord(InlineData& cord, const InlineData& src,
   if (cordz_info != nullptr) cordz_info->Untrack();
 
   // Start new cord sample
-  cordz_info = new CordzInfo(cord.as_tree(), src.cordz_info(), method);
+  cordz_info = new CordzInfo(cord.as_tree(), src.cordz_info(), method,
+                             src.cordz_info()->sampling_stride());
   cord.set_cordz_info(cordz_info);
   cordz_info->Track();
 }
@@ -298,9 +303,8 @@ size_t CordzInfo::FillParentStack(const CordzInfo* src, void** stack) {
   return src->stack_depth_;
 }
 
-CordzInfo::CordzInfo(CordRep* rep,
-                     const CordzInfo* src,
-                     MethodIdentifier method)
+CordzInfo::CordzInfo(CordRep* rep, const CordzInfo* src,
+                     MethodIdentifier method, int64_t sampling_stride)
     : rep_(rep),
       stack_depth_(
           static_cast<size_t>(absl::GetStackTrace(stack_,
@@ -309,7 +313,8 @@ CordzInfo::CordzInfo(CordRep* rep,
       parent_stack_depth_(FillParentStack(src, parent_stack_)),
       method_(method),
       parent_method_(GetParentMethod(src)),
-      create_time_(absl::Now()) {
+      create_time_(absl::Now()),
+      sampling_stride_(sampling_stride) {
   update_tracker_.LossyAdd(method);
   if (src) {
     // Copy parent counters.

absl/strings/internal/cordz_info.h

@@ -60,7 +60,8 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
   // and/or deleted. `method` identifies the Cord public API method initiating
   // the cord to be sampled.
   // Requires `cord` to hold a tree, and `cord.cordz_info()` to be null.
-  static void TrackCord(InlineData& cord, MethodIdentifier method);
+  static void TrackCord(InlineData& cord, MethodIdentifier method,
+                        int64_t sampling_stride);
 
   // Identical to TrackCord(), except that this function fills the
   // `parent_stack` and `parent_method` properties of the returned CordzInfo
@@ -181,6 +182,8 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
   // or RemovePrefix.
   CordzStatistics GetCordzStatistics() const;
 
+  int64_t sampling_stride() const { return sampling_stride_; }
+
  private:
   using SpinLock = absl::base_internal::SpinLock;
   using SpinLockHolder = ::absl::base_internal::SpinLockHolder;
@@ -199,7 +202,7 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
   static constexpr size_t kMaxStackDepth = 64;
 
   explicit CordzInfo(CordRep* rep, const CordzInfo* src,
-                     MethodIdentifier method);
+                     MethodIdentifier method, int64_t weight);
   ~CordzInfo() override;
 
   // Sets `rep_` without holding a lock.
@@ -250,12 +253,14 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
   const MethodIdentifier parent_method_;
   CordzUpdateTracker update_tracker_;
   const absl::Time create_time_;
+  const int64_t sampling_stride_;
 };
 
 inline ABSL_ATTRIBUTE_ALWAYS_INLINE void CordzInfo::MaybeTrackCord(
     InlineData& cord, MethodIdentifier method) {
-  if (ABSL_PREDICT_FALSE(cordz_should_profile())) {
-    TrackCord(cord, method);
+  auto stride = cordz_should_profile();
+  if (ABSL_PREDICT_FALSE(stride > 0)) {
+    TrackCord(cord, method, stride);
   }
 }
 

absl/strings/internal/cordz_info_statistics_test.cc

@@ -152,7 +152,7 @@ size_t FairShare(CordRep* rep, size_t ref = 1) {
 // Samples the cord and returns CordzInfo::GetStatistics()
 CordzStatistics SampleCord(CordRep* rep) {
   InlineData cord(rep);
-  CordzInfo::TrackCord(cord, CordzUpdateTracker::kUnknown);
+  CordzInfo::TrackCord(cord, CordzUpdateTracker::kUnknown, 1);
   CordzStatistics stats = cord.cordz_info()->GetCordzStatistics();
   cord.cordz_info()->Untrack();
   return stats;
@@ -480,7 +480,7 @@ TEST(CordzInfoStatisticsTest, ThreadSafety) {
 
                 // 50/50 sample
                 if (coin_toss(gen) != 0) {
-                  CordzInfo::TrackCord(cord, CordzUpdateTracker::kUnknown);
+                  CordzInfo::TrackCord(cord, CordzUpdateTracker::kUnknown, 1);
                 }
               }
             }