CPU profiler fixes followup

include/seastar/core/internal/cpu_profiler.hh

@@ -76,6 +76,24 @@ struct cpu_profiler_config {
     std::chrono::nanoseconds period;
 };
 
+struct cpu_profiler_stats {
+    unsigned dropped_samples_from_exceptions{0};
+    unsigned dropped_samples_from_buffer_full{0};
+    unsigned dropped_samples_from_mutex_contention{0};
+
+    void clear_dropped() {
+        dropped_samples_from_exceptions = 0;
+        dropped_samples_from_buffer_full = 0;
+        dropped_samples_from_mutex_contention = 0;
+    }
+
+    unsigned sum_dropped() const {
+        return dropped_samples_from_buffer_full
+            + dropped_samples_from_exceptions
+            + dropped_samples_from_mutex_contention;
+    }
+};
+
 class cpu_profiler {
 private:
     circular_buffer_fixed_capacity<cpu_profiler_trace, max_number_of_traces> _traces;
@@ -85,7 +103,7 @@ private:
     signal_mutex _traces_mutex;
     cpu_profiler_config _cfg;
     std::chrono::nanoseconds _last_set_timeout;
-    size_t _dropped_samples{0};
+    cpu_profiler_stats _stats;
     bool _is_stopped{true};
 
 

src/core/cpu_profiler.cc

@@ -134,6 +134,9 @@ void cpu_profiler::on_signal() {
     // Note: this only protects against C++ exceptions, therefore foreign
     // exceptions or long jumps could still cause segfaults within the profiler.
     const bool no_uncaught_exceptions = std::uncaught_exceptions() == 0;
+    if(!no_uncaught_exceptions) {
+        _stats.dropped_samples_from_exceptions++;
+    }
 
     // Skip the sample if the main thread is currently reading
     // _traces. This case shouldn't happen often though.
@@ -144,7 +147,7 @@ void cpu_profiler::on_signal() {
         // this.
         if (_traces.size() == _traces.capacity()) {
             _traces.pop_front();
-            _dropped_samples++;
+            _stats.dropped_samples_from_buffer_full++;
         }
         _traces.emplace_back();
         _traces.back().user_backtrace = current_backtrace_tasklocal();
@@ -159,6 +162,8 @@ void cpu_profiler::on_signal() {
                 }
             });
         }
+    } else {
+        _stats.dropped_samples_from_mutex_contention++;
     }
 
     auto next = get_next_timeout();
@@ -176,8 +181,11 @@ size_t cpu_profiler::results(std::vector<cpu_profiler_trace>& results_buffer) {
 
     results_buffer.assign(_traces.cbegin(), _traces.cend());
     _traces.clear();
-
-    return std::exchange(_dropped_samples, 0);
+
+    auto dropped_samples = _stats.sum_dropped();
+    _stats.clear_dropped();
+
+    return dropped_samples;
 }
 
 void cpu_profiler_posix_timer::arm_timer(std::chrono::nanoseconds ns) {

tests/unit/cpu_profiler_test.cc

@@ -174,3 +174,75 @@ SEASTAR_THREAD_TEST_CASE(signal_mutex_basic) {
     auto guard_opt_3 = mutex.try_lock();
     BOOST_REQUIRE(guard_opt_3.has_value());
 }
+
+namespace {
+void random_exception_catcher(int p, int a);
+
+[[gnu::noinline]] void
+random_exception_thrower(int a) {
+  static thread_local std::random_device rd;
+  static thread_local std::mt19937 gen(rd());
+  std::uniform_int_distribution<> d(1, 100);
+
+  a -= 1;
+
+  if (a <= 0) {
+    throw std::invalid_argument("noop");
+  }
+
+  random_exception_catcher(d(gen), a);
+}
+
+[[gnu::noinline]] void random_exception_catcher(int p, int a) {
+  static thread_local std::random_device rd;
+  static thread_local std::mt19937 gen(rd());
+  std::uniform_int_distribution<> d(1, 100);
+
+  try {
+    random_exception_thrower(a);
+  } catch (...) {
+    int r = d(gen);
+    if (r > p) {
+      throw;
+    }
+  }
+}
+
+} // namespace
+
+SEASTAR_THREAD_TEST_CASE(exception_handler_case) {
+  // Ensure that exception unwinding doesn't cause any issues
+  // while profiling.
+  temporary_profiler_settings cp{true, 10us};
+
+  std::random_device rd;
+  std::mt19937 gen(rd());
+  std::uniform_int_distribution<> d(1, 100);
+  for (int a = 0; a < 100; a++) {
+    random_exception_catcher(100, d(gen));
+  }
+
+  std::vector<cpu_profiler_trace> results;
+  auto dropped_samples = engine().profiler_results(results);
+  BOOST_REQUIRE(results.size() > 0);
+  BOOST_REQUIRE(dropped_samples > 0);
+}
+
+SEASTAR_THREAD_TEST_CASE(config_thrashing) {
+  // Ensure that fast config changes leave the profiler in a valid
+  // state.
+  temporary_profiler_settings cp{true, 10us};
+
+  std::random_device rd;
+  std::mt19937 gen(rd());
+  std::uniform_int_distribution<> d(1, 100);
+
+  for (int a = 0; a < 100; a++) {
+    int r = d(gen);
+    temporary_profiler_settings cp_0{r % 2 == 0, std::chrono::microseconds(r)};
+  }
+
+  std::vector<cpu_profiler_trace> results;
+  engine().profiler_results(results);
+  BOOST_REQUIRE(results.size() > 0);
+}