From 3b9896acfcaf992ac233578ca8ec5bb69978de4a Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 6 Nov 2024 22:19:40 +1300 Subject: [PATCH] Revert "Introduce Fiber Scheduler `blocking_region` hook. (#11963)" (#12013) This reverts some of commit 87fb44dff6409a19d12052cf0fc07ba80a4c45ac. We will rename and propose a slightly different interface. --- NEWS.md | 6 ---- common.mk | 1 - include/ruby/fiber/scheduler.h | 20 ----------- scheduler.c | 63 ---------------------------------- test/fiber/scheduler.rb | 4 --- thread.c | 12 ------- 6 files changed, 106 deletions(-) diff --git a/NEWS.md b/NEWS.md index 608bef2b0a..0a2b8d28df 100644 --- a/NEWS.md +++ b/NEWS.md @@ -57,11 +57,6 @@ Note: We're only listing outstanding class updates. associated with the AST node. [[Feature #20624]] * Add RubyVM::AbstractSyntaxTree::Location class which holds location information. [[Feature #20624]] -* Fiber::Scheduler - - * An optional `Fiber::Scheduler#blocking_region` hook allows blocking operations to be moved out of the event loop - in order to reduce latency and improve multi-core processor utilization. [[Feature #20855]] - ## Stdlib updates * Tempfile @@ -228,4 +223,3 @@ details of the default gems or bundled gems. [Feature #20497]: https://bugs.ruby-lang.org/issues/20497 [Feature #20624]: https://bugs.ruby-lang.org/issues/20624 [Feature #20775]: https://bugs.ruby-lang.org/issues/20775 -[Feature #20855]: https://bugs.ruby-lang.org/issues/20855 diff --git a/common.mk b/common.mk index 77e4fe2f6d..3eb12a2b1f 100644 --- a/common.mk +++ b/common.mk @@ -16683,7 +16683,6 @@ scheduler.$(OBJEXT): {$(VPATH)}scheduler.c scheduler.$(OBJEXT): {$(VPATH)}shape.h scheduler.$(OBJEXT): {$(VPATH)}st.h scheduler.$(OBJEXT): {$(VPATH)}subst.h -scheduler.$(OBJEXT): {$(VPATH)}thread.h scheduler.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h scheduler.$(OBJEXT): {$(VPATH)}thread_native.h scheduler.$(OBJEXT): {$(VPATH)}vm_core.h diff --git a/include/ruby/fiber/scheduler.h b/include/ruby/fiber/scheduler.h index bb18b5e01b..8f3d383330 100644 --- a/include/ruby/fiber/scheduler.h +++ b/include/ruby/fiber/scheduler.h @@ -391,26 +391,6 @@ VALUE rb_fiber_scheduler_io_close(VALUE scheduler, VALUE io); */ VALUE rb_fiber_scheduler_address_resolve(VALUE scheduler, VALUE hostname); -struct rb_fiber_scheduler_blocking_region_state { - void *result; - int saved_errno; -}; - -/** - * Defer the execution of the passed function to the scheduler. - * - * @param[in] scheduler Target scheduler. - * @param[in] function The function to run. - * @param[in] data The data to pass to the function. - * @param[in] unblock_function The unblock function to use to interrupt the operation. - * @param[in] data2 The data to pass to the unblock function. - * @param[in] flags Flags passed to `rb_nogvl`. - * @param[out] state The result and errno of the operation. - * @retval RUBY_Qundef `scheduler` doesn't have `#blocking_region`. - * @return otherwise What `scheduler.blocking_region` returns. - */ -VALUE rb_fiber_scheduler_blocking_region(VALUE scheduler, void* (*function)(void *), void *data, rb_unblock_function_t *unblock_function, void *data2, int flags, struct rb_fiber_scheduler_blocking_region_state *state); - /** * Create and schedule a non-blocking fiber. * diff --git a/scheduler.c b/scheduler.c index 0d51a0d951..3159635dba 100644 --- a/scheduler.c +++ b/scheduler.c @@ -13,9 +13,6 @@ #include "ruby/io.h" #include "ruby/io/buffer.h" -#include "ruby/thread.h" - -// For `ruby_thread_has_gvl_p`. #include "internal/thread.h" static ID id_close; @@ -36,8 +33,6 @@ static ID id_io_close; static ID id_address_resolve; -static ID id_blocking_region; - static ID id_fiber_schedule; /* @@ -114,8 +109,6 @@ Init_Fiber_Scheduler(void) id_address_resolve = rb_intern_const("address_resolve"); - id_blocking_region = rb_intern_const("blocking_region"); - id_fiber_schedule = rb_intern_const("fiber"); #if 0 /* for RDoc */ @@ -700,62 +693,6 @@ rb_fiber_scheduler_address_resolve(VALUE scheduler, VALUE hostname) return rb_check_funcall(scheduler, id_address_resolve, 1, arguments); } -struct rb_blocking_region_arguments { - void *(*function)(void *); - void *data; - rb_unblock_function_t *unblock_function; - void *data2; - int flags; - - struct rb_fiber_scheduler_blocking_region_state *state; -}; - -static VALUE -rb_fiber_scheduler_blocking_region_proc(RB_BLOCK_CALL_FUNC_ARGLIST(value, _arguments)) -{ - struct rb_blocking_region_arguments *arguments = (struct rb_blocking_region_arguments*)_arguments; - - if (arguments->state == NULL) { - rb_raise(rb_eRuntimeError, "Blocking function was already invoked!"); - } - - arguments->state->result = rb_nogvl(arguments->function, arguments->data, arguments->unblock_function, arguments->data2, arguments->flags); - arguments->state->saved_errno = rb_errno(); - - // Make sure it's only invoked once. - arguments->state = NULL; - - return Qnil; -} - -/* - * Document-method: Fiber::Scheduler#blocking_region - * call-seq: blocking_region(work) - * - * Invoked by Ruby's core methods to run a blocking operation in a non-blocking way. - * - * Minimal suggested implementation is: - * - * def blocking_region(work) - * Thread.new(&work).join - * end - */ -VALUE rb_fiber_scheduler_blocking_region(VALUE scheduler, void* (*function)(void *), void *data, rb_unblock_function_t *unblock_function, void *data2, int flags, struct rb_fiber_scheduler_blocking_region_state *state) -{ - struct rb_blocking_region_arguments arguments = { - .function = function, - .data = data, - .unblock_function = unblock_function, - .data2 = data2, - .flags = flags, - .state = state - }; - - VALUE proc = rb_proc_new(rb_fiber_scheduler_blocking_region_proc, (VALUE)&arguments); - - return rb_check_funcall(scheduler, id_blocking_region, 1, &proc); -} - /* * Document-method: Fiber::Scheduler#fiber * call-seq: fiber(&block) diff --git a/test/fiber/scheduler.rb b/test/fiber/scheduler.rb index 91fba0476e..e6a8256232 100644 --- a/test/fiber/scheduler.rb +++ b/test/fiber/scheduler.rb @@ -309,10 +309,6 @@ class Scheduler Addrinfo.getaddrinfo(hostname, nil).map(&:ip_address).uniq end.value end - - def blocking_region(work) - Thread.new(&work).join - end end # This scheduler class implements `io_read` and `io_write` hooks which require diff --git a/thread.c b/thread.c index 498b84174e..e1e846c621 100644 --- a/thread.c +++ b/thread.c @@ -1523,18 +1523,6 @@ rb_nogvl(void *(*func)(void *), void *data1, rb_unblock_function_t *ubf, void *data2, int flags) { - VALUE scheduler = rb_fiber_scheduler_current(); - if (scheduler != Qnil) { - struct rb_fiber_scheduler_blocking_region_state state; - - VALUE result = rb_fiber_scheduler_blocking_region(scheduler, func, data1, ubf, data2, flags, &state); - - if (!UNDEF_P(result)) { - rb_errno_set(state.saved_errno); - return state.result; - } - } - void *val = 0; rb_execution_context_t *ec = GET_EC(); rb_thread_t *th = rb_ec_thread_ptr(ec);