From 9ca41e999159096cb0872b4babbb94bf5d1af4ce Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 9 Nov 2023 13:56:29 +0100 Subject: GVL Instrumentation: pass thread->self as part of event data Context: https://github.com/ivoanjo/gvl-tracing/pull/4 Some hooks may want to collect data on a per thread basis. Right now the only way to identify the concerned thread is to use `rb_nativethread_self()` or similar, but even then because of the thread cache or MaNy, two distinct Ruby threads may report the same native thread id. By passing `thread->self`, hooks can use it as a key to store the metadata. NB: Most hooks are executed outside the GVL, so such data collection need to use a thread-safe data-structure, and shouldn't use the reference in other ways from inside the hook. They must also either pin that value or handle compaction. --- .../thread/instrumentation/instrumentation.c | 19 +++++++++++++++ include/ruby/thread.h | 4 +++- test/-ext-/thread/test_instrumentation_api.rb | 7 ++++++ thread.c | 2 +- thread_pthread.c | 27 ++++++++++++---------- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/ext/-test-/thread/instrumentation/instrumentation.c b/ext/-test-/thread/instrumentation/instrumentation.c index edb8738a29..4d76f4cbb3 100644 --- a/ext/-test-/thread/instrumentation/instrumentation.c +++ b/ext/-test-/thread/instrumentation/instrumentation.c @@ -16,11 +16,14 @@ static RB_THREAD_LOCAL_SPECIFIER unsigned int local_ready_count = 0; static RB_THREAD_LOCAL_SPECIFIER unsigned int local_resumed_count = 0; static RB_THREAD_LOCAL_SPECIFIER unsigned int local_suspended_count = 0; +static VALUE last_thread = Qnil; + static void ex_callback(rb_event_flag_t event, const rb_internal_thread_event_data_t *event_data, void *user_data) { switch (event) { case RUBY_INTERNAL_THREAD_EVENT_STARTED: + last_thread = event_data->thread; RUBY_ATOMIC_INC(started_count); break; case RUBY_INTERNAL_THREAD_EVENT_READY: @@ -122,15 +125,31 @@ thread_register_and_unregister_callback(VALUE thread) return Qtrue; } +static VALUE +thread_last_spawned(VALUE mod) +{ + return last_thread; +} + +static VALUE +thread_set_last_spawned(VALUE mod, VALUE value) +{ + return last_thread = value; +} + void Init_instrumentation(void) { VALUE mBug = rb_define_module("Bug"); VALUE klass = rb_define_module_under(mBug, "ThreadInstrumentation"); + rb_global_variable(&last_thread); rb_define_singleton_method(klass, "counters", thread_counters, 0); rb_define_singleton_method(klass, "local_counters", thread_local_counters, 0); rb_define_singleton_method(klass, "reset_counters", thread_reset_counters, 0); rb_define_singleton_method(klass, "register_callback", thread_register_callback, 0); rb_define_singleton_method(klass, "unregister_callback", thread_unregister_callback, 0); rb_define_singleton_method(klass, "register_and_unregister_callbacks", thread_register_and_unregister_callback, 0); + + rb_define_singleton_method(klass, "last_spawned_thread", thread_last_spawned, 0); + rb_define_singleton_method(klass, "last_spawned_thread=", thread_set_last_spawned, 1); } diff --git a/include/ruby/thread.h b/include/ruby/thread.h index d6a543af91..f6eea65b70 100644 --- a/include/ruby/thread.h +++ b/include/ruby/thread.h @@ -227,7 +227,9 @@ void *rb_nogvl(void *(*func)(void *), void *data1, #define RUBY_INTERNAL_THREAD_EVENT_MASK 0xff /** All Thread events */ -typedef void rb_internal_thread_event_data_t; // for future extension. +typedef struct rb_internal_thread_event_data { + VALUE thread; +} rb_internal_thread_event_data_t; typedef void (*rb_internal_thread_event_callback)(rb_event_flag_t event, const rb_internal_thread_event_data_t *event_data, diff --git a/test/-ext-/thread/test_instrumentation_api.rb b/test/-ext-/thread/test_instrumentation_api.rb index dd620e7380..208d11de85 100644 --- a/test/-ext-/thread/test_instrumentation_api.rb +++ b/test/-ext-/thread/test_instrumentation_api.rb @@ -22,6 +22,7 @@ class TestThreadInstrumentation < Test::Unit::TestCase def teardown return if /mswin|mingw|bccwin/ =~ RUBY_PLATFORM Bug::ThreadInstrumentation::unregister_callback + Bug::ThreadInstrumentation.last_spawned_thread = nil end THREADS_COUNT = 3 @@ -68,6 +69,12 @@ class TestThreadInstrumentation < Test::Unit::TestCase assert Bug::ThreadInstrumentation::register_and_unregister_callbacks end + def test_thread_instrumentation_event_data + assert_nil Bug::ThreadInstrumentation.last_spawned_thread + thr = Thread.new{ }.join + assert_same thr, Bug::ThreadInstrumentation.last_spawned_thread + end + private def fib(n = 20) diff --git a/thread.c b/thread.c index 71e33b164a..094ff5c77f 100644 --- a/thread.c +++ b/thread.c @@ -5409,7 +5409,7 @@ Init_Thread(void) // thread_sched_to_running(sched, th); #ifdef RB_INTERNAL_THREAD_HOOK - RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_RESUMED); + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_RESUMED, th); #endif th->pending_interrupt_queue = rb_ary_hidden_new(0); diff --git a/thread_pthread.c b/thread_pthread.c index eaf9229a06..c7319678c4 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -265,8 +265,8 @@ rb_native_cond_timedwait(rb_nativethread_cond_t *cond, pthread_mutex_t *mutex, u // thread scheduling static rb_internal_thread_event_hook_t *rb_internal_thread_event_hooks = NULL; -static void rb_thread_execute_hooks(rb_event_flag_t event); -#define RB_INTERNAL_THREAD_HOOK(event) if (rb_internal_thread_event_hooks) { rb_thread_execute_hooks(event); } +static void rb_thread_execute_hooks(rb_event_flag_t event, rb_thread_t *th); +#define RB_INTERNAL_THREAD_HOOK(event, th) if (rb_internal_thread_event_hooks) { rb_thread_execute_hooks(event, th); } static rb_serial_t current_fork_gen = 1; /* We can't use GET_VM()->fork_gen */ @@ -781,7 +781,7 @@ thread_sched_to_ready_common(struct rb_thread_sched *sched, rb_thread_t *th, boo thread_sched_enq(sched, th); } - RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_READY); + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_READY, th); } // waiting -> ready @@ -876,7 +876,7 @@ thread_sched_wait_running_turn(struct rb_thread_sched *sched, rb_thread_t *th, b } // VM_ASSERT(ractor_sched_running_threads_contain_p(th->vm, th)); need locking - RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_RESUMED); + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_RESUMED, th); } // waiting -> ready -> running (locked) @@ -959,7 +959,7 @@ static void thread_sched_to_waiting_common0(struct rb_thread_sched *sched, rb_thread_t *th, bool to_dead) { if (rb_internal_thread_event_hooks) { - rb_thread_execute_hooks(RUBY_INTERNAL_THREAD_EVENT_SUSPENDED); + rb_thread_execute_hooks(RUBY_INTERNAL_THREAD_EVENT_SUSPENDED, th); } if (!to_dead) native_thread_dedicated_inc(th->vm, th->ractor, th->nt); @@ -975,7 +975,7 @@ static void thread_sched_to_dead_common(struct rb_thread_sched *sched, rb_thread_t *th) { RUBY_DEBUG_LOG("dedicated:%d", th->nt->dedicated); - RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_EXITED); + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_EXITED, th); thread_sched_to_waiting_common0(sched, th, true); } @@ -1007,7 +1007,7 @@ thread_sched_to_waiting_common(struct rb_thread_sched *sched, rb_thread_t *th) static void thread_sched_to_waiting(struct rb_thread_sched *sched, rb_thread_t *th) { - RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_SUSPENDED); + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_SUSPENDED, th); thread_sched_lock(sched, th); { @@ -2148,7 +2148,7 @@ native_thread_create_dedicated(rb_thread_t *th) static void call_thread_start_func_2(rb_thread_t *th) { - RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_STARTED); + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_STARTED, th); #if defined USE_NATIVE_THREAD_INIT native_thread_init_stack(th); @@ -3232,7 +3232,7 @@ static void native_sleep(rb_thread_t *th, rb_hrtime_t *rel) { struct rb_thread_sched *sched = TH_SCHED(th); - RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_SUSPENDED); + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_SUSPENDED, th); RUBY_DEBUG_LOG("rel:%d", rel ? (int)*rel : 0); if (rel) { @@ -3248,7 +3248,7 @@ native_sleep(rb_thread_t *th, rb_hrtime_t *rel) } RUBY_DEBUG_LOG("wakeup"); - RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_READY); + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_READY, th); } // thread internal event hooks (only for pthread) @@ -3322,7 +3322,7 @@ rb_internal_thread_remove_event_hook(rb_internal_thread_event_hook_t * hook) } static void -rb_thread_execute_hooks(rb_event_flag_t event) +rb_thread_execute_hooks(rb_event_flag_t event, rb_thread_t *th) { int r; if ((r = pthread_rwlock_rdlock(&rb_internal_thread_event_hooks_rw_lock))) { @@ -3333,7 +3333,10 @@ rb_thread_execute_hooks(rb_event_flag_t event) rb_internal_thread_event_hook_t *h = rb_internal_thread_event_hooks; do { if (h->event & event) { - (*h->callback)(event, NULL, h->user_data); + rb_internal_thread_event_data_t event_data = { + .thread = th->self, + }; + (*h->callback)(event, &event_data, h->user_data); } } while((h = h->next)); } -- cgit v1.2.3