ruby-changes:72208
From: Jean <ko1@a...>
Date: Fri, 17 Jun 2022 16:08:43 +0900 (JST)
Subject: [ruby-changes:72208] b6c1e1158d (master): GVL Instrumentation API: add STARTED and EXITED events
https://git.ruby-lang.org/ruby.git/commit/?id=b6c1e1158d From b6c1e1158d71b533b255ae7a2731598455918071 Mon Sep 17 00:00:00 2001 From: Jean Boussier <jean.boussier@g...> Date: Wed, 15 Jun 2022 14:37:41 +0200 Subject: GVL Instrumentation API: add STARTED and EXITED events [Feature #18339] After experimenting with the initial version of the API I figured there is a need for an exit event to cleanup instrumentation data. e.g. if you record data in a {thread_id -> data} table, you need to free associated data when a thread goes away. --- .../thread/instrumentation/instrumentation.c | 44 +++++++++++++++------- include/ruby/thread.h | 10 +++-- test/-ext-/thread/test_instrumentation_api.rb | 4 +- thread.c | 1 - thread_none.c | 2 + thread_pthread.c | 25 ++++++------ thread_win32.c | 2 + 7 files changed, 57 insertions(+), 31 deletions(-) diff --git a/ext/-test-/thread/instrumentation/instrumentation.c b/ext/-test-/thread/instrumentation/instrumentation.c index c298d76ad6..1251029327 100644 --- a/ext/-test-/thread/instrumentation/instrumentation.c +++ b/ext/-test-/thread/instrumentation/instrumentation.c @@ -2,22 +2,30 @@ https://github.com/ruby/ruby/blob/trunk/ext/-test-/thread/instrumentation/instrumentation.c#L2 #include "ruby/atomic.h" #include "ruby/thread.h" -static rb_atomic_t acquire_enter_count = 0; -static rb_atomic_t acquire_exit_count = 0; -static rb_atomic_t release_count = 0; +static rb_atomic_t started_count = 0; +static rb_atomic_t ready_count = 0; +static rb_atomic_t resumed_count = 0; +static rb_atomic_t suspended_count = 0; +static rb_atomic_t exited_count = 0; 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: + RUBY_ATOMIC_INC(started_count); + break; case RUBY_INTERNAL_THREAD_EVENT_READY: - RUBY_ATOMIC_INC(acquire_enter_count); + RUBY_ATOMIC_INC(ready_count); break; case RUBY_INTERNAL_THREAD_EVENT_RESUMED: - RUBY_ATOMIC_INC(acquire_exit_count); + RUBY_ATOMIC_INC(resumed_count); break; case RUBY_INTERNAL_THREAD_EVENT_SUSPENDED: - RUBY_ATOMIC_INC(release_count); + RUBY_ATOMIC_INC(suspended_count); + break; + case RUBY_INTERNAL_THREAD_EVENT_EXITED: + RUBY_ATOMIC_INC(exited_count); break; } } @@ -27,19 +35,23 @@ static rb_internal_thread_event_hook_t * single_hook = NULL; https://github.com/ruby/ruby/blob/trunk/ext/-test-/thread/instrumentation/instrumentation.c#L35 static VALUE thread_counters(VALUE thread) { - VALUE array = rb_ary_new2(3); - rb_ary_push(array, UINT2NUM(acquire_enter_count)); - rb_ary_push(array, UINT2NUM(acquire_exit_count)); - rb_ary_push(array, UINT2NUM(release_count)); + VALUE array = rb_ary_new2(5); + rb_ary_push(array, UINT2NUM(started_count)); + rb_ary_push(array, UINT2NUM(ready_count)); + rb_ary_push(array, UINT2NUM(resumed_count)); + rb_ary_push(array, UINT2NUM(suspended_count)); + rb_ary_push(array, UINT2NUM(exited_count)); return array; } static VALUE thread_reset_counters(VALUE thread) { - RUBY_ATOMIC_SET(acquire_enter_count, 0); - RUBY_ATOMIC_SET(acquire_exit_count, 0); - RUBY_ATOMIC_SET(release_count, 0); + RUBY_ATOMIC_SET(started_count, 0); + RUBY_ATOMIC_SET(ready_count, 0); + RUBY_ATOMIC_SET(resumed_count, 0); + RUBY_ATOMIC_SET(suspended_count, 0); + RUBY_ATOMIC_SET(exited_count, 0); return Qtrue; } @@ -48,7 +60,11 @@ thread_register_callback(VALUE thread) https://github.com/ruby/ruby/blob/trunk/ext/-test-/thread/instrumentation/instrumentation.c#L60 { single_hook = rb_internal_thread_add_event_hook( *ex_callback, - RUBY_INTERNAL_THREAD_EVENT_READY | RUBY_INTERNAL_THREAD_EVENT_RESUMED | RUBY_INTERNAL_THREAD_EVENT_SUSPENDED, + RUBY_INTERNAL_THREAD_EVENT_STARTED | + RUBY_INTERNAL_THREAD_EVENT_READY | + RUBY_INTERNAL_THREAD_EVENT_RESUMED | + RUBY_INTERNAL_THREAD_EVENT_SUSPENDED | + RUBY_INTERNAL_THREAD_EVENT_EXITED, NULL ); diff --git a/include/ruby/thread.h b/include/ruby/thread.h index 7f9e623fe5..299aa2d62e 100644 --- a/include/ruby/thread.h +++ b/include/ruby/thread.h @@ -190,10 +190,12 @@ void *rb_nogvl(void *(*func)(void *), void *data1, https://github.com/ruby/ruby/blob/trunk/include/ruby/thread.h#L190 */ #define RUBY_CALL_WO_GVL_FLAG_SKIP_CHECK_INTS_ -#define RUBY_INTERNAL_THREAD_EVENT_READY 0x01 /** acquiring GVL */ -#define RUBY_INTERNAL_THREAD_EVENT_RESUMED 0x02 /** acquired GVL */ -#define RUBY_INTERNAL_THREAD_EVENT_SUSPENDED 0x04 /** released GVL */ -#define RUBY_INTERNAL_THREAD_EVENT_MASK 0x07 /** All Thread events */ +#define RUBY_INTERNAL_THREAD_EVENT_STARTED 1 << 0 /** thread started */ +#define RUBY_INTERNAL_THREAD_EVENT_READY 1 << 1 /** acquiring GVL */ +#define RUBY_INTERNAL_THREAD_EVENT_RESUMED 1 << 2 /** acquired GVL */ +#define RUBY_INTERNAL_THREAD_EVENT_SUSPENDED 1 << 3 /** released GVL */ +#define RUBY_INTERNAL_THREAD_EVENT_EXITED 1 << 4 /** thread terminated */ +#define RUBY_INTERNAL_THREAD_EVENT_MASK 0xff /** All Thread events */ typedef void rb_internal_thread_event_data_t; // for future extension. diff --git a/test/-ext-/thread/test_instrumentation_api.rb b/test/-ext-/thread/test_instrumentation_api.rb index d110348d73..ff76e95bcd 100644 --- a/test/-ext-/thread/test_instrumentation_api.rb +++ b/test/-ext-/thread/test_instrumentation_api.rb @@ -44,8 +44,10 @@ class TestThreadInstrumentation < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/-ext-/thread/test_instrumentation_api.rb#L44 counters = Marshal.load(read_pipe) read_pipe.close counters.each do |c| - assert_predicate c,:nonzero?, "Call counters: #{counters.inspect}" + assert_predicate c, :nonzero?, "Call counters: #{counters.inspect}" end + + assert_equal counters.first, counters.last # exited as many times as we entered ensure Bug::ThreadInstrumentation::unregister_callback end diff --git a/thread.c b/thread.c index 6c22867bab..620a56d9a0 100644 --- a/thread.c +++ b/thread.c @@ -631,7 +631,6 @@ thread_do_start(rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/thread.c#L631 } void rb_ec_clear_current_thread_trace_func(const rb_execution_context_t *ec); -#define thread_sched_to_dead thread_sched_to_waiting static int thread_start_func_2(rb_thread_t *th, VALUE *stack_start) diff --git a/thread_none.c b/thread_none.c index cf4658e571..00004ed4dd 100644 --- a/thread_none.c +++ b/thread_none.c @@ -30,6 +30,8 @@ thread_sched_to_waiting(struct rb_thread_sched *sched) https://github.com/ruby/ruby/blob/trunk/thread_none.c#L30 { } +#define thread_sched_to_dead thread_sched_to_waiting + static void thread_sched_yield(struct rb_thread_sched *sched, rb_thread_t *th) { diff --git a/thread_pthread.c b/thread_pthread.c index 1f5b5b9030..da0efeb99b 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -109,6 +109,8 @@ struct rb_internal_thread_event_hook { https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L109 static rb_internal_thread_event_hook_t *rb_internal_thread_event_hooks = NULL; static pthread_rwlock_t rb_internal_thread_event_hooks_rw_lock = PTHREAD_RWLOCK_INITIALIZER; +#define RB_INTERNAL_THREAD_HOOK(event) if (rb_internal_thread_event_hooks) { rb_thread_execute_hooks(event); } + rb_internal_thread_event_hook_t * rb_internal_thread_add_event_hook(rb_internal_thread_event_callback callback, rb_event_flag_t internal_event, void *user_data) { @@ -377,10 +379,7 @@ thread_sched_to_ready_common(struct rb_thread_sched *sched, rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L379 static void thread_sched_to_running_common(struct rb_thread_sched *sched, rb_thread_t *th) { - if (rb_internal_thread_event_hooks) { - rb_thread_execute_hooks(RUBY_INTERNAL_THREAD_EVENT_READY); - } - + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_READY); if (sched->running) { VM_ASSERT(th->unblock.func == 0 && "we must not be in ubf_list and GVL readyq at the same time"); @@ -412,9 +411,7 @@ thread_sched_to_running_common(struct rb_thread_sched *sched, rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L411 // ready -> running sched->running = th; - if (rb_internal_thread_event_hooks) { - rb_thread_execute_hooks(RUBY_INTERNAL_THREAD_EVENT_RESUMED); - } + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_RESUMED); if (!sched->timer) { if (!designate_timer_thread(sched) && !ubf_threads_empty()) { @@ -434,10 +431,6 @@ thread_sched_to_running(struct rb_thread_sched *sched, rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L431 static rb_thread_t * thread_sched_to_waiting_common(struct rb_thread_sched *sched) { - if (rb_internal_thread_event_hooks) { - rb_thread_execute_hooks(RUBY_INTERNAL_THREAD_EVENT_SUSPENDED); - } - rb_thread_t *next; sched->running = NULL; next = ccan_list_top(&sched->readyq, rb_thread_t, sched.node.readyq); @@ -449,11 +442,19 @@ thread_sched_to_waiting_common(struct rb_thread_sched *sched) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L442 static void thread_sched_to_waiting(struct rb_thread_sched *sched) { + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_SUSPENDED); rb_native_mutex_lock(&sched->lock); thread_sched_to_waiting_common(sched); rb_native_mutex_unlock(&sched->lock); } +static void +thread_sched_to_dead(struct rb_thread_sched *sched) +{ + thread_sched_to_waiting(sched); + RB_INTERNAL_THREAD_HOOK(RUBY_INTERNAL_THREAD_EVENT_EXITED); + (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/