From 42df2379883ffc21f9dfa36171397ca8fa818d5a Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Fri, 6 Dec 2024 22:48:17 -0800 Subject: [PATCH 1/9] Extract periodic thread into own file --- ext/vernier/memory.cc | 57 +------------------------------ ext/vernier/periodic_thread.hh | 61 ++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 56 deletions(-) create mode 100644 ext/vernier/periodic_thread.hh diff --git a/ext/vernier/memory.cc b/ext/vernier/memory.cc index 565e689..d55f9b8 100644 --- a/ext/vernier/memory.cc +++ b/ext/vernier/memory.cc @@ -6,6 +6,7 @@ #include "vernier.hh" #include "timestamp.hh" +#include "periodic_thread.hh" #if defined(__APPLE__) @@ -60,62 +61,6 @@ static VALUE rb_memory_rss(VALUE self) { return ULL2NUM(memory_rss()); } -class PeriodicThread { - std::atomic running; - pthread_t pthread; - TimeStamp interval; - - public: - PeriodicThread() : interval(TimeStamp::from_milliseconds(10)) { - } - - void set_interval(TimeStamp timestamp) { - interval = timestamp; - } - - static void *thread_entrypoint(void *arg) { - static_cast(arg)->run(); - return NULL; - } - - void run() { - TimeStamp next_sample_schedule = TimeStamp::Now(); - while (running) { - TimeStamp sample_complete = TimeStamp::Now(); - - run_iteration(); - - next_sample_schedule += interval; - - if (next_sample_schedule < sample_complete) { - next_sample_schedule = sample_complete + interval; - } - - TimeStamp::SleepUntil(next_sample_schedule); - } - } - - virtual void run_iteration() = 0; - - void start() { - if (running) return; - - running = true; - - int ret = pthread_create(&pthread, NULL, &thread_entrypoint, this); - if (ret != 0) { - perror("pthread_create"); - rb_bug("VERNIER: pthread_create failed"); - } - } - - void stop() { - if (!running) return; - - running = false; - } -}; - class MemoryTracker : public PeriodicThread { public: struct Record { diff --git a/ext/vernier/periodic_thread.hh b/ext/vernier/periodic_thread.hh new file mode 100644 index 0000000..15fee28 --- /dev/null +++ b/ext/vernier/periodic_thread.hh @@ -0,0 +1,61 @@ +#include "ruby.h" + +#include +#include "timestamp.hh" + +class PeriodicThread { + std::atomic running; + pthread_t pthread; + TimeStamp interval; + + public: + PeriodicThread() : interval(TimeStamp::from_milliseconds(10)) { + } + + void set_interval(TimeStamp timestamp) { + interval = timestamp; + } + + static void *thread_entrypoint(void *arg) { + static_cast(arg)->run(); + return NULL; + } + + void run() { + TimeStamp next_sample_schedule = TimeStamp::Now(); + while (running) { + TimeStamp sample_complete = TimeStamp::Now(); + + run_iteration(); + + next_sample_schedule += interval; + + if (next_sample_schedule < sample_complete) { + next_sample_schedule = sample_complete + interval; + } + + TimeStamp::SleepUntil(next_sample_schedule); + } + } + + virtual void run_iteration() = 0; + + void start() { + if (running) return; + + running = true; + + int ret = pthread_create(&pthread, NULL, &thread_entrypoint, this); + if (ret != 0) { + perror("pthread_create"); + rb_bug("VERNIER: pthread_create failed"); + } + } + + void stop() { + if (!running) return; + + running = false; + } +}; + From 3f7eb1d4d9b0236c5e0cb6dca1fb28d854cbe8fb Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Fri, 6 Dec 2024 23:06:00 -0800 Subject: [PATCH 2/9] Split into run_iteration --- ext/vernier/vernier.cc | 74 ++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/ext/vernier/vernier.cc b/ext/vernier/vernier.cc index e087656..961179c 100644 --- a/ext/vernier/vernier.cc +++ b/ext/vernier/vernier.cc @@ -1641,48 +1641,52 @@ class TimeCollector : public BaseCollector { } } - void sample_thread_run() { - LiveSample sample; + void run_iteration() { + TimeStamp sample_start = TimeStamp::Now(); - TimeStamp next_sample_schedule = TimeStamp::Now(); - while (running) { - TimeStamp sample_start = TimeStamp::Now(); + LiveSample sample; - threads.mutex.lock(); - for (auto &threadptr : threads.list) { - auto &thread = *threadptr; + threads.mutex.lock(); + for (auto &threadptr : threads.list) { + auto &thread = *threadptr; - //if (thread.state == Thread::State::RUNNING) { - //if (thread.state == Thread::State::RUNNING || (thread.state == Thread::State::SUSPENDED && thread.stack_on_suspend_idx < 0)) { - if (thread.state == Thread::State::RUNNING) { - //fprintf(stderr, "sampling %p on tid:%i\n", thread.ruby_thread, thread.native_tid); - bool signal_sent = GlobalSignalHandler::get_instance()->record_sample(sample, thread.pthread_id); - - if (!signal_sent) { - // The thread has died. We probably should have caught - // that by the GVL instrumentation, but let's try to get - // it to a consistent state and stop profiling it. - thread.set_state(Thread::State::STOPPED); - } else if (sample.sample.empty()) { - // fprintf(stderr, "skipping GC sample\n"); - } else { - record_sample(sample.sample, sample_start, thread, CATEGORY_NORMAL); - } - } else if (thread.state == Thread::State::SUSPENDED) { - thread.samples.record_sample( - thread.stack_on_suspend_idx, - sample_start, - CATEGORY_IDLE); - } else if (thread.state == Thread::State::READY) { - thread.samples.record_sample( - thread.stack_on_suspend_idx, - sample_start, - CATEGORY_STALLED); + //if (thread.state == Thread::State::RUNNING) { + //if (thread.state == Thread::State::RUNNING || (thread.state == Thread::State::SUSPENDED && thread.stack_on_suspend_idx < 0)) { + if (thread.state == Thread::State::RUNNING) { + //fprintf(stderr, "sampling %p on tid:%i\n", thread.ruby_thread, thread.native_tid); + bool signal_sent = GlobalSignalHandler::get_instance()->record_sample(sample, thread.pthread_id); + + if (!signal_sent) { + // The thread has died. We probably should have caught + // that by the GVL instrumentation, but let's try to get + // it to a consistent state and stop profiling it. + thread.set_state(Thread::State::STOPPED); + } else if (sample.sample.empty()) { + // fprintf(stderr, "skipping GC sample\n"); } else { + record_sample(sample.sample, sample_start, thread, CATEGORY_NORMAL); } + } else if (thread.state == Thread::State::SUSPENDED) { + thread.samples.record_sample( + thread.stack_on_suspend_idx, + sample_start, + CATEGORY_IDLE); + } else if (thread.state == Thread::State::READY) { + thread.samples.record_sample( + thread.stack_on_suspend_idx, + sample_start, + CATEGORY_STALLED); + } else { } + } - threads.mutex.unlock(); + threads.mutex.unlock(); + } + + void sample_thread_run() { + TimeStamp next_sample_schedule = TimeStamp::Now(); + while (running) { + run_iteration(); TimeStamp sample_complete = TimeStamp::Now(); From 418aabc5430f10c8292f24bb1ecb0d027e1d2203 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Fri, 6 Dec 2024 23:14:01 -0800 Subject: [PATCH 3/9] Extract signal safe semaphore --- ext/vernier/signal_safe_semaphore.hh | 56 ++++++++++++++++++++++++++++ ext/vernier/vernier.cc | 54 +-------------------------- 2 files changed, 58 insertions(+), 52 deletions(-) create mode 100644 ext/vernier/signal_safe_semaphore.hh diff --git a/ext/vernier/signal_safe_semaphore.hh b/ext/vernier/signal_safe_semaphore.hh new file mode 100644 index 0000000..7c8e927 --- /dev/null +++ b/ext/vernier/signal_safe_semaphore.hh @@ -0,0 +1,56 @@ +#ifndef SIGNAL_SAFE_SEMAPHORE_HH +#define SIGNAL_SAFE_SEMAPHORE_HH + +// A basic semaphore built on sem_wait/sem_post +// post() is guaranteed to be async-signal-safe +class SignalSafeSemaphore { +#ifdef __APPLE__ + dispatch_semaphore_t sem; +#else + sem_t sem; +#endif + + public: + + SignalSafeSemaphore(unsigned int value = 0) { +#ifdef __APPLE__ + sem = dispatch_semaphore_create(value); +#else + sem_init(&sem, 0, value); +#endif + }; + + ~SignalSafeSemaphore() { +#ifdef __APPLE__ + dispatch_release(sem); +#else + sem_destroy(&sem); +#endif + }; + + void wait() { +#ifdef __APPLE__ + dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER); +#else + // Use sem_timedwait so that we get a crash instead of a deadlock for + // easier debugging + auto ts = (TimeStamp::Now() + TimeStamp::from_seconds(5)).timespec(); + + int ret; + do { + ret = sem_wait(&sem); + } while (ret && errno == EINTR); + assert(ret == 0); +#endif + } + + void post() { +#ifdef __APPLE__ + dispatch_semaphore_signal(sem); +#else + sem_post(&sem); +#endif + } +}; + +#endif diff --git a/ext/vernier/vernier.cc b/ext/vernier/vernier.cc index 961179c..cd47581 100644 --- a/ext/vernier/vernier.cc +++ b/ext/vernier/vernier.cc @@ -29,6 +29,8 @@ #include "vernier.hh" #include "timestamp.hh" +#include "periodic_thread.hh" +#include "signal_safe_semaphore.hh" #include "ruby/ruby.h" #include "ruby/encoding.h" @@ -148,58 +150,6 @@ namespace std { }; } -// A basic semaphore built on sem_wait/sem_post -// post() is guaranteed to be async-signal-safe -class SignalSafeSemaphore { -#ifdef __APPLE__ - dispatch_semaphore_t sem; -#else - sem_t sem; -#endif - - public: - - SignalSafeSemaphore(unsigned int value = 0) { -#ifdef __APPLE__ - sem = dispatch_semaphore_create(value); -#else - sem_init(&sem, 0, value); -#endif - }; - - ~SignalSafeSemaphore() { -#ifdef __APPLE__ - dispatch_release(sem); -#else - sem_destroy(&sem); -#endif - }; - - void wait() { -#ifdef __APPLE__ - dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER); -#else - // Use sem_timedwait so that we get a crash instead of a deadlock for - // easier debugging - auto ts = (TimeStamp::Now() + TimeStamp::from_seconds(5)).timespec(); - - int ret; - do { - ret = sem_wait(&sem); - } while (ret && errno == EINTR); - assert(ret == 0); -#endif - } - - void post() { -#ifdef __APPLE__ - dispatch_semaphore_signal(sem); -#else - sem_post(&sem); -#endif - } -}; - class RawSample { public: From 9a51408bbd26768650ef7cee4730284303c01a67 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Sat, 7 Dec 2024 00:09:22 -0800 Subject: [PATCH 4/9] Use PeriodicThread for sampling --- ext/vernier/memory.cc | 4 +- ext/vernier/periodic_thread.hh | 13 ++++- ext/vernier/signal_safe_semaphore.hh | 13 +++++ ext/vernier/vernier.cc | 71 +++++++--------------------- 4 files changed, 45 insertions(+), 56 deletions(-) diff --git a/ext/vernier/memory.cc b/ext/vernier/memory.cc index d55f9b8..8177d94 100644 --- a/ext/vernier/memory.cc +++ b/ext/vernier/memory.cc @@ -1,4 +1,3 @@ -#include #include #include #include @@ -70,6 +69,9 @@ class MemoryTracker : public PeriodicThread { std::vector results; std::mutex mutex; + MemoryTracker() : PeriodicThread(TimeStamp::from_milliseconds(10)) { + } + void run_iteration() { record(); } diff --git a/ext/vernier/periodic_thread.hh b/ext/vernier/periodic_thread.hh index 15fee28..6a99149 100644 --- a/ext/vernier/periodic_thread.hh +++ b/ext/vernier/periodic_thread.hh @@ -4,12 +4,12 @@ #include "timestamp.hh" class PeriodicThread { - std::atomic running; + std::atomic_bool running; pthread_t pthread; TimeStamp interval; public: - PeriodicThread() : interval(TimeStamp::from_milliseconds(10)) { + PeriodicThread(TimeStamp interval) : interval(interval), running(false) { } void set_interval(TimeStamp timestamp) { @@ -22,6 +22,14 @@ class PeriodicThread { } void run() { +#if HAVE_PTHREAD_SETNAME_NP +#ifdef __APPLE__ + pthread_setname_np("Vernier profiler"); +#else + pthread_setname_np(pthread_self(), "Vernier profiler"); +#endif +#endif + TimeStamp next_sample_schedule = TimeStamp::Now(); while (running) { TimeStamp sample_complete = TimeStamp::Now(); @@ -56,6 +64,7 @@ class PeriodicThread { if (!running) return; running = false; + pthread_join(pthread, NULL); } }; diff --git a/ext/vernier/signal_safe_semaphore.hh b/ext/vernier/signal_safe_semaphore.hh index 7c8e927..78377d8 100644 --- a/ext/vernier/signal_safe_semaphore.hh +++ b/ext/vernier/signal_safe_semaphore.hh @@ -1,6 +1,19 @@ #ifndef SIGNAL_SAFE_SEMAPHORE_HH #define SIGNAL_SAFE_SEMAPHORE_HH +#if defined(__APPLE__) +/* macOS */ +#include +#elif defined(__FreeBSD__) +/* FreeBSD */ +#include +#include +#else +/* Linux */ +#include +#include /* for SYS_gettid */ +#endif + // A basic semaphore built on sem_wait/sem_post // post() is guaranteed to be async-signal-safe class SignalSafeSemaphore { diff --git a/ext/vernier/vernier.cc b/ext/vernier/vernier.cc index cd47581..e018d1e 100644 --- a/ext/vernier/vernier.cc +++ b/ext/vernier/vernier.cc @@ -14,18 +14,6 @@ #include #include -#if defined(__APPLE__) -/* macOS */ -#include -#elif defined(__FreeBSD__) -/* FreeBSD */ -#include -#include -#else -/* Linux */ -#include -#include /* for SYS_gettid */ -#endif #include "vernier.hh" #include "timestamp.hh" @@ -1514,6 +1502,19 @@ class GlobalSignalHandler { LiveSample *GlobalSignalHandler::live_sample; class TimeCollector : public BaseCollector { + class TimeCollectorThread : public PeriodicThread { + TimeCollector &time_collector; + + void run_iteration() { + time_collector.run_iteration(); + } + + public: + + TimeCollectorThread(TimeCollector &tc, TimeStamp interval) : PeriodicThread(interval), time_collector(tc) { + }; + }; + GCMarkerTable gc_markers; ThreadTable threads; @@ -1536,8 +1537,10 @@ class TimeCollector : public BaseCollector { collector->record_newobj(obj); } + TimeCollectorThread collector_thread; + public: - TimeCollector(VALUE stack_table, TimeStamp interval, unsigned int allocation_interval) : BaseCollector(stack_table), interval(interval), allocation_interval(allocation_interval), threads(*get_stack_table(stack_table)) { + TimeCollector(VALUE stack_table, TimeStamp interval, unsigned int allocation_interval) : BaseCollector(stack_table), interval(interval), allocation_interval(allocation_interval), threads(*get_stack_table(stack_table)), collector_thread(*this, interval) { } void record_newobj(VALUE obj) { @@ -1631,39 +1634,6 @@ class TimeCollector : public BaseCollector { } threads.mutex.unlock(); - } - - void sample_thread_run() { - TimeStamp next_sample_schedule = TimeStamp::Now(); - while (running) { - run_iteration(); - - TimeStamp sample_complete = TimeStamp::Now(); - - next_sample_schedule += interval; - - // If sampling falls behind, restart, and check in another interval - if (next_sample_schedule < sample_complete) { - next_sample_schedule = sample_complete + interval; - } - - TimeStamp::SleepUntil(next_sample_schedule); - } - - thread_stopped.post(); - } - - static void *sample_thread_entry(void *arg) { -#if HAVE_PTHREAD_SETNAME_NP -#ifdef __APPLE__ - pthread_setname_np("Vernier profiler"); -#else - pthread_setname_np(pthread_self(), "Vernier profiler"); -#endif -#endif - TimeCollector *collector = static_cast(arg); - collector->sample_thread_run(); - return NULL; } static void internal_thread_event_cb(rb_event_flag_t event, VALUE data, VALUE self, ID mid, VALUE klass) { @@ -1766,11 +1736,7 @@ class TimeCollector : public BaseCollector { running = true; - int ret = pthread_create(&sample_thread, NULL, &sample_thread_entry, this); - if (ret != 0) { - perror("pthread_create"); - rb_bug("VERNIER: pthread_create failed"); - } + collector_thread.start(); // Set the state of the current Ruby thread to RUNNING, which we know it // is as it must have held the GVL to start the collector. We want to @@ -1789,8 +1755,7 @@ class TimeCollector : public BaseCollector { VALUE stop() { BaseCollector::stop(); - running = false; - thread_stopped.wait(); + collector_thread.stop(); GlobalSignalHandler::get_instance()->uninstall(); From cade52846554088fb9533432412f340e62f38bf0 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Sat, 7 Dec 2024 00:48:00 -0800 Subject: [PATCH 5/9] Fail if we hang on a semaphore --- ext/vernier/signal_safe_semaphore.hh | 7 +++++-- ext/vernier/timestamp.hh | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/ext/vernier/signal_safe_semaphore.hh b/ext/vernier/signal_safe_semaphore.hh index 78377d8..8986d74 100644 --- a/ext/vernier/signal_safe_semaphore.hh +++ b/ext/vernier/signal_safe_semaphore.hh @@ -47,12 +47,15 @@ class SignalSafeSemaphore { #else // Use sem_timedwait so that we get a crash instead of a deadlock for // easier debugging - auto ts = (TimeStamp::Now() + TimeStamp::from_seconds(5)).timespec(); + struct timespec ts = (TimeStamp::NowRealtime() + TimeStamp::from_seconds(5)).timespec(); int ret; do { - ret = sem_wait(&sem); + ret = sem_timedwait(&sem, &ts); } while (ret && errno == EINTR); + if (ret != 0) { + rb_bug("VERNIER: sem_timedwait waited over 5 seconds"); + } assert(ret == 0); #endif } diff --git a/ext/vernier/timestamp.hh b/ext/vernier/timestamp.hh index 46dd9fc..93f8a8a 100644 --- a/ext/vernier/timestamp.hh +++ b/ext/vernier/timestamp.hh @@ -21,6 +21,12 @@ class TimeStamp { return TimeStamp(ts.tv_sec * nanoseconds_per_second + ts.tv_nsec); } + static TimeStamp NowRealtime() { + struct timespec ts; + clock_gettime(CLOCK_REALTIME, &ts); + return TimeStamp(ts.tv_sec * nanoseconds_per_second + ts.tv_nsec); + } + static TimeStamp Zero() { return TimeStamp(0); } From 249351436c92255ef9e35a6f1879cb2210a6d59e Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Sat, 7 Dec 2024 00:59:43 -0800 Subject: [PATCH 6/9] Switch sleep condition to exit on condvar This allows us to cancel the periodic thread before waiting for the sampling interval. --- ext/vernier/periodic_thread.hh | 56 +++++++++++++++++++++++++--------- test/test_time_collector.rb | 13 ++++++++ 2 files changed, 54 insertions(+), 15 deletions(-) diff --git a/ext/vernier/periodic_thread.hh b/ext/vernier/periodic_thread.hh index 6a99149..5338dbd 100644 --- a/ext/vernier/periodic_thread.hh +++ b/ext/vernier/periodic_thread.hh @@ -4,12 +4,19 @@ #include "timestamp.hh" class PeriodicThread { - std::atomic_bool running; pthread_t pthread; TimeStamp interval; + pthread_mutex_t running_mutex = PTHREAD_MUTEX_INITIALIZER; + pthread_cond_t running_cv; + std::atomic_bool running; + public: PeriodicThread(TimeStamp interval) : interval(interval), running(false) { + pthread_condattr_t attr; + pthread_condattr_init(&attr); + pthread_condattr_setclock(&attr, CLOCK_MONOTONIC); + pthread_cond_init(&running_cv, &attr); } void set_interval(TimeStamp timestamp) { @@ -31,7 +38,8 @@ class PeriodicThread { #endif TimeStamp next_sample_schedule = TimeStamp::Now(); - while (running) { + bool done = false; + while (!done) { TimeStamp sample_complete = TimeStamp::Now(); run_iteration(); @@ -42,29 +50,47 @@ class PeriodicThread { next_sample_schedule = sample_complete + interval; } - TimeStamp::SleepUntil(next_sample_schedule); + struct timespec next_sample_ts = next_sample_schedule.timespec(); + + pthread_mutex_lock(&running_mutex); + if (running) { + int ret; + do { + ret = pthread_cond_timedwait(&running_cv, &running_mutex, &next_sample_ts); + } while(running && ret == EINTR); + } + done = !running; + pthread_mutex_unlock(&running_mutex); } } virtual void run_iteration() = 0; void start() { - if (running) return; - - running = true; - - int ret = pthread_create(&pthread, NULL, &thread_entrypoint, this); - if (ret != 0) { - perror("pthread_create"); - rb_bug("VERNIER: pthread_create failed"); + pthread_mutex_lock(&running_mutex); + if (!running) { + running = true; + + int ret = pthread_create(&pthread, NULL, &thread_entrypoint, this); + if (ret != 0) { + perror("pthread_create"); + rb_bug("VERNIER: pthread_create failed"); + } } + pthread_mutex_unlock(&running_mutex); } void stop() { - if (!running) return; - - running = false; - pthread_join(pthread, NULL); + pthread_mutex_lock(&running_mutex); + bool was_running = running; + if (running) { + running = false; + pthread_cond_broadcast(&running_cv); + } + pthread_mutex_unlock(&running_mutex); + if (was_running) + pthread_join(pthread, NULL); + pthread = 0; } }; diff --git a/test/test_time_collector.rb b/test/test_time_collector.rb index 20d5893..d83360d 100644 --- a/test/test_time_collector.rb +++ b/test/test_time_collector.rb @@ -257,6 +257,19 @@ def test_stop_without_start assert_equal "profile not started", error.message end + def test_stops_in_reasonable_time + collector = Vernier::Collector.new(:wall, interval: 10_000_000) + collector.start + sleep 0.2 + time_before = Process.clock_gettime(Process::CLOCK_MONOTONIC) + collector.stop + time_after = Process.clock_gettime(Process::CLOCK_MONOTONIC) + time = time_after - time_before + + # Under one second + assert_operator time, :<, 1 + end + def test_includes_options_in_result_meta output_file = File.join(__dir__, "../tmp/exception_output.json") result = Vernier.profile( From 0f9fe93acbee2a8a0933dda216e87666a521a0dd Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Sat, 7 Dec 2024 16:41:07 -0800 Subject: [PATCH 7/9] macos does not have pthread_condattr_setclock --- ext/vernier/extconf.rb | 1 + ext/vernier/periodic_thread.hh | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ext/vernier/extconf.rb b/ext/vernier/extconf.rb index 246305b..2f00d4e 100644 --- a/ext/vernier/extconf.rb +++ b/ext/vernier/extconf.rb @@ -12,5 +12,6 @@ have_func("rb_profile_thread_frames", "ruby/debug.h") have_func("pthread_setname_np") +have_func("pthread_condattr_setclock") create_makefile("vernier/vernier") diff --git a/ext/vernier/periodic_thread.hh b/ext/vernier/periodic_thread.hh index 5338dbd..744b163 100644 --- a/ext/vernier/periodic_thread.hh +++ b/ext/vernier/periodic_thread.hh @@ -15,7 +15,9 @@ class PeriodicThread { PeriodicThread(TimeStamp interval) : interval(interval), running(false) { pthread_condattr_t attr; pthread_condattr_init(&attr); +#if HAVE_PTHREAD_CONDATTR_SETCLOCK pthread_condattr_setclock(&attr, CLOCK_MONOTONIC); +#endif pthread_cond_init(&running_cv, &attr); } @@ -50,10 +52,14 @@ class PeriodicThread { next_sample_schedule = sample_complete + interval; } - struct timespec next_sample_ts = next_sample_schedule.timespec(); - pthread_mutex_lock(&running_mutex); if (running) { +#if HAVE_PTHREAD_CONDATTR_SETCLOCK + struct timespec next_sample_ts = next_sample_schedule.timespec(); +#else + auto offset = TimeStamp::NowRealtime() - TimeStamp::Now(); + struct timespec next_sample_ts = (next_sample_schedule + offset).timespec(); +#endif int ret; do { ret = pthread_cond_timedwait(&running_cv, &running_mutex, &next_sample_ts); From 8b5263205d6d3b66b6f5e9a0cd266c12db113418 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Sun, 8 Dec 2024 23:14:29 -0800 Subject: [PATCH 8/9] Set thread policy on macos --- ext/vernier/periodic_thread.hh | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/ext/vernier/periodic_thread.hh b/ext/vernier/periodic_thread.hh index 744b163..72c6a79 100644 --- a/ext/vernier/periodic_thread.hh +++ b/ext/vernier/periodic_thread.hh @@ -3,6 +3,43 @@ #include #include "timestamp.hh" +#ifdef __APPLE__ + +#include +#include +#include + +// https://developer.apple.com/library/archive/technotes/tn2169/_index.html +inline void upgrade_thread_priority(pthread_t pthread) { + mach_timebase_info_data_t timebase_info; + mach_timebase_info(&timebase_info); + + const uint64_t NANOS_PER_MSEC = 1000000ULL; + double clock2abs = ((double)timebase_info.denom / (double)timebase_info.numer) * NANOS_PER_MSEC; + + thread_time_constraint_policy_data_t policy; + policy.period = 0; + + // FIXME: I really don't know what these value should be + policy.computation = (uint32_t)(5 * clock2abs); // 5 ms of work + policy.constraint = (uint32_t)(10 * clock2abs); + policy.preemptible = FALSE; + + int kr = thread_policy_set(pthread_mach_thread_np(pthread_self()), + THREAD_TIME_CONSTRAINT_POLICY, + (thread_policy_t)&policy, + THREAD_TIME_CONSTRAINT_POLICY_COUNT); + + if (kr != KERN_SUCCESS) { + mach_error("thread_policy_set:", kr); + exit(1); + } +} +#else +inline void upgrade_thread_priority(pthread_t pthread) { +} +#endif + class PeriodicThread { pthread_t pthread; TimeStamp interval; @@ -26,6 +63,8 @@ class PeriodicThread { } static void *thread_entrypoint(void *arg) { + upgrade_thread_priority(pthread_self()); + static_cast(arg)->run(); return NULL; } From f8b779338ab5842ba6745bf8f749266ec9b7a876 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Sun, 8 Dec 2024 23:02:58 -0800 Subject: [PATCH 9/9] Give more leeway to thread allocations --- test/test_allocations.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_allocations.rb b/test/test_allocations.rb index 6d15f0f..bfd1ccf 100644 --- a/test/test_allocations.rb +++ b/test/test_allocations.rb @@ -49,7 +49,7 @@ def test_sample_rate def test_thread_allocation result = Vernier.trace(allocation_interval: 1) do Thread.new do - 100.times do + 1000.times do Object.new end end.join @@ -62,6 +62,6 @@ def test_thread_allocation allocation_samples = other_thread.dig(:allocations, :samples) - assert_includes 100..130, allocation_samples.count + assert_includes 1000..1100, allocation_samples.count end end