1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00

MJIT: Stop using the VM barrier for jit_cont

This solves multiple problems.

First, RB_VM_LOCK_ENTER/LEAVE is a barrier. We could at least use the
_NO_BARRIER variant.

Second, this doesn't need to interfere with GC or other GVL users when
multiple Ractors are used. This needs to be used in very few places, so
the benefit of fine-grained locking would outweigh its small maintenance
cost.

Third, it fixes a crash for YJIT. Because YJIT is never disabled until a
process exits unlike MJIT that finishes earlier, we could call jit_cont_free
when EC no longer exists, which crashes RB_VM_LOCK_ENTER.
This commit is contained in:
Takashi Kokubun 2022-10-19 17:18:59 -07:00
parent 0d360ee7ff
commit d9d9005a3a
No known key found for this signature in database
GPG key ID: 6FFC433B12EE23DD
4 changed files with 35 additions and 20 deletions

35
cont.c
View file

@ -1201,6 +1201,8 @@ cont_save_thread(rb_context_t *cont, rb_thread_t *th)
sec->machine.stack_end = NULL; sec->machine.stack_end = NULL;
} }
static rb_nativethread_lock_t jit_cont_lock;
// Register a new continuation with execution context `ec`. Return JIT info about // Register a new continuation with execution context `ec`. Return JIT info about
// the continuation. // the continuation.
static struct rb_jit_cont * static struct rb_jit_cont *
@ -1216,7 +1218,7 @@ jit_cont_new(rb_execution_context_t *ec)
rb_memerror(); rb_memerror();
cont->ec = ec; cont->ec = ec;
RB_VM_LOCK_ENTER(); rb_native_mutex_lock(&jit_cont_lock);
if (first_jit_cont == NULL) { if (first_jit_cont == NULL) {
cont->next = cont->prev = NULL; cont->next = cont->prev = NULL;
} }
@ -1226,7 +1228,7 @@ jit_cont_new(rb_execution_context_t *ec)
first_jit_cont->prev = cont; first_jit_cont->prev = cont;
} }
first_jit_cont = cont; first_jit_cont = cont;
RB_VM_LOCK_LEAVE(); rb_native_mutex_unlock(&jit_cont_lock);
return cont; return cont;
} }
@ -1235,7 +1237,7 @@ jit_cont_new(rb_execution_context_t *ec)
static void static void
jit_cont_free(struct rb_jit_cont *cont) jit_cont_free(struct rb_jit_cont *cont)
{ {
RB_VM_LOCK_ENTER(); rb_native_mutex_lock(&jit_cont_lock);
if (cont == first_jit_cont) { if (cont == first_jit_cont) {
first_jit_cont = cont->next; first_jit_cont = cont->next;
if (first_jit_cont != NULL) if (first_jit_cont != NULL)
@ -1246,7 +1248,7 @@ jit_cont_free(struct rb_jit_cont *cont)
if (cont->next != NULL) if (cont->next != NULL)
cont->next->prev = cont->prev; cont->next->prev = cont->prev;
} }
RB_VM_LOCK_LEAVE(); rb_native_mutex_unlock(&jit_cont_lock);
free(cont); free(cont);
} }
@ -1273,7 +1275,7 @@ rb_jit_cont_each_iseq(rb_iseq_callback callback, void *data)
} }
} }
// Finish working with continuation info. // Finish working with jit_cont.
void void
rb_jit_cont_finish(void) rb_jit_cont_finish(void)
{ {
@ -1283,8 +1285,9 @@ rb_jit_cont_finish(void)
struct rb_jit_cont *cont, *next; struct rb_jit_cont *cont, *next;
for (cont = first_jit_cont; cont != NULL; cont = next) { for (cont = first_jit_cont; cont != NULL; cont = next) {
next = cont->next; next = cont->next;
xfree(cont); free(cont); // Don't use xfree because it's allocated by calloc.
} }
rb_native_mutex_destroy(&jit_cont_lock);
} }
static void static void
@ -1340,11 +1343,15 @@ rb_fiberptr_blocking(struct rb_fiber_struct *fiber)
return fiber->blocking; return fiber->blocking;
} }
// This is used for root_fiber because other fibers call cont_init_mjit_cont through cont_new. // Start working with jit_cont.
void void
rb_fiber_init_jit_cont(struct rb_fiber_struct *fiber) rb_jit_cont_init(void)
{ {
cont_init_jit_cont(&fiber->cont); if (!jit_cont_enabled)
return;
rb_native_mutex_initialize(&jit_cont_lock);
cont_init_jit_cont(&GET_EC()->fiber_ptr->cont);
} }
#if 0 #if 0
@ -2273,6 +2280,7 @@ root_fiber_alloc(rb_thread_t *th)
return fiber; return fiber;
} }
// Set up a "root fiber", which is the fiber that every Ractor has.
void void
rb_threadptr_root_fiber_setup(rb_thread_t *th) rb_threadptr_root_fiber_setup(rb_thread_t *th)
{ {
@ -2287,10 +2295,11 @@ rb_threadptr_root_fiber_setup(rb_thread_t *th)
fiber->blocking = 1; fiber->blocking = 1;
fiber_status_set(fiber, FIBER_RESUMED); /* skip CREATED */ fiber_status_set(fiber, FIBER_RESUMED); /* skip CREATED */
th->ec = &fiber->cont.saved_ec; th->ec = &fiber->cont.saved_ec;
// This skips jit_cont_new for the initial thread because rb_yjit_enabled_p() and // When rb_threadptr_root_fiber_setup is called for the first time, mjit_enabled and
// mjit_enabled are false at this point. ruby_opt_init will call rb_fiber_init_jit_cont // rb_yjit_enabled_p() are still false. So this does nothing and rb_jit_cont_init() that is
// again for this root_fiber. // called later will take care of it. However, you still have to call cont_init_jit_cont()
rb_fiber_init_jit_cont(fiber); // here for other Ractors, which are not initialized by rb_jit_cont_init().
cont_init_jit_cont(&fiber->cont);
} }
void void

3
eval.c
View file

@ -253,7 +253,6 @@ rb_ec_cleanup(rb_execution_context_t *ec, int ex0)
} }
mjit_finish(true); // We still need ISeqs here, so it's before rb_ec_finalize(). mjit_finish(true); // We still need ISeqs here, so it's before rb_ec_finalize().
rb_jit_cont_finish();
rb_ec_finalize(ec); rb_ec_finalize(ec);
@ -264,6 +263,8 @@ rb_ec_cleanup(rb_execution_context_t *ec, int ex0)
th = th0; th = th0;
rb_thread_stop_timer_thread(); rb_thread_stop_timer_thread();
ruby_vm_destruct(th->vm); ruby_vm_destruct(th->vm);
// For YJIT, call this after ruby_vm_destruct() frees jit_cont for the root fiber.
rb_jit_cont_finish();
if (state) ruby_default_signal(state); if (state) ruby_default_signal(state);
return sysex; return sysex;

View file

@ -18,7 +18,7 @@ struct rb_execution_context_struct; /* in vm_core.c */
/* cont.c */ /* cont.c */
void rb_fiber_reset_root_local_storage(struct rb_thread_struct *); void rb_fiber_reset_root_local_storage(struct rb_thread_struct *);
void ruby_register_rollback_func_for_ensure(VALUE (*ensure_func)(VALUE), VALUE (*rollback_func)(VALUE)); void ruby_register_rollback_func_for_ensure(VALUE (*ensure_func)(VALUE), VALUE (*rollback_func)(VALUE));
void rb_fiber_init_jit_cont(struct rb_fiber_struct *fiber); void rb_jit_cont_init(void);
void rb_jit_cont_each_iseq(rb_iseq_callback callback, void *data); void rb_jit_cont_each_iseq(rb_iseq_callback callback, void *data);
void rb_jit_cont_finish(void); void rb_jit_cont_finish(void);

15
ruby.c
View file

@ -1613,8 +1613,13 @@ ruby_opt_init(ruby_cmdline_options_t *opt)
#if USE_MJIT #if USE_MJIT
// rb_call_builtin_inits depends on RubyVM::MJIT.enabled? // rb_call_builtin_inits depends on RubyVM::MJIT.enabled?
if (opt->mjit.on) if (opt->mjit.on) {
mjit_enabled = true; mjit_enabled = true;
// rb_threadptr_root_fiber_setup for the initial thread is called before rb_yjit_enabled_p()
// or mjit_enabled becomes true, meaning jit_cont_new is skipped for the initial root fiber.
// Therefore we need to call this again here to set the initial root fiber's jit_cont.
rb_jit_cont_init(); // must be after mjit_enabled = true
}
#endif #endif
Init_ext(); /* load statically linked extensions before rubygems */ Init_ext(); /* load statically linked extensions before rubygems */
@ -1624,10 +1629,6 @@ ruby_opt_init(ruby_cmdline_options_t *opt)
// Make sure the saved_ec of the initial thread's root_fiber is scanned by rb_jit_cont_each_ec. // Make sure the saved_ec of the initial thread's root_fiber is scanned by rb_jit_cont_each_ec.
// //
// rb_threadptr_root_fiber_setup for the initial thread is called before rb_yjit_enabled_p()
// or mjit_enabled becomes true, meaning jit_cont_new is skipped for the root_fiber.
// Therefore we need to call this again here to set the root_fiber's jit_cont.
rb_fiber_init_jit_cont(GET_EC()->fiber_ptr);
#if USE_MJIT #if USE_MJIT
// mjit_init is safe only after rb_call_builtin_inits defines RubyVM::MJIT::Compiler // mjit_init is safe only after rb_call_builtin_inits defines RubyVM::MJIT::Compiler
if (opt->mjit.on) if (opt->mjit.on)
@ -1922,6 +1923,10 @@ process_options(int argc, char **argv, ruby_cmdline_options_t *opt)
#if USE_YJIT #if USE_YJIT
if (FEATURE_SET_P(opt->features, yjit)) { if (FEATURE_SET_P(opt->features, yjit)) {
rb_yjit_init(); rb_yjit_init();
// rb_threadptr_root_fiber_setup for the initial thread is called before rb_yjit_enabled_p()
// or mjit_enabled becomes true, meaning jit_cont_new is skipped for the initial root fiber.
// Therefore we need to call this again here to set the initial root fiber's jit_cont.
rb_jit_cont_init(); // must be after rb_yjit_init(), but before parsing options raises an exception.
} }
#endif #endif
#if USE_MJIT #if USE_MJIT