From c3fa0fdf8c333955de80167972bf83b22c6564c0 Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Mon, 24 Jan 2022 16:05:23 +0000 Subject: [PATCH 01/11] [wip] Allow callbacks to be registered for GVL related events --- .../gvl/call_without_gvl/call_without_gvl.c | 22 +++++++++++++ include/ruby/thread.h | 1 + include/ruby/thread_native.h | 7 +++++ test/-ext-/gvl/test_last_thread.rb | 7 +++++ thread_pthread.c | 31 +++++++++++++++++++ thread_pthread.h | 11 +++++++ 6 files changed, 79 insertions(+) diff --git a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c index 233635421b816f..e845914782d8ae 100644 --- a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c +++ b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c @@ -1,5 +1,6 @@ #include "ruby/ruby.h" #include "ruby/thread.h" +#include "ruby/thread_native.h" static void* native_sleep_callback(void *data) @@ -68,6 +69,25 @@ thread_ubf_async_safe(VALUE thread, VALUE notify_fd) return Qnil; } +void +ex_callback(uint32_t e, struct gvl_hook_event_args args) { + fprintf(stderr, "calling callback\n"); +} + +static VALUE +thread_register_gvl_callback(VALUE thread) { + rb_gvl_event_new(*ex_callback, 0x12); + + + return Qnil; +} + +static VALUE +thread_call_gvl_callback(VALUE thread) { + rb_gvl_execute_hooks(0x12); + return Qnil; +} + void Init_call_without_gvl(void) { @@ -75,4 +95,6 @@ Init_call_without_gvl(void) VALUE klass = rb_define_module_under(mBug, "Thread"); rb_define_singleton_method(klass, "runnable_sleep", thread_runnable_sleep, 1); rb_define_singleton_method(klass, "ubf_async_safe", thread_ubf_async_safe, 1); + rb_define_singleton_method(klass, "register_callback", thread_register_gvl_callback, 0); + rb_define_singleton_method(klass, "call_callbacks", thread_call_gvl_callback, 0); } diff --git a/include/ruby/thread.h b/include/ruby/thread.h index 18c792b3861ccc..cc0ceeffc4cc09 100644 --- a/include/ruby/thread.h +++ b/include/ruby/thread.h @@ -190,6 +190,7 @@ void *rb_nogvl(void *(*func)(void *), void *data1, */ #define RUBY_CALL_WO_GVL_FLAG_SKIP_CHECK_INTS_ + RBIMPL_SYMBOL_EXPORT_END() #endif /* RUBY_THREAD_H */ diff --git a/include/ruby/thread_native.h b/include/ruby/thread_native.h index c23b15e133a81f..ae1df02343f7ee 100644 --- a/include/ruby/thread_native.h +++ b/include/ruby/thread_native.h @@ -201,5 +201,12 @@ void rb_native_cond_initialize(rb_nativethread_cond_t *cond); */ void rb_native_cond_destroy(rb_nativethread_cond_t *cond); +#include +struct gvl_hook_event_args { + // +}; +typedef void (*rb_gvl_callback)(uint32_t event, struct gvl_hook_event_args args); +void rb_gvl_event_new(void *callback, uint32_t event); +void rb_gvl_execute_hooks(uint32_t event); RBIMPL_SYMBOL_EXPORT_END() #endif diff --git a/test/-ext-/gvl/test_last_thread.rb b/test/-ext-/gvl/test_last_thread.rb index f1bebafeea942e..791e13834d581a 100644 --- a/test/-ext-/gvl/test_last_thread.rb +++ b/test/-ext-/gvl/test_last_thread.rb @@ -18,5 +18,12 @@ def test_last_thread assert_in_delta(1.0, t, 0.16) end; end + + def test_gvl_instrumentation + require '-test-/gvl/call_without_gvl' + Bug::Thread::register_callback + + Bug::Thread::call_callbacks + end end diff --git a/thread_pthread.c b/thread_pthread.c index 55289de73a3be4..5665afbe727679 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -101,6 +101,37 @@ # endif #endif +static gvl_hook_t * rb_gvl_hooks = NULL; + +void +rb_gvl_event_new(void *callback, uint32_t event) { + gvl_hook_t *hook = ALLOC_N(gvl_hook_t, 1); + hook->callback = callback; + hook->event = event; + + if(!rb_gvl_hooks) { + rb_gvl_hooks = hook; + } else { + hook->next = rb_gvl_hooks; + rb_gvl_hooks = hook; + } +} + +void +rb_gvl_execute_hooks(uint32_t event) { + if (!rb_gvl_hooks) { + return; + } + gvl_hook_t *h = rb_gvl_hooks; + struct gvl_hook_event_args args = {}; + + do { + if (h->event & event) { + (*h->callback)(event, args); + } + } while((h = h->next)); +} + enum rtimer_state { /* alive, after timer_create: */ RTIMER_DISARM, diff --git a/thread_pthread.h b/thread_pthread.h index 2ac354046c010b..240c8d9fe453e6 100644 --- a/thread_pthread.h +++ b/thread_pthread.h @@ -69,6 +69,17 @@ typedef struct rb_global_vm_lock_struct { int wait_yield; } rb_global_vm_lock_t; +#include + +// TODO: this is going to be the same on Windows so move it somewhere sensible +typedef struct gvl_hook { + rb_gvl_callback callback; + uint32_t event; + + struct gvl_hook *next; +} gvl_hook_t; + +#include "ruby/internal/memory.h" #if __STDC_VERSION__ >= 201112 #define RB_THREAD_LOCAL_SPECIFIER _Thread_local From e0c541b8c4dd6fe015df3f26f1efe494ecb93870 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 25 Jan 2022 13:19:08 +0100 Subject: [PATCH 02/11] Basic unregister GVL callbacks API (thread unsafe) --- .../gvl/call_without_gvl/call_without_gvl.c | 32 +++++++++++++++++-- include/ruby/thread_native.h | 16 +++++++++- test/-ext-/gvl/test_last_thread.rb | 11 ++++++- thread_pthread.c | 24 +++++++++++++- thread_pthread.h | 11 ------- 5 files changed, 78 insertions(+), 16 deletions(-) diff --git a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c index e845914782d8ae..dd861ac4516402 100644 --- a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c +++ b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c @@ -74,20 +74,46 @@ ex_callback(uint32_t e, struct gvl_hook_event_args args) { fprintf(stderr, "calling callback\n"); } +static gvl_hook_t * single_hook = NULL; + static VALUE thread_register_gvl_callback(VALUE thread) { - rb_gvl_event_new(*ex_callback, 0x12); + single_hook = rb_gvl_event_new(*ex_callback, 0x12); + + return Qnil; +} +static VALUE +thread_unregister_gvl_callback(VALUE thread) { + if (single_hook) { + rb_gvl_event_delete(single_hook); + single_hook = NULL; + } return Qnil; } static VALUE thread_call_gvl_callback(VALUE thread) { - rb_gvl_execute_hooks(0x12); + rb_gvl_execute_hooks(0x12); return Qnil; } +static VALUE +thread_register_and_unregister_gvl_callback(VALUE thread) { + gvl_hook_t * hooks[5]; + for (int i = 0; i < 5; i++) { + hooks[i] = rb_gvl_event_new(*ex_callback, 0x12); + } + + if (!rb_gvl_event_delete(hooks[4])) return Qfalse; + if (!rb_gvl_event_delete(hooks[0])) return Qfalse; + if (!rb_gvl_event_delete(hooks[3])) return Qfalse; + if (!rb_gvl_event_delete(hooks[2])) return Qfalse; + if (!rb_gvl_event_delete(hooks[1])) return Qfalse; + return Qtrue; +} + void Init_call_without_gvl(void) { @@ -96,5 +122,7 @@ Init_call_without_gvl(void) rb_define_singleton_method(klass, "runnable_sleep", thread_runnable_sleep, 1); rb_define_singleton_method(klass, "ubf_async_safe", thread_ubf_async_safe, 1); rb_define_singleton_method(klass, "register_callback", thread_register_gvl_callback, 0); + rb_define_singleton_method(klass, "unregister_callback", thread_unregister_gvl_callback, 0); + rb_define_singleton_method(klass, "register_and_unregister_callbacks", thread_register_and_unregister_gvl_callback, 0); rb_define_singleton_method(klass, "call_callbacks", thread_call_gvl_callback, 0); } diff --git a/include/ruby/thread_native.h b/include/ruby/thread_native.h index ae1df02343f7ee..d722013ea0f470 100644 --- a/include/ruby/thread_native.h +++ b/include/ruby/thread_native.h @@ -205,8 +205,22 @@ void rb_native_cond_destroy(rb_nativethread_cond_t *cond); struct gvl_hook_event_args { // }; +#include + typedef void (*rb_gvl_callback)(uint32_t event, struct gvl_hook_event_args args); -void rb_gvl_event_new(void *callback, uint32_t event); + +// TODO: this is going to be the same on Windows so move it somewhere sensible +typedef struct gvl_hook { + rb_gvl_callback callback; + uint32_t event; + + struct gvl_hook *next; +} gvl_hook_t; + +#include "ruby/internal/memory.h" + +gvl_hook_t * rb_gvl_event_new(void *callback, uint32_t event); +bool rb_gvl_event_delete(gvl_hook_t * hook); void rb_gvl_execute_hooks(uint32_t event); RBIMPL_SYMBOL_EXPORT_END() #endif diff --git a/test/-ext-/gvl/test_last_thread.rb b/test/-ext-/gvl/test_last_thread.rb index 791e13834d581a..dab8783e568992 100644 --- a/test/-ext-/gvl/test_last_thread.rb +++ b/test/-ext-/gvl/test_last_thread.rb @@ -23,7 +23,16 @@ def test_gvl_instrumentation require '-test-/gvl/call_without_gvl' Bug::Thread::register_callback - Bug::Thread::call_callbacks + begin + Bug::Thread::call_callbacks + ensure + Bug::Thread::unregister_callback + end + end + + def test_gvl_instrumentation_unregister + require '-test-/gvl/call_without_gvl' + assert Bug::Thread::register_and_unregister_callbacks end end diff --git a/thread_pthread.c b/thread_pthread.c index 5665afbe727679..8672ab36237b9a 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -103,7 +103,7 @@ static gvl_hook_t * rb_gvl_hooks = NULL; -void +gvl_hook_t * rb_gvl_event_new(void *callback, uint32_t event) { gvl_hook_t *hook = ALLOC_N(gvl_hook_t, 1); hook->callback = callback; @@ -115,6 +115,28 @@ rb_gvl_event_new(void *callback, uint32_t event) { hook->next = rb_gvl_hooks; rb_gvl_hooks = hook; } + return hook; +} + +bool +rb_gvl_event_delete(gvl_hook_t * hook) { + if (rb_gvl_hooks == hook) { + rb_gvl_hooks = hook->next; + ruby_xfree(hook); + return TRUE; + } + + gvl_hook_t *h = rb_gvl_hooks; + + do { + if (h->next == hook) { + h->next = hook->next; + ruby_xfree(hook); + return TRUE; + } + } while ((h = h->next)); + + return FALSE; } void diff --git a/thread_pthread.h b/thread_pthread.h index 240c8d9fe453e6..2ac354046c010b 100644 --- a/thread_pthread.h +++ b/thread_pthread.h @@ -69,17 +69,6 @@ typedef struct rb_global_vm_lock_struct { int wait_yield; } rb_global_vm_lock_t; -#include - -// TODO: this is going to be the same on Windows so move it somewhere sensible -typedef struct gvl_hook { - rb_gvl_callback callback; - uint32_t event; - - struct gvl_hook *next; -} gvl_hook_t; - -#include "ruby/internal/memory.h" #if __STDC_VERSION__ >= 201112 #define RB_THREAD_LOCAL_SPECIFIER _Thread_local From 4d48cfe3944f019c9cb4dae4de4895f798f7a4e2 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 25 Jan 2022 15:01:07 +0100 Subject: [PATCH 03/11] Make rb_gvl hook API thread safe --- thread_pthread.c | 61 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/thread_pthread.c b/thread_pthread.c index 8672ab36237b9a..6f3d8b00da1224 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -102,6 +102,7 @@ #endif static gvl_hook_t * rb_gvl_hooks = NULL; +static pthread_rwlock_t rb_gvl_hooks_rw_lock = PTHREAD_RWLOCK_INITIALIZER; gvl_hook_t * rb_gvl_event_new(void *callback, uint32_t event) { @@ -109,34 +110,48 @@ rb_gvl_event_new(void *callback, uint32_t event) { hook->callback = callback; hook->event = event; - if(!rb_gvl_hooks) { - rb_gvl_hooks = hook; - } else { - hook->next = rb_gvl_hooks; - rb_gvl_hooks = hook; + if (pthread_rwlock_wrlock(&rb_gvl_hooks_rw_lock)) { + // TODO: better way to deal with error? + ruby_xfree(hook); + return NULL; } + + hook->next = rb_gvl_hooks; + ATOMIC_PTR_EXCHANGE(rb_gvl_hooks, hook); + + pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock); return hook; } bool rb_gvl_event_delete(gvl_hook_t * hook) { + if (pthread_rwlock_wrlock(&rb_gvl_hooks_rw_lock)) { + // TODO: better way to deal with error? + return FALSE; + } + + bool success = FALSE; + if (rb_gvl_hooks == hook) { - rb_gvl_hooks = hook->next; - ruby_xfree(hook); - return TRUE; + ATOMIC_PTR_EXCHANGE(rb_gvl_hooks, hook->next); + success = TRUE; + } else { + gvl_hook_t *h = rb_gvl_hooks; + + do { + if (h->next == hook) { + h->next = hook->next; + success = TRUE; + } + } while ((h = h->next)); } - - gvl_hook_t *h = rb_gvl_hooks; - do { - if (h->next == hook) { - h->next = hook->next; - ruby_xfree(hook); - return TRUE; - } - } while ((h = h->next)); + pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock); - return FALSE; + if (success) { + ruby_xfree(hook); + } + return success; } void @@ -144,6 +159,12 @@ rb_gvl_execute_hooks(uint32_t event) { if (!rb_gvl_hooks) { return; } + + if (pthread_rwlock_rdlock(&rb_gvl_hooks_rw_lock)) { + // TODO: better way to deal with error? + return; + } + gvl_hook_t *h = rb_gvl_hooks; struct gvl_hook_event_args args = {}; @@ -152,6 +173,8 @@ rb_gvl_execute_hooks(uint32_t event) { (*h->callback)(event, args); } } while((h = h->next)); + + pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock); } enum rtimer_state { @@ -697,6 +720,8 @@ static void native_thread_init(rb_thread_t *th); void Init_native_thread(rb_thread_t *th) { + pthread_rwlock_init(&rb_gvl_hooks_rw_lock, NULL); + #if defined(HAVE_PTHREAD_CONDATTR_SETCLOCK) if (condattr_monotonic) { int r = pthread_condattr_init(condattr_monotonic); From 316c44d911ef8df5ad36ca449c66a27fedc5ee95 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 25 Jan 2022 16:01:09 +0100 Subject: [PATCH 04/11] Call the gvl hooks --- .../gvl/call_without_gvl/call_without_gvl.c | 6 +-- include/ruby/internal/event.h | 28 +++++++------ include/ruby/thread_native.h | 16 ++++---- thread_pthread.c | 41 ++++++++++++------- thread_pthread.h | 1 + 5 files changed, 53 insertions(+), 39 deletions(-) diff --git a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c index dd861ac4516402..5c5eb5d5e83344 100644 --- a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c +++ b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c @@ -78,7 +78,7 @@ static gvl_hook_t * single_hook = NULL; static VALUE thread_register_gvl_callback(VALUE thread) { - single_hook = rb_gvl_event_new(*ex_callback, 0x12); + single_hook = rb_gvl_event_new(*ex_callback, RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER); return Qnil; } @@ -95,7 +95,7 @@ thread_unregister_gvl_callback(VALUE thread) { static VALUE thread_call_gvl_callback(VALUE thread) { - rb_gvl_execute_hooks(0x12); + rb_gvl_execute_hooks(RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER, 1); return Qnil; } @@ -103,7 +103,7 @@ static VALUE thread_register_and_unregister_gvl_callback(VALUE thread) { gvl_hook_t * hooks[5]; for (int i = 0; i < 5; i++) { - hooks[i] = rb_gvl_event_new(*ex_callback, 0x12); + hooks[i] = rb_gvl_event_new(*ex_callback, RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER); } if (!rb_gvl_event_delete(hooks[4])) return Qfalse; diff --git a/include/ruby/internal/event.h b/include/ruby/internal/event.h index 04b137a1939e85..633d17fa9e0541 100644 --- a/include/ruby/internal/event.h +++ b/include/ruby/internal/event.h @@ -82,18 +82,22 @@ * * @{ */ -#define RUBY_INTERNAL_EVENT_SWITCH 0x040000 /**< Thread switched. */ -#define RUBY_EVENT_SWITCH 0x040000 /**< @old{RUBY_INTERNAL_EVENT_SWITCH} */ - /* 0x080000 */ -#define RUBY_INTERNAL_EVENT_NEWOBJ 0x100000 /**< Object allocated. */ -#define RUBY_INTERNAL_EVENT_FREEOBJ 0x200000 /**< Object swept. */ -#define RUBY_INTERNAL_EVENT_GC_START 0x400000 /**< GC started. */ -#define RUBY_INTERNAL_EVENT_GC_END_MARK 0x800000 /**< GC ended mark phase. */ -#define RUBY_INTERNAL_EVENT_GC_END_SWEEP 0x1000000 /**< GC ended sweep phase. */ -#define RUBY_INTERNAL_EVENT_GC_ENTER 0x2000000 /**< `gc_enter()` is called. */ -#define RUBY_INTERNAL_EVENT_GC_EXIT 0x4000000 /**< `gc_exit()` is called. */ -#define RUBY_INTERNAL_EVENT_OBJSPACE_MASK 0x7f00000 /**< Bitmask of GC events. */ -#define RUBY_INTERNAL_EVENT_MASK 0xffff0000 /**< Bitmask of internal events. */ +#define RUBY_INTERNAL_EVENT_SWITCH 0x00040000 /**< Thread switched. */ +#define RUBY_EVENT_SWITCH 0x00040000 /**< @old{RUBY_INTERNAL_EVENT_SWITCH} */ + /*0x00080000 */ +#define RUBY_INTERNAL_EVENT_NEWOBJ 0x00100000 /**< Object allocated. */ +#define RUBY_INTERNAL_EVENT_FREEOBJ 0x00200000 /**< Object swept. */ +#define RUBY_INTERNAL_EVENT_GC_START 0x00400000 /**< GC started. */ +#define RUBY_INTERNAL_EVENT_GC_END_MARK 0x00800000 /**< GC ended mark phase. */ +#define RUBY_INTERNAL_EVENT_GC_END_SWEEP 0x01000000 /**< GC ended sweep phase. */ +#define RUBY_INTERNAL_EVENT_GC_ENTER 0x02000000 /**< `gc_enter()` is called. */ +#define RUBY_INTERNAL_EVENT_GC_EXIT 0x04000000 /**< `gc_exit()` is called. */ +#define RUBY_INTERNAL_EVENT_OBJSPACE_MASK 0x07f00000 /**< Bitmask of GC events. */ +#define RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER 0x10000000 /** `gvl_acquire() is called */ +#define RUBY_INTERNAL_EVENT_GVL_ACQUIRE_EXIT 0x20000000 /** `gvl_acquire() is exiting successfully */ +#define RUBY_INTERNAL_EVENT_GVL_RELEASE 0x40000000 /** `gvl_release() is called */ +#define RUBY_INTERNAL_EVENT_GVL_MASK 0x70000000 /**< Bitmask of GVL events. */ +#define RUBY_INTERNAL_EVENT_MASK 0xffff0000 /**< Bitmask of internal events. */ /** @} */ diff --git a/include/ruby/thread_native.h b/include/ruby/thread_native.h index d722013ea0f470..84a208ce3ba64e 100644 --- a/include/ruby/thread_native.h +++ b/include/ruby/thread_native.h @@ -201,26 +201,24 @@ void rb_native_cond_initialize(rb_nativethread_cond_t *cond); */ void rb_native_cond_destroy(rb_nativethread_cond_t *cond); -#include -struct gvl_hook_event_args { - // -}; -#include +typedef struct gvl_hook_event_args { + unsigned long waiting; +} gvl_hook_event_args_t; -typedef void (*rb_gvl_callback)(uint32_t event, struct gvl_hook_event_args args); +typedef void (*rb_gvl_callback)(uint32_t event, gvl_hook_event_args_t args); // TODO: this is going to be the same on Windows so move it somewhere sensible typedef struct gvl_hook { rb_gvl_callback callback; - uint32_t event; + rb_event_flag_t event; struct gvl_hook *next; } gvl_hook_t; #include "ruby/internal/memory.h" -gvl_hook_t * rb_gvl_event_new(void *callback, uint32_t event); +gvl_hook_t * rb_gvl_event_new(void *callback, rb_event_flag_t event); bool rb_gvl_event_delete(gvl_hook_t * hook); -void rb_gvl_execute_hooks(uint32_t event); +void rb_gvl_execute_hooks(rb_event_flag_t event, unsigned long waiting); RBIMPL_SYMBOL_EXPORT_END() #endif diff --git a/thread_pthread.c b/thread_pthread.c index 6f3d8b00da1224..1c6648a19b56eb 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -105,7 +105,7 @@ static gvl_hook_t * rb_gvl_hooks = NULL; static pthread_rwlock_t rb_gvl_hooks_rw_lock = PTHREAD_RWLOCK_INITIALIZER; gvl_hook_t * -rb_gvl_event_new(void *callback, uint32_t event) { +rb_gvl_event_new(void *callback, rb_event_flag_t event) { gvl_hook_t *hook = ALLOC_N(gvl_hook_t, 1); hook->callback = callback; hook->event = event; @@ -155,25 +155,21 @@ rb_gvl_event_delete(gvl_hook_t * hook) { } void -rb_gvl_execute_hooks(uint32_t event) { - if (!rb_gvl_hooks) { - return; - } - +rb_gvl_execute_hooks(rb_event_flag_t event, unsigned long waiting) { if (pthread_rwlock_rdlock(&rb_gvl_hooks_rw_lock)) { // TODO: better way to deal with error? return; } - gvl_hook_t *h = rb_gvl_hooks; - struct gvl_hook_event_args args = {}; - - do { - if (h->event & event) { - (*h->callback)(event, args); - } - } while((h = h->next)); - + if (rb_gvl_hooks) { + gvl_hook_t *h = rb_gvl_hooks; + gvl_hook_event_args_t args = { .waiting = waiting }; + do { + if (h->event & event) { + (*h->callback)(event, args); + } + } while((h = h->next)); + } pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock); } @@ -366,6 +362,10 @@ gvl_acquire_common(rb_global_vm_lock_t *gvl, rb_thread_t *th) "we must not be in ubf_list and GVL waitq at the same time"); list_add_tail(&gvl->waitq, &nd->node.gvl); + gvl->waiting++; + if (rb_gvl_hooks) { + rb_gvl_execute_hooks(RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER, gvl->waiting); + } do { if (!gvl->timer) { @@ -377,6 +377,7 @@ gvl_acquire_common(rb_global_vm_lock_t *gvl, rb_thread_t *th) } while (gvl->owner); list_del_init(&nd->node.gvl); + gvl->waiting--; if (gvl->need_yield) { gvl->need_yield = 0; @@ -387,6 +388,11 @@ gvl_acquire_common(rb_global_vm_lock_t *gvl, rb_thread_t *th) gvl->timer_err = ETIMEDOUT; } gvl->owner = th; + + if (rb_gvl_hooks) { + rb_gvl_execute_hooks(RUBY_INTERNAL_EVENT_GVL_ACQUIRE_EXIT, gvl->waiting); + } + if (!gvl->timer) { if (!designate_timer_thread(gvl) && !ubf_threads_empty()) { rb_thread_wakeup_timer_thread(-1); @@ -405,6 +411,10 @@ gvl_acquire(rb_global_vm_lock_t *gvl, rb_thread_t *th) static const native_thread_data_t * gvl_release_common(rb_global_vm_lock_t *gvl) { + if (rb_gvl_hooks) { + rb_gvl_execute_hooks(RUBY_INTERNAL_EVENT_GVL_RELEASE, gvl->waiting); + } + native_thread_data_t *next; gvl->owner = 0; next = list_top(&gvl->waitq, native_thread_data_t, node.ubf); @@ -466,6 +476,7 @@ rb_gvl_init(rb_global_vm_lock_t *gvl) rb_native_cond_initialize(&gvl->switch_wait_cond); list_head_init(&gvl->waitq); gvl->owner = 0; + gvl->waiting = 0; gvl->timer = 0; gvl->timer_err = ETIMEDOUT; gvl->need_yield = 0; diff --git a/thread_pthread.h b/thread_pthread.h index 2ac354046c010b..9830a428162a81 100644 --- a/thread_pthread.h +++ b/thread_pthread.h @@ -59,6 +59,7 @@ typedef struct rb_global_vm_lock_struct { * timer. */ struct list_head waitq; /* <=> native_thread_data_t.node.ubf */ + volatile unsigned long waiting; const struct rb_thread_struct *timer; int timer_err; From cd2222b48a102c643f5b0634462a663749c6482c Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 26 Jan 2022 16:48:29 +0100 Subject: [PATCH 05/11] Move GVL instrumentation tests in their own files --- .../gvl/call_without_gvl/call_without_gvl.c | 50 ------ .../instrumentation/instrumentation/depend | 161 ++++++++++++++++++ .../instrumentation/extconf.rb | 2 + .../instrumentation/instrumentation.c | 95 +++++++++++ include/ruby/thread.h | 1 - test/-ext-/gvl/test_instrumentation_api.rb | 24 +++ test/-ext-/gvl/test_last_thread.rb | 16 -- 7 files changed, 282 insertions(+), 67 deletions(-) create mode 100644 ext/-test-/gvl/instrumentation/instrumentation/depend create mode 100644 ext/-test-/gvl/instrumentation/instrumentation/extconf.rb create mode 100644 ext/-test-/gvl/instrumentation/instrumentation/instrumentation.c create mode 100644 test/-ext-/gvl/test_instrumentation_api.rb diff --git a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c index 5c5eb5d5e83344..233635421b816f 100644 --- a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c +++ b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c @@ -1,6 +1,5 @@ #include "ruby/ruby.h" #include "ruby/thread.h" -#include "ruby/thread_native.h" static void* native_sleep_callback(void *data) @@ -69,51 +68,6 @@ thread_ubf_async_safe(VALUE thread, VALUE notify_fd) return Qnil; } -void -ex_callback(uint32_t e, struct gvl_hook_event_args args) { - fprintf(stderr, "calling callback\n"); -} - -static gvl_hook_t * single_hook = NULL; - -static VALUE -thread_register_gvl_callback(VALUE thread) { - single_hook = rb_gvl_event_new(*ex_callback, RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER); - - return Qnil; -} - -static VALUE -thread_unregister_gvl_callback(VALUE thread) { - if (single_hook) { - rb_gvl_event_delete(single_hook); - single_hook = NULL; - } - - return Qnil; -} - -static VALUE -thread_call_gvl_callback(VALUE thread) { - rb_gvl_execute_hooks(RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER, 1); - return Qnil; -} - -static VALUE -thread_register_and_unregister_gvl_callback(VALUE thread) { - gvl_hook_t * hooks[5]; - for (int i = 0; i < 5; i++) { - hooks[i] = rb_gvl_event_new(*ex_callback, RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER); - } - - if (!rb_gvl_event_delete(hooks[4])) return Qfalse; - if (!rb_gvl_event_delete(hooks[0])) return Qfalse; - if (!rb_gvl_event_delete(hooks[3])) return Qfalse; - if (!rb_gvl_event_delete(hooks[2])) return Qfalse; - if (!rb_gvl_event_delete(hooks[1])) return Qfalse; - return Qtrue; -} - void Init_call_without_gvl(void) { @@ -121,8 +75,4 @@ Init_call_without_gvl(void) VALUE klass = rb_define_module_under(mBug, "Thread"); rb_define_singleton_method(klass, "runnable_sleep", thread_runnable_sleep, 1); rb_define_singleton_method(klass, "ubf_async_safe", thread_ubf_async_safe, 1); - rb_define_singleton_method(klass, "register_callback", thread_register_gvl_callback, 0); - rb_define_singleton_method(klass, "unregister_callback", thread_unregister_gvl_callback, 0); - rb_define_singleton_method(klass, "register_and_unregister_callbacks", thread_register_and_unregister_gvl_callback, 0); - rb_define_singleton_method(klass, "call_callbacks", thread_call_gvl_callback, 0); } diff --git a/ext/-test-/gvl/instrumentation/instrumentation/depend b/ext/-test-/gvl/instrumentation/instrumentation/depend new file mode 100644 index 00000000000000..d74a525224f668 --- /dev/null +++ b/ext/-test-/gvl/instrumentation/instrumentation/depend @@ -0,0 +1,161 @@ +# AUTOGENERATED DEPENDENCIES START +call_without_gvl.o: $(RUBY_EXTCONF_H) +call_without_gvl.o: $(arch_hdrdir)/ruby/config.h +call_without_gvl.o: $(hdrdir)/ruby/assert.h +call_without_gvl.o: $(hdrdir)/ruby/backward.h +call_without_gvl.o: $(hdrdir)/ruby/backward/2/assume.h +call_without_gvl.o: $(hdrdir)/ruby/backward/2/attributes.h +call_without_gvl.o: $(hdrdir)/ruby/backward/2/bool.h +call_without_gvl.o: $(hdrdir)/ruby/backward/2/inttypes.h +call_without_gvl.o: $(hdrdir)/ruby/backward/2/limits.h +call_without_gvl.o: $(hdrdir)/ruby/backward/2/long_long.h +call_without_gvl.o: $(hdrdir)/ruby/backward/2/stdalign.h +call_without_gvl.o: $(hdrdir)/ruby/backward/2/stdarg.h +call_without_gvl.o: $(hdrdir)/ruby/defines.h +call_without_gvl.o: $(hdrdir)/ruby/intern.h +call_without_gvl.o: $(hdrdir)/ruby/internal/anyargs.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/char.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/double.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/fixnum.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/gid_t.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/int.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/intptr_t.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/long.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/long_long.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/mode_t.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/off_t.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/pid_t.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/short.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/size_t.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/st_data_t.h +call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/uid_t.h +call_without_gvl.o: $(hdrdir)/ruby/internal/assume.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/alloc_size.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/artificial.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/cold.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/const.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/constexpr.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/deprecated.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/diagnose_if.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/enum_extensibility.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/error.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/flag_enum.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/forceinline.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/format.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/maybe_unused.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/noalias.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/nodiscard.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/noexcept.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/noinline.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/nonnull.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/noreturn.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/pure.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/restrict.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/returns_nonnull.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/warning.h +call_without_gvl.o: $(hdrdir)/ruby/internal/attr/weakref.h +call_without_gvl.o: $(hdrdir)/ruby/internal/cast.h +call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is.h +call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/apple.h +call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/clang.h +call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/gcc.h +call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/intel.h +call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/msvc.h +call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/sunpro.h +call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_since.h +call_without_gvl.o: $(hdrdir)/ruby/internal/config.h +call_without_gvl.o: $(hdrdir)/ruby/internal/constant_p.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rarray.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rbasic.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rbignum.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rclass.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rdata.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rfile.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rhash.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/robject.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rregexp.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rstring.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rstruct.h +call_without_gvl.o: $(hdrdir)/ruby/internal/core/rtypeddata.h +call_without_gvl.o: $(hdrdir)/ruby/internal/ctype.h +call_without_gvl.o: $(hdrdir)/ruby/internal/dllexport.h +call_without_gvl.o: $(hdrdir)/ruby/internal/dosish.h +call_without_gvl.o: $(hdrdir)/ruby/internal/error.h +call_without_gvl.o: $(hdrdir)/ruby/internal/eval.h +call_without_gvl.o: $(hdrdir)/ruby/internal/event.h +call_without_gvl.o: $(hdrdir)/ruby/internal/fl_type.h +call_without_gvl.o: $(hdrdir)/ruby/internal/gc.h +call_without_gvl.o: $(hdrdir)/ruby/internal/glob.h +call_without_gvl.o: $(hdrdir)/ruby/internal/globals.h +call_without_gvl.o: $(hdrdir)/ruby/internal/has/attribute.h +call_without_gvl.o: $(hdrdir)/ruby/internal/has/builtin.h +call_without_gvl.o: $(hdrdir)/ruby/internal/has/c_attribute.h +call_without_gvl.o: $(hdrdir)/ruby/internal/has/cpp_attribute.h +call_without_gvl.o: $(hdrdir)/ruby/internal/has/declspec_attribute.h +call_without_gvl.o: $(hdrdir)/ruby/internal/has/extension.h +call_without_gvl.o: $(hdrdir)/ruby/internal/has/feature.h +call_without_gvl.o: $(hdrdir)/ruby/internal/has/warning.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/array.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/bignum.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/class.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/compar.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/complex.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/cont.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/dir.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/enum.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/enumerator.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/error.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/eval.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/file.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/gc.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/hash.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/io.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/load.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/marshal.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/numeric.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/object.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/parse.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/proc.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/process.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/random.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/range.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/rational.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/re.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/ruby.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/select.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/select/largesize.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/signal.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/sprintf.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/string.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/struct.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/thread.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/time.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/variable.h +call_without_gvl.o: $(hdrdir)/ruby/internal/intern/vm.h +call_without_gvl.o: $(hdrdir)/ruby/internal/interpreter.h +call_without_gvl.o: $(hdrdir)/ruby/internal/iterator.h +call_without_gvl.o: $(hdrdir)/ruby/internal/memory.h +call_without_gvl.o: $(hdrdir)/ruby/internal/method.h +call_without_gvl.o: $(hdrdir)/ruby/internal/module.h +call_without_gvl.o: $(hdrdir)/ruby/internal/newobj.h +call_without_gvl.o: $(hdrdir)/ruby/internal/rgengc.h +call_without_gvl.o: $(hdrdir)/ruby/internal/scan_args.h +call_without_gvl.o: $(hdrdir)/ruby/internal/special_consts.h +call_without_gvl.o: $(hdrdir)/ruby/internal/static_assert.h +call_without_gvl.o: $(hdrdir)/ruby/internal/stdalign.h +call_without_gvl.o: $(hdrdir)/ruby/internal/stdbool.h +call_without_gvl.o: $(hdrdir)/ruby/internal/symbol.h +call_without_gvl.o: $(hdrdir)/ruby/internal/value.h +call_without_gvl.o: $(hdrdir)/ruby/internal/value_type.h +call_without_gvl.o: $(hdrdir)/ruby/internal/variable.h +call_without_gvl.o: $(hdrdir)/ruby/internal/warning_push.h +call_without_gvl.o: $(hdrdir)/ruby/internal/xmalloc.h +call_without_gvl.o: $(hdrdir)/ruby/missing.h +call_without_gvl.o: $(hdrdir)/ruby/ruby.h +call_without_gvl.o: $(hdrdir)/ruby/st.h +call_without_gvl.o: $(hdrdir)/ruby/subst.h +call_without_gvl.o: $(hdrdir)/ruby/thread.h +call_without_gvl.o: call_without_gvl.c +# AUTOGENERATED DEPENDENCIES END diff --git a/ext/-test-/gvl/instrumentation/instrumentation/extconf.rb b/ext/-test-/gvl/instrumentation/instrumentation/extconf.rb new file mode 100644 index 00000000000000..81845048b23816 --- /dev/null +++ b/ext/-test-/gvl/instrumentation/instrumentation/extconf.rb @@ -0,0 +1,2 @@ +# frozen_string_literal: false +create_makefile("-test-/gvl/instrumentation") diff --git a/ext/-test-/gvl/instrumentation/instrumentation/instrumentation.c b/ext/-test-/gvl/instrumentation/instrumentation/instrumentation.c new file mode 100644 index 00000000000000..a7c72cc2371604 --- /dev/null +++ b/ext/-test-/gvl/instrumentation/instrumentation/instrumentation.c @@ -0,0 +1,95 @@ +#include "ruby/ruby.h" +#include "ruby/atomic.h" +#include "ruby/thread.h" +#include "ruby/thread_native.h" + +static rb_atomic_t acquire_enter_count = 0; +static rb_atomic_t acquire_exit_count = 0; +static rb_atomic_t release_count = 0; + +void +ex_callback(rb_event_flag_t event, gvl_hook_event_args_t args) +{ + switch(event) { + case RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER: + RUBY_ATOMIC_INC(acquire_enter_count); + break; + case RUBY_INTERNAL_EVENT_GVL_ACQUIRE_EXIT: + RUBY_ATOMIC_INC(acquire_exit_count); + break; + case RUBY_INTERNAL_EVENT_GVL_RELEASE: + RUBY_ATOMIC_INC(release_count); + break; + } +} + +static gvl_hook_t * single_hook = NULL; + +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)); + 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); + return Qtrue; +} + +static VALUE +thread_register_gvl_callback(VALUE thread) +{ + single_hook = rb_gvl_event_new( + *ex_callback, + RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER | RUBY_INTERNAL_EVENT_GVL_ACQUIRE_EXIT | RUBY_INTERNAL_EVENT_GVL_RELEASE + ); + + return Qnil; +} + +static VALUE +thread_unregister_gvl_callback(VALUE thread) +{ + if (single_hook) { + rb_gvl_event_delete(single_hook); + single_hook = NULL; + } + + return Qnil; +} + +static VALUE +thread_register_and_unregister_gvl_callback(VALUE thread) +{ + gvl_hook_t * hooks[5]; + for (int i = 0; i < 5; i++) { + hooks[i] = rb_gvl_event_new(*ex_callback, RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER); + } + + if (!rb_gvl_event_delete(hooks[4])) return Qfalse; + if (!rb_gvl_event_delete(hooks[0])) return Qfalse; + if (!rb_gvl_event_delete(hooks[3])) return Qfalse; + if (!rb_gvl_event_delete(hooks[2])) return Qfalse; + if (!rb_gvl_event_delete(hooks[1])) return Qfalse; + return Qtrue; +} + +void +Init_instrumentation(void) +{ + VALUE mBug = rb_define_module("Bug"); + VALUE klass = rb_define_module_under(mBug, "GVLInstrumentation"); + rb_define_singleton_method(klass, "counters", thread_counters, 0); + rb_define_singleton_method(klass, "reset_counters", thread_reset_counters, 0); + rb_define_singleton_method(klass, "register_callback", thread_register_gvl_callback, 0); + rb_define_singleton_method(klass, "unregister_callback", thread_unregister_gvl_callback, 0); + rb_define_singleton_method(klass, "register_and_unregister_callbacks", thread_register_and_unregister_gvl_callback, 0); +} diff --git a/include/ruby/thread.h b/include/ruby/thread.h index cc0ceeffc4cc09..18c792b3861ccc 100644 --- a/include/ruby/thread.h +++ b/include/ruby/thread.h @@ -190,7 +190,6 @@ void *rb_nogvl(void *(*func)(void *), void *data1, */ #define RUBY_CALL_WO_GVL_FLAG_SKIP_CHECK_INTS_ - RBIMPL_SYMBOL_EXPORT_END() #endif /* RUBY_THREAD_H */ diff --git a/test/-ext-/gvl/test_instrumentation_api.rb b/test/-ext-/gvl/test_instrumentation_api.rb new file mode 100644 index 00000000000000..e30a85507718c9 --- /dev/null +++ b/test/-ext-/gvl/test_instrumentation_api.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: false +class TestGVLInstrumentation < Test::Unit::TestCase + def test_gvl_instrumentation + require '-test-/gvl/instrumentation' + Bug::GVLInstrumentation.reset_counters + Bug::GVLInstrumentation::register_callback + + begin + threads = 5.times.map { Thread.new { sleep 0.05; 1 + 1; sleep 0.02 } } + threads.each(&:join) + Bug::GVLInstrumentation.counters.each do |c| + assert_predicate c,:nonzero? + end + ensure + Bug::GVLInstrumentation::unregister_callback + end + end + + def test_gvl_instrumentation_unregister + require '-test-/gvl/instrumentation' + assert Bug::GVLInstrumentation::register_and_unregister_callbacks + end +end + diff --git a/test/-ext-/gvl/test_last_thread.rb b/test/-ext-/gvl/test_last_thread.rb index dab8783e568992..f1bebafeea942e 100644 --- a/test/-ext-/gvl/test_last_thread.rb +++ b/test/-ext-/gvl/test_last_thread.rb @@ -18,21 +18,5 @@ def test_last_thread assert_in_delta(1.0, t, 0.16) end; end - - def test_gvl_instrumentation - require '-test-/gvl/call_without_gvl' - Bug::Thread::register_callback - - begin - Bug::Thread::call_callbacks - ensure - Bug::Thread::unregister_callback - end - end - - def test_gvl_instrumentation_unregister - require '-test-/gvl/call_without_gvl' - assert Bug::Thread::register_and_unregister_callbacks - end end From 720262b1b97a642a61a5a7ca02d41f5f93c65501 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 26 Jan 2022 17:28:54 +0100 Subject: [PATCH 06/11] Make gvl->waiting atomic --- include/ruby/thread_native.h | 3 ++- thread_pthread.c | 6 +++--- thread_pthread.h | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/include/ruby/thread_native.h b/include/ruby/thread_native.h index 84a208ce3ba64e..9e49bbe84e291f 100644 --- a/include/ruby/thread_native.h +++ b/include/ruby/thread_native.h @@ -216,9 +216,10 @@ typedef struct gvl_hook { } gvl_hook_t; #include "ruby/internal/memory.h" +#include "ruby/atomic.h" gvl_hook_t * rb_gvl_event_new(void *callback, rb_event_flag_t event); bool rb_gvl_event_delete(gvl_hook_t * hook); -void rb_gvl_execute_hooks(rb_event_flag_t event, unsigned long waiting); +void rb_gvl_execute_hooks(rb_event_flag_t event, rb_atomic_t waiting); RBIMPL_SYMBOL_EXPORT_END() #endif diff --git a/thread_pthread.c b/thread_pthread.c index 1c6648a19b56eb..91c68654ea32d4 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -155,7 +155,7 @@ rb_gvl_event_delete(gvl_hook_t * hook) { } void -rb_gvl_execute_hooks(rb_event_flag_t event, unsigned long waiting) { +rb_gvl_execute_hooks(rb_event_flag_t event, rb_atomic_t waiting) { if (pthread_rwlock_rdlock(&rb_gvl_hooks_rw_lock)) { // TODO: better way to deal with error? return; @@ -362,7 +362,7 @@ gvl_acquire_common(rb_global_vm_lock_t *gvl, rb_thread_t *th) "we must not be in ubf_list and GVL waitq at the same time"); list_add_tail(&gvl->waitq, &nd->node.gvl); - gvl->waiting++; + ATOMIC_INC(gvl->waiting); if (rb_gvl_hooks) { rb_gvl_execute_hooks(RUBY_INTERNAL_EVENT_GVL_ACQUIRE_ENTER, gvl->waiting); } @@ -377,7 +377,7 @@ gvl_acquire_common(rb_global_vm_lock_t *gvl, rb_thread_t *th) } while (gvl->owner); list_del_init(&nd->node.gvl); - gvl->waiting--; + ATOMIC_DEC(gvl->waiting); if (gvl->need_yield) { gvl->need_yield = 0; diff --git a/thread_pthread.h b/thread_pthread.h index 9830a428162a81..1b38a6d34cedbc 100644 --- a/thread_pthread.h +++ b/thread_pthread.h @@ -59,7 +59,7 @@ typedef struct rb_global_vm_lock_struct { * timer. */ struct list_head waitq; /* <=> native_thread_data_t.node.ubf */ - volatile unsigned long waiting; + rb_atomic_t waiting; const struct rb_thread_struct *timer; int timer_err; From b867e19f05b3660a215c82a8c55ce2491a8f7512 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 27 Jan 2022 13:43:11 +0100 Subject: [PATCH 07/11] Hard crash on pthread_rwlock errors --- thread_pthread.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/thread_pthread.c b/thread_pthread.c index 91c68654ea32d4..c8c13f7d6aecf2 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -111,9 +111,7 @@ rb_gvl_event_new(void *callback, rb_event_flag_t event) { hook->event = event; if (pthread_rwlock_wrlock(&rb_gvl_hooks_rw_lock)) { - // TODO: better way to deal with error? - ruby_xfree(hook); - return NULL; + rb_bug("GVL hooks deadlock"); } hook->next = rb_gvl_hooks; @@ -126,8 +124,7 @@ rb_gvl_event_new(void *callback, rb_event_flag_t event) { bool rb_gvl_event_delete(gvl_hook_t * hook) { if (pthread_rwlock_wrlock(&rb_gvl_hooks_rw_lock)) { - // TODO: better way to deal with error? - return FALSE; + rb_bug("GVL hooks deadlock"); } bool success = FALSE; @@ -157,8 +154,7 @@ rb_gvl_event_delete(gvl_hook_t * hook) { void rb_gvl_execute_hooks(rb_event_flag_t event, rb_atomic_t waiting) { if (pthread_rwlock_rdlock(&rb_gvl_hooks_rw_lock)) { - // TODO: better way to deal with error? - return; + rb_bug("GVL hooks deadlock"); } if (rb_gvl_hooks) { From 1f599a6b34c901a1c919c07745a3a915830ed72a Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 27 Jan 2022 13:46:51 +0100 Subject: [PATCH 08/11] Stub rb_gvl_event_* on win32 --- include/ruby/thread_native.h | 1 - thread_pthread.c | 2 +- thread_win32.c | 10 ++++++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/include/ruby/thread_native.h b/include/ruby/thread_native.h index 9e49bbe84e291f..1e8925135b1831 100644 --- a/include/ruby/thread_native.h +++ b/include/ruby/thread_native.h @@ -220,6 +220,5 @@ typedef struct gvl_hook { gvl_hook_t * rb_gvl_event_new(void *callback, rb_event_flag_t event); bool rb_gvl_event_delete(gvl_hook_t * hook); -void rb_gvl_execute_hooks(rb_event_flag_t event, rb_atomic_t waiting); RBIMPL_SYMBOL_EXPORT_END() #endif diff --git a/thread_pthread.c b/thread_pthread.c index c8c13f7d6aecf2..5d2057b61a8cec 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -151,7 +151,7 @@ rb_gvl_event_delete(gvl_hook_t * hook) { return success; } -void +static void rb_gvl_execute_hooks(rb_event_flag_t event, rb_atomic_t waiting) { if (pthread_rwlock_rdlock(&rb_gvl_hooks_rw_lock)) { rb_bug("GVL hooks deadlock"); diff --git a/thread_win32.c b/thread_win32.c index 9b44ceb96a9e4a..527aae1f566dd1 100644 --- a/thread_win32.c +++ b/thread_win32.c @@ -35,6 +35,16 @@ static volatile DWORD ruby_native_thread_key = TLS_OUT_OF_INDEXES; static int w32_wait_events(HANDLE *events, int count, DWORD timeout, rb_thread_t *th); +gvl_hook_t * +rb_gvl_event_new(void *callback, rb_event_flag_t event) { + // not implemented yet +} + +bool +rb_gvl_event_delete(gvl_hook_t * hook) { + // not implemented yet +} + RBIMPL_ATTR_NORETURN() static void w32_error(const char *func) From ed89a1b7c68e227491527dea6101bf771c08d215 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 27 Jan 2022 14:07:19 +0100 Subject: [PATCH 09/11] More pthread_rwlock error handling --- test/-ext-/gvl/test_instrumentation_api.rb | 26 +++++++++++++++++ thread_pthread.c | 34 +++++++++++++++------- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/test/-ext-/gvl/test_instrumentation_api.rb b/test/-ext-/gvl/test_instrumentation_api.rb index e30a85507718c9..97940bdaaf329b 100644 --- a/test/-ext-/gvl/test_instrumentation_api.rb +++ b/test/-ext-/gvl/test_instrumentation_api.rb @@ -1,5 +1,9 @@ # frozen_string_literal: false class TestGVLInstrumentation < Test::Unit::TestCase + def setup + skip("No windows support yet") if /mswin|mingw|bccwin/ =~ RUBY_PLATFORM + end + def test_gvl_instrumentation require '-test-/gvl/instrumentation' Bug::GVLInstrumentation.reset_counters @@ -16,6 +20,28 @@ def test_gvl_instrumentation end end + def test_gvl_instrumentation_fork_safe + skip "No fork()" unless Process.respond_to?(:fork) + + require '-test-/gvl/instrumentation' + Bug::GVLInstrumentation::register_callback + + begin + pid = fork do + Bug::GVLInstrumentation.reset_counters + threads = 5.times.map { Thread.new { sleep 0.05; 1 + 1; sleep 0.02 } } + threads.each(&:join) + Bug::GVLInstrumentation.counters.each do |c| + assert_predicate c,:nonzero? + end + end + _, status = Process.wait2(pid) + assert_predicate status, :success? + ensure + Bug::GVLInstrumentation::unregister_callback + end + end + def test_gvl_instrumentation_unregister require '-test-/gvl/instrumentation' assert Bug::GVLInstrumentation::register_and_unregister_callbacks diff --git a/thread_pthread.c b/thread_pthread.c index 5d2057b61a8cec..8c874df0a31f34 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -110,21 +110,25 @@ rb_gvl_event_new(void *callback, rb_event_flag_t event) { hook->callback = callback; hook->event = event; - if (pthread_rwlock_wrlock(&rb_gvl_hooks_rw_lock)) { - rb_bug("GVL hooks deadlock"); + int r; + if ((r = pthread_rwlock_wrlock(&rb_gvl_hooks_rw_lock))) { + rb_bug_errno("pthread_rwlock_wrlock", r); } hook->next = rb_gvl_hooks; ATOMIC_PTR_EXCHANGE(rb_gvl_hooks, hook); - pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock); + if ((r = pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock))) { + rb_bug_errno("pthread_rwlock_unlock", r); + } return hook; } bool rb_gvl_event_delete(gvl_hook_t * hook) { - if (pthread_rwlock_wrlock(&rb_gvl_hooks_rw_lock)) { - rb_bug("GVL hooks deadlock"); + int r; + if ((r = pthread_rwlock_wrlock(&rb_gvl_hooks_rw_lock))) { + rb_bug_errno("pthread_rwlock_wrlock", r); } bool success = FALSE; @@ -143,7 +147,9 @@ rb_gvl_event_delete(gvl_hook_t * hook) { } while ((h = h->next)); } - pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock); + if ((r = pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock))) { + rb_bug_errno("pthread_rwlock_unlock", r); + } if (success) { ruby_xfree(hook); @@ -153,8 +159,9 @@ rb_gvl_event_delete(gvl_hook_t * hook) { static void rb_gvl_execute_hooks(rb_event_flag_t event, rb_atomic_t waiting) { - if (pthread_rwlock_rdlock(&rb_gvl_hooks_rw_lock)) { - rb_bug("GVL hooks deadlock"); + int r; + if ((r = pthread_rwlock_rdlock(&rb_gvl_hooks_rw_lock))) { + rb_bug_errno("pthread_rwlock_rdlock", r); } if (rb_gvl_hooks) { @@ -166,7 +173,9 @@ rb_gvl_execute_hooks(rb_event_flag_t event, rb_atomic_t waiting) { } } while((h = h->next)); } - pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock); + if ((r = pthread_rwlock_unlock(&rb_gvl_hooks_rw_lock))) { + rb_bug_errno("pthread_rwlock_unlock", r); + } } enum rtimer_state { @@ -727,11 +736,14 @@ static void native_thread_init(rb_thread_t *th); void Init_native_thread(rb_thread_t *th) { - pthread_rwlock_init(&rb_gvl_hooks_rw_lock, NULL); + int r; + if ((r = pthread_rwlock_init(&rb_gvl_hooks_rw_lock, NULL))) { + rb_bug_errno("pthread_rwlock_init", r); + } #if defined(HAVE_PTHREAD_CONDATTR_SETCLOCK) if (condattr_monotonic) { - int r = pthread_condattr_init(condattr_monotonic); + r = pthread_condattr_init(condattr_monotonic); if (r == 0) { r = pthread_condattr_setclock(condattr_monotonic, CLOCK_MONOTONIC); } From 5c9e48ccd61e063e28a614869128b6740af82472 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 27 Jan 2022 14:47:22 +0100 Subject: [PATCH 10/11] Fix depend --- .../instrumentation/instrumentation/depend | 320 +++++++++--------- include/ruby/thread_native.h | 4 - test/-ext-/gvl/test_instrumentation_api.rb | 2 +- 3 files changed, 162 insertions(+), 164 deletions(-) diff --git a/ext/-test-/gvl/instrumentation/instrumentation/depend b/ext/-test-/gvl/instrumentation/instrumentation/depend index d74a525224f668..33366d5bb2aace 100644 --- a/ext/-test-/gvl/instrumentation/instrumentation/depend +++ b/ext/-test-/gvl/instrumentation/instrumentation/depend @@ -1,161 +1,163 @@ # AUTOGENERATED DEPENDENCIES START -call_without_gvl.o: $(RUBY_EXTCONF_H) -call_without_gvl.o: $(arch_hdrdir)/ruby/config.h -call_without_gvl.o: $(hdrdir)/ruby/assert.h -call_without_gvl.o: $(hdrdir)/ruby/backward.h -call_without_gvl.o: $(hdrdir)/ruby/backward/2/assume.h -call_without_gvl.o: $(hdrdir)/ruby/backward/2/attributes.h -call_without_gvl.o: $(hdrdir)/ruby/backward/2/bool.h -call_without_gvl.o: $(hdrdir)/ruby/backward/2/inttypes.h -call_without_gvl.o: $(hdrdir)/ruby/backward/2/limits.h -call_without_gvl.o: $(hdrdir)/ruby/backward/2/long_long.h -call_without_gvl.o: $(hdrdir)/ruby/backward/2/stdalign.h -call_without_gvl.o: $(hdrdir)/ruby/backward/2/stdarg.h -call_without_gvl.o: $(hdrdir)/ruby/defines.h -call_without_gvl.o: $(hdrdir)/ruby/intern.h -call_without_gvl.o: $(hdrdir)/ruby/internal/anyargs.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/char.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/double.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/fixnum.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/gid_t.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/int.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/intptr_t.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/long.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/long_long.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/mode_t.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/off_t.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/pid_t.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/short.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/size_t.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/st_data_t.h -call_without_gvl.o: $(hdrdir)/ruby/internal/arithmetic/uid_t.h -call_without_gvl.o: $(hdrdir)/ruby/internal/assume.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/alloc_size.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/artificial.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/cold.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/const.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/constexpr.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/deprecated.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/diagnose_if.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/enum_extensibility.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/error.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/flag_enum.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/forceinline.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/format.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/maybe_unused.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/noalias.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/nodiscard.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/noexcept.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/noinline.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/nonnull.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/noreturn.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/pure.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/restrict.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/returns_nonnull.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/warning.h -call_without_gvl.o: $(hdrdir)/ruby/internal/attr/weakref.h -call_without_gvl.o: $(hdrdir)/ruby/internal/cast.h -call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is.h -call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/apple.h -call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/clang.h -call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/gcc.h -call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/intel.h -call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/msvc.h -call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_is/sunpro.h -call_without_gvl.o: $(hdrdir)/ruby/internal/compiler_since.h -call_without_gvl.o: $(hdrdir)/ruby/internal/config.h -call_without_gvl.o: $(hdrdir)/ruby/internal/constant_p.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rarray.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rbasic.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rbignum.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rclass.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rdata.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rfile.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rhash.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/robject.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rregexp.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rstring.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rstruct.h -call_without_gvl.o: $(hdrdir)/ruby/internal/core/rtypeddata.h -call_without_gvl.o: $(hdrdir)/ruby/internal/ctype.h -call_without_gvl.o: $(hdrdir)/ruby/internal/dllexport.h -call_without_gvl.o: $(hdrdir)/ruby/internal/dosish.h -call_without_gvl.o: $(hdrdir)/ruby/internal/error.h -call_without_gvl.o: $(hdrdir)/ruby/internal/eval.h -call_without_gvl.o: $(hdrdir)/ruby/internal/event.h -call_without_gvl.o: $(hdrdir)/ruby/internal/fl_type.h -call_without_gvl.o: $(hdrdir)/ruby/internal/gc.h -call_without_gvl.o: $(hdrdir)/ruby/internal/glob.h -call_without_gvl.o: $(hdrdir)/ruby/internal/globals.h -call_without_gvl.o: $(hdrdir)/ruby/internal/has/attribute.h -call_without_gvl.o: $(hdrdir)/ruby/internal/has/builtin.h -call_without_gvl.o: $(hdrdir)/ruby/internal/has/c_attribute.h -call_without_gvl.o: $(hdrdir)/ruby/internal/has/cpp_attribute.h -call_without_gvl.o: $(hdrdir)/ruby/internal/has/declspec_attribute.h -call_without_gvl.o: $(hdrdir)/ruby/internal/has/extension.h -call_without_gvl.o: $(hdrdir)/ruby/internal/has/feature.h -call_without_gvl.o: $(hdrdir)/ruby/internal/has/warning.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/array.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/bignum.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/class.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/compar.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/complex.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/cont.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/dir.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/enum.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/enumerator.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/error.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/eval.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/file.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/gc.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/hash.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/io.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/load.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/marshal.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/numeric.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/object.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/parse.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/proc.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/process.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/random.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/range.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/rational.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/re.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/ruby.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/select.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/select/largesize.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/signal.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/sprintf.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/string.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/struct.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/thread.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/time.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/variable.h -call_without_gvl.o: $(hdrdir)/ruby/internal/intern/vm.h -call_without_gvl.o: $(hdrdir)/ruby/internal/interpreter.h -call_without_gvl.o: $(hdrdir)/ruby/internal/iterator.h -call_without_gvl.o: $(hdrdir)/ruby/internal/memory.h -call_without_gvl.o: $(hdrdir)/ruby/internal/method.h -call_without_gvl.o: $(hdrdir)/ruby/internal/module.h -call_without_gvl.o: $(hdrdir)/ruby/internal/newobj.h -call_without_gvl.o: $(hdrdir)/ruby/internal/rgengc.h -call_without_gvl.o: $(hdrdir)/ruby/internal/scan_args.h -call_without_gvl.o: $(hdrdir)/ruby/internal/special_consts.h -call_without_gvl.o: $(hdrdir)/ruby/internal/static_assert.h -call_without_gvl.o: $(hdrdir)/ruby/internal/stdalign.h -call_without_gvl.o: $(hdrdir)/ruby/internal/stdbool.h -call_without_gvl.o: $(hdrdir)/ruby/internal/symbol.h -call_without_gvl.o: $(hdrdir)/ruby/internal/value.h -call_without_gvl.o: $(hdrdir)/ruby/internal/value_type.h -call_without_gvl.o: $(hdrdir)/ruby/internal/variable.h -call_without_gvl.o: $(hdrdir)/ruby/internal/warning_push.h -call_without_gvl.o: $(hdrdir)/ruby/internal/xmalloc.h -call_without_gvl.o: $(hdrdir)/ruby/missing.h -call_without_gvl.o: $(hdrdir)/ruby/ruby.h -call_without_gvl.o: $(hdrdir)/ruby/st.h -call_without_gvl.o: $(hdrdir)/ruby/subst.h -call_without_gvl.o: $(hdrdir)/ruby/thread.h -call_without_gvl.o: call_without_gvl.c +instrumentation.o: $(RUBY_EXTCONF_H) +instrumentation.o: $(arch_hdrdir)/ruby/config.h +instrumentation.o: $(hdrdir)/ruby/assert.h +instrumentation.o: $(hdrdir)/ruby/atomic.h +instrumentation.o: $(hdrdir)/ruby/backward.h +instrumentation.o: $(hdrdir)/ruby/backward/2/assume.h +instrumentation.o: $(hdrdir)/ruby/backward/2/attributes.h +instrumentation.o: $(hdrdir)/ruby/backward/2/bool.h +instrumentation.o: $(hdrdir)/ruby/backward/2/inttypes.h +instrumentation.o: $(hdrdir)/ruby/backward/2/limits.h +instrumentation.o: $(hdrdir)/ruby/backward/2/long_long.h +instrumentation.o: $(hdrdir)/ruby/backward/2/stdalign.h +instrumentation.o: $(hdrdir)/ruby/backward/2/stdarg.h +instrumentation.o: $(hdrdir)/ruby/defines.h +instrumentation.o: $(hdrdir)/ruby/intern.h +instrumentation.o: $(hdrdir)/ruby/internal/anyargs.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/char.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/double.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/fixnum.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/gid_t.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/int.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/intptr_t.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/long.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/long_long.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/mode_t.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/off_t.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/pid_t.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/short.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/size_t.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/st_data_t.h +instrumentation.o: $(hdrdir)/ruby/internal/arithmetic/uid_t.h +instrumentation.o: $(hdrdir)/ruby/internal/assume.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/alloc_size.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/artificial.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/cold.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/const.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/constexpr.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/deprecated.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/diagnose_if.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/enum_extensibility.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/error.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/flag_enum.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/forceinline.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/format.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/maybe_unused.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/noalias.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/nodiscard.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/noexcept.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/noinline.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/nonnull.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/noreturn.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/pure.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/restrict.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/returns_nonnull.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/warning.h +instrumentation.o: $(hdrdir)/ruby/internal/attr/weakref.h +instrumentation.o: $(hdrdir)/ruby/internal/cast.h +instrumentation.o: $(hdrdir)/ruby/internal/compiler_is.h +instrumentation.o: $(hdrdir)/ruby/internal/compiler_is/apple.h +instrumentation.o: $(hdrdir)/ruby/internal/compiler_is/clang.h +instrumentation.o: $(hdrdir)/ruby/internal/compiler_is/gcc.h +instrumentation.o: $(hdrdir)/ruby/internal/compiler_is/intel.h +instrumentation.o: $(hdrdir)/ruby/internal/compiler_is/msvc.h +instrumentation.o: $(hdrdir)/ruby/internal/compiler_is/sunpro.h +instrumentation.o: $(hdrdir)/ruby/internal/compiler_since.h +instrumentation.o: $(hdrdir)/ruby/internal/config.h +instrumentation.o: $(hdrdir)/ruby/internal/constant_p.h +instrumentation.o: $(hdrdir)/ruby/internal/core.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rarray.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rbasic.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rbignum.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rclass.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rdata.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rfile.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rhash.h +instrumentation.o: $(hdrdir)/ruby/internal/core/robject.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rregexp.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rstring.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rstruct.h +instrumentation.o: $(hdrdir)/ruby/internal/core/rtypeddata.h +instrumentation.o: $(hdrdir)/ruby/internal/ctype.h +instrumentation.o: $(hdrdir)/ruby/internal/dllexport.h +instrumentation.o: $(hdrdir)/ruby/internal/dosish.h +instrumentation.o: $(hdrdir)/ruby/internal/error.h +instrumentation.o: $(hdrdir)/ruby/internal/eval.h +instrumentation.o: $(hdrdir)/ruby/internal/event.h +instrumentation.o: $(hdrdir)/ruby/internal/fl_type.h +instrumentation.o: $(hdrdir)/ruby/internal/gc.h +instrumentation.o: $(hdrdir)/ruby/internal/glob.h +instrumentation.o: $(hdrdir)/ruby/internal/globals.h +instrumentation.o: $(hdrdir)/ruby/internal/has/attribute.h +instrumentation.o: $(hdrdir)/ruby/internal/has/builtin.h +instrumentation.o: $(hdrdir)/ruby/internal/has/c_attribute.h +instrumentation.o: $(hdrdir)/ruby/internal/has/cpp_attribute.h +instrumentation.o: $(hdrdir)/ruby/internal/has/declspec_attribute.h +instrumentation.o: $(hdrdir)/ruby/internal/has/extension.h +instrumentation.o: $(hdrdir)/ruby/internal/has/feature.h +instrumentation.o: $(hdrdir)/ruby/internal/has/warning.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/array.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/bignum.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/class.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/compar.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/complex.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/cont.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/dir.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/enum.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/enumerator.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/error.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/eval.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/file.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/gc.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/hash.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/io.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/load.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/marshal.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/numeric.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/object.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/parse.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/proc.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/process.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/random.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/range.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/rational.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/re.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/ruby.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/select.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/select/largesize.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/signal.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/sprintf.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/string.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/struct.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/thread.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/time.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/variable.h +instrumentation.o: $(hdrdir)/ruby/internal/intern/vm.h +instrumentation.o: $(hdrdir)/ruby/internal/interpreter.h +instrumentation.o: $(hdrdir)/ruby/internal/iterator.h +instrumentation.o: $(hdrdir)/ruby/internal/memory.h +instrumentation.o: $(hdrdir)/ruby/internal/method.h +instrumentation.o: $(hdrdir)/ruby/internal/module.h +instrumentation.o: $(hdrdir)/ruby/internal/newobj.h +instrumentation.o: $(hdrdir)/ruby/internal/rgengc.h +instrumentation.o: $(hdrdir)/ruby/internal/scan_args.h +instrumentation.o: $(hdrdir)/ruby/internal/special_consts.h +instrumentation.o: $(hdrdir)/ruby/internal/static_assert.h +instrumentation.o: $(hdrdir)/ruby/internal/stdalign.h +instrumentation.o: $(hdrdir)/ruby/internal/stdbool.h +instrumentation.o: $(hdrdir)/ruby/internal/symbol.h +instrumentation.o: $(hdrdir)/ruby/internal/value.h +instrumentation.o: $(hdrdir)/ruby/internal/value_type.h +instrumentation.o: $(hdrdir)/ruby/internal/variable.h +instrumentation.o: $(hdrdir)/ruby/internal/warning_push.h +instrumentation.o: $(hdrdir)/ruby/internal/xmalloc.h +instrumentation.o: $(hdrdir)/ruby/missing.h +instrumentation.o: $(hdrdir)/ruby/ruby.h +instrumentation.o: $(hdrdir)/ruby/st.h +instrumentation.o: $(hdrdir)/ruby/subst.h +instrumentation.o: $(hdrdir)/ruby/thread.h +instrumentation.o: $(hdrdir)/ruby/thread_native.h +instrumentation.o: instrumentation.c # AUTOGENERATED DEPENDENCIES END diff --git a/include/ruby/thread_native.h b/include/ruby/thread_native.h index 1e8925135b1831..80853d07fbd61e 100644 --- a/include/ruby/thread_native.h +++ b/include/ruby/thread_native.h @@ -207,7 +207,6 @@ typedef struct gvl_hook_event_args { typedef void (*rb_gvl_callback)(uint32_t event, gvl_hook_event_args_t args); -// TODO: this is going to be the same on Windows so move it somewhere sensible typedef struct gvl_hook { rb_gvl_callback callback; rb_event_flag_t event; @@ -215,9 +214,6 @@ typedef struct gvl_hook { struct gvl_hook *next; } gvl_hook_t; -#include "ruby/internal/memory.h" -#include "ruby/atomic.h" - gvl_hook_t * rb_gvl_event_new(void *callback, rb_event_flag_t event); bool rb_gvl_event_delete(gvl_hook_t * hook); RBIMPL_SYMBOL_EXPORT_END() diff --git a/test/-ext-/gvl/test_instrumentation_api.rb b/test/-ext-/gvl/test_instrumentation_api.rb index 97940bdaaf329b..e93df2becfbbee 100644 --- a/test/-ext-/gvl/test_instrumentation_api.rb +++ b/test/-ext-/gvl/test_instrumentation_api.rb @@ -1,7 +1,7 @@ # frozen_string_literal: false class TestGVLInstrumentation < Test::Unit::TestCase def setup - skip("No windows support yet") if /mswin|mingw|bccwin/ =~ RUBY_PLATFORM + pend("TODO: No windows support yet") if /mswin|mingw|bccwin/ =~ RUBY_PLATFORM end def test_gvl_instrumentation From fed36e094a5cb42496e3ec1209737300dbfda7b6 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 27 Jan 2022 15:50:29 +0100 Subject: [PATCH 11/11] Fix callback type --- include/ruby/thread_native.h | 2 +- thread_pthread.c | 2 +- thread_win32.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/ruby/thread_native.h b/include/ruby/thread_native.h index 80853d07fbd61e..7c4b24273fc685 100644 --- a/include/ruby/thread_native.h +++ b/include/ruby/thread_native.h @@ -214,7 +214,7 @@ typedef struct gvl_hook { struct gvl_hook *next; } gvl_hook_t; -gvl_hook_t * rb_gvl_event_new(void *callback, rb_event_flag_t event); +gvl_hook_t * rb_gvl_event_new(rb_gvl_callback callback, rb_event_flag_t event); bool rb_gvl_event_delete(gvl_hook_t * hook); RBIMPL_SYMBOL_EXPORT_END() #endif diff --git a/thread_pthread.c b/thread_pthread.c index 8c874df0a31f34..2e581a10a81c28 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -105,7 +105,7 @@ static gvl_hook_t * rb_gvl_hooks = NULL; static pthread_rwlock_t rb_gvl_hooks_rw_lock = PTHREAD_RWLOCK_INITIALIZER; gvl_hook_t * -rb_gvl_event_new(void *callback, rb_event_flag_t event) { +rb_gvl_event_new(rb_gvl_callback callback, rb_event_flag_t event) { gvl_hook_t *hook = ALLOC_N(gvl_hook_t, 1); hook->callback = callback; hook->event = event; diff --git a/thread_win32.c b/thread_win32.c index 527aae1f566dd1..6838820084aa36 100644 --- a/thread_win32.c +++ b/thread_win32.c @@ -36,7 +36,7 @@ static volatile DWORD ruby_native_thread_key = TLS_OUT_OF_INDEXES; static int w32_wait_events(HANDLE *events, int count, DWORD timeout, rb_thread_t *th); gvl_hook_t * -rb_gvl_event_new(void *callback, rb_event_flag_t event) { +rb_gvl_event_new(rb_gvl_callback callback, rb_event_flag_t event) { // not implemented yet }