From bb2777cec91850366ce8d5764ee71d134bdcea03 Mon Sep 17 00:00:00 2001 From: Brandon Allard Date: Tue, 29 Aug 2023 21:48:19 -0400 Subject: [PATCH 1/2] tests/cpu_profiler: add tests that ensure correct behavior during exceptions --- tests/unit/cpu_profiler_test.cc | 72 +++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/tests/unit/cpu_profiler_test.cc b/tests/unit/cpu_profiler_test.cc index ac678197b91..fd3df7a73ff 100644 --- a/tests/unit/cpu_profiler_test.cc +++ b/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 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 results; + engine().profiler_results(results); + BOOST_REQUIRE(results.size() > 0); +} From 6c8105f700ea12b68dd12d3cd56cf5a9ce8c9638 Mon Sep 17 00:00:00 2001 From: Brandon Allard Date: Sun, 27 Aug 2023 20:28:08 -0400 Subject: [PATCH 2/2] core/cpu_profiler: finer-grain stats for why a sample was dropped --- include/seastar/core/internal/cpu_profiler.hh | 20 ++++++++++++++++++- src/core/cpu_profiler.cc | 14 ++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/include/seastar/core/internal/cpu_profiler.hh b/include/seastar/core/internal/cpu_profiler.hh index 6ae638f7c6c..11f5f1eec88 100644 --- a/include/seastar/core/internal/cpu_profiler.hh +++ b/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 _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}; diff --git a/src/core/cpu_profiler.cc b/src/core/cpu_profiler.cc index d45625bd8ec..6a04b07b8af 100644 --- a/src/core/cpu_profiler.cc +++ b/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& 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) {