Skip to content

Commit 28b195f

Browse files
committed
Store the fiber_serial in the EC to allow inlining
Mutexes spend a significant amount of time in `rb_fiber_serial` because it can't be inlined (except with LTO). The fiber struct is opaque the so function can't be defined as inlineable. Ideally the while fiber struct would not be opaque to the rest of Ruby core, but it's tricky to do. Instead we can store the fiber serial in the execution context itself, and make its access cheaper: ``` $ hyperfine './miniruby-baseline --yjit /tmp/mut.rb' './miniruby-inline-serial --yjit /tmp/mut.rb' Benchmark 1: ./miniruby-baseline --yjit /tmp/mut.rb Time (mean ± σ): 4.011 s ± 0.084 s [User: 3.977 s, System: 0.011 s] Range (min … max): 3.950 s … 4.245 s 10 runs Benchmark 2: ./miniruby-inline-serial --yjit /tmp/mut.rb Time (mean ± σ): 3.495 s ± 0.150 s [User: 3.448 s, System: 0.009 s] Range (min … max): 3.340 s … 3.869 s 10 runs Summary ./miniruby-inline-serial --yjit /tmp/mut.rb ran 1.15 ± 0.05 times faster than ./miniruby-baseline --yjit /tmp/mut.rb ``` ```ruby i = 10_000_000 mut = Mutex.new while i > 0 i -= 1 mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } end ```
1 parent 85b40c5 commit 28b195f

File tree

5 files changed

+38
-40
lines changed

5 files changed

+38
-40
lines changed

cont.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,6 @@ struct rb_fiber_struct {
268268

269269
unsigned int killed : 1;
270270

271-
rb_serial_t serial;
272-
273271
struct coroutine_context context;
274272
struct fiber_pool_stack stack;
275273
};
@@ -1012,13 +1010,6 @@ rb_fiber_threadptr(const rb_fiber_t *fiber)
10121010
return fiber->cont.saved_ec.thread_ptr;
10131011
}
10141012

1015-
rb_serial_t
1016-
rb_fiber_serial(const rb_fiber_t *fiber)
1017-
{
1018-
VM_ASSERT(fiber->serial >= 1);
1019-
return fiber->serial;
1020-
}
1021-
10221013
static VALUE
10231014
cont_thread_value(const rb_context_t *cont)
10241015
{
@@ -2026,10 +2017,10 @@ fiber_t_alloc(VALUE fiber_value, unsigned int blocking)
20262017
fiber->cont.type = FIBER_CONTEXT;
20272018
fiber->blocking = blocking;
20282019
fiber->killed = 0;
2029-
fiber->serial = next_fiber_serial(th->ractor);
20302020
cont_init(&fiber->cont, th);
20312021

20322022
fiber->cont.saved_ec.fiber_ptr = fiber;
2023+
fiber->cont.saved_ec.fiber_serial = next_fiber_serial(th->ractor);
20332024
rb_ec_clear_vm_stack(&fiber->cont.saved_ec);
20342025

20352026
fiber->prev = NULL;
@@ -2576,10 +2567,10 @@ rb_threadptr_root_fiber_setup(rb_thread_t *th)
25762567
}
25772568
fiber->cont.type = FIBER_CONTEXT;
25782569
fiber->cont.saved_ec.fiber_ptr = fiber;
2570+
fiber->cont.saved_ec.fiber_serial = next_fiber_serial(th->ractor);
25792571
fiber->cont.saved_ec.thread_ptr = th;
25802572
fiber->blocking = 1;
25812573
fiber->killed = 0;
2582-
fiber->serial = next_fiber_serial(th->ractor);
25832574
fiber_status_set(fiber, FIBER_RESUMED); /* skip CREATED */
25842575
th->ec = &fiber->cont.saved_ec;
25852576
cont_init_jit_cont(&fiber->cont);

internal/cont.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ VALUE rb_fiber_inherit_storage(struct rb_execution_context_struct *ec, struct rb
3131
VALUE rb_fiberptr_self(struct rb_fiber_struct *fiber);
3232
unsigned int rb_fiberptr_blocking(struct rb_fiber_struct *fiber);
3333
struct rb_execution_context_struct * rb_fiberptr_get_ec(struct rb_fiber_struct *fiber);
34-
rb_serial_t rb_fiber_serial(const struct rb_fiber_struct *fiber);
3534

35+
static inline rb_serial_t
36+
rb_ec_fiber_serial(struct rb_execution_context_struct *ec)
37+
{
38+
VM_ASSERT(ec->fiber_serial >= 1);
39+
return ec->fiber_serial;
40+
}
3641
#endif /* INTERNAL_CONT_H */

thread.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ rb_threadptr_unlock_all_locking_mutexes(rb_thread_t *th)
454454

455455
// rb_warn("mutex #<%p> was not unlocked by thread #<%p>", (void *)mutex, (void*)th);
456456
VM_ASSERT(mutex->fiber_serial);
457-
const char *error_message = rb_mutex_unlock_th(mutex, th, NULL);
457+
const char *error_message = rb_mutex_unlock_th(mutex, th, 0);
458458
if (error_message) rb_bug("invalid keeping_mutexes: %s", error_message);
459459
}
460460
}
@@ -5283,7 +5283,7 @@ rb_thread_shield_owned(VALUE self)
52835283

52845284
rb_mutex_t *m = mutex_ptr(mutex);
52855285

5286-
return m->fiber_serial == rb_fiber_serial(GET_EC()->fiber_ptr);
5286+
return m->fiber_serial == rb_ec_fiber_serial(GET_EC());
52875287
}
52885288

52895289
/*
@@ -5302,7 +5302,7 @@ rb_thread_shield_wait(VALUE self)
53025302

53035303
if (!mutex) return Qfalse;
53045304
m = mutex_ptr(mutex);
5305-
if (m->fiber_serial == rb_fiber_serial(GET_EC()->fiber_ptr)) return Qnil;
5305+
if (m->fiber_serial == rb_ec_fiber_serial(GET_EC())) return Qnil;
53065306
rb_thread_shield_waiting_inc(self);
53075307
rb_mutex_lock(mutex);
53085308
rb_thread_shield_waiting_dec(self);
@@ -5860,7 +5860,7 @@ rb_check_deadlock(rb_ractor_t *r)
58605860
}
58615861
else if (th->locking_mutex) {
58625862
rb_mutex_t *mutex = mutex_ptr(th->locking_mutex);
5863-
if (mutex->fiber_serial == rb_fiber_serial(th->ec->fiber_ptr) || (!mutex->fiber_serial && !ccan_list_empty(&mutex->waitq))) {
5863+
if (mutex->fiber_serial == rb_ec_fiber_serial(th->ec) || (!mutex->fiber_serial && !ccan_list_empty(&mutex->waitq))) {
58645864
found = 1;
58655865
}
58665866
}

thread_sync.c

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ static void rb_mutex_abandon_all(rb_mutex_t *mutexes);
8181
static void rb_mutex_abandon_keeping_mutexes(rb_thread_t *th);
8282
static void rb_mutex_abandon_locking_mutex(rb_thread_t *th);
8383
#endif
84-
static const char* rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber);
84+
static const char* rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial);
8585

8686
/*
8787
* Document-class: Thread::Mutex
@@ -133,7 +133,7 @@ mutex_free(void *ptr)
133133
{
134134
rb_mutex_t *mutex = ptr;
135135
if (mutex_locked_p(mutex)) {
136-
const char *err = rb_mutex_unlock_th(mutex, rb_thread_ptr(mutex->thread), NULL);
136+
const char *err = rb_mutex_unlock_th(mutex, rb_thread_ptr(mutex->thread), 0);
137137
if (err) rb_bug("%s", err);
138138
}
139139
ruby_xfree(ptr);
@@ -221,26 +221,26 @@ thread_mutex_remove(rb_thread_t *thread, rb_mutex_t *mutex)
221221
}
222222

223223
static void
224-
mutex_set_owner(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber)
224+
mutex_set_owner(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial)
225225
{
226226
mutex->thread = th->self;
227-
mutex->fiber_serial = rb_fiber_serial(fiber);
227+
mutex->fiber_serial = fiber_serial;
228228
}
229229

230230
static void
231-
mutex_locked(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber)
231+
mutex_locked(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial)
232232
{
233-
mutex_set_owner(mutex, th, fiber);
233+
mutex_set_owner(mutex, th, fiber_serial);
234234
thread_mutex_insert(th, mutex);
235235
}
236236

237237
static inline bool
238-
mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber)
238+
mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial)
239239
{
240240
if (mutex->fiber_serial == 0) {
241241
RUBY_DEBUG_LOG("%p ok", mutex);
242242

243-
mutex_locked(mutex, th, fiber);
243+
mutex_locked(mutex, th, fiber_serial);
244244
return true;
245245
}
246246
else {
@@ -252,7 +252,7 @@ mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber)
252252
static VALUE
253253
rb_mut_trylock(rb_execution_context_t *ec, VALUE self)
254254
{
255-
return RBOOL(mutex_trylock(mutex_ptr(self), ec->thread_ptr, ec->fiber_ptr));
255+
return RBOOL(mutex_trylock(mutex_ptr(self), ec->thread_ptr, rb_ec_fiber_serial(ec)));
256256
}
257257

258258
VALUE
@@ -262,9 +262,9 @@ rb_mutex_trylock(VALUE self)
262262
}
263263

264264
static VALUE
265-
mutex_owned_p(rb_fiber_t *fiber, rb_mutex_t *mutex)
265+
mutex_owned_p(rb_serial_t fiber_serial, rb_mutex_t *mutex)
266266
{
267-
return RBOOL(mutex->fiber_serial == rb_fiber_serial(fiber));
267+
return RBOOL(mutex->fiber_serial == fiber_serial);
268268
}
269269

270270
static VALUE
@@ -305,6 +305,7 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p)
305305
rb_execution_context_t *ec = args->ec;
306306
rb_thread_t *th = ec->thread_ptr;
307307
rb_fiber_t *fiber = ec->fiber_ptr;
308+
rb_serial_t fiber_serial = rb_ec_fiber_serial(ec);
308309
rb_mutex_t *mutex = args->mutex;
309310
rb_atomic_t saved_ints = 0;
310311

@@ -314,12 +315,12 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p)
314315
rb_raise(rb_eThreadError, "can't be called from trap context");
315316
}
316317

317-
if (!mutex_trylock(mutex, th, fiber)) {
318-
if (mutex->fiber_serial == rb_fiber_serial(fiber)) {
318+
if (!mutex_trylock(mutex, th, fiber_serial)) {
319+
if (mutex->fiber_serial == fiber_serial) {
319320
rb_raise(rb_eThreadError, "deadlock; recursive locking");
320321
}
321322

322-
while (mutex->fiber_serial != rb_fiber_serial(fiber)) {
323+
while (mutex->fiber_serial != fiber_serial) {
323324
VM_ASSERT(mutex->fiber_serial != 0);
324325

325326
VALUE scheduler = rb_fiber_scheduler_current();
@@ -335,7 +336,7 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p)
335336
rb_ensure(call_rb_fiber_scheduler_block, self, delete_from_waitq, (VALUE)&sync_waiter);
336337

337338
if (!mutex->fiber_serial) {
338-
mutex_set_owner(mutex, th, fiber);
339+
mutex_set_owner(mutex, th, fiber_serial);
339340
}
340341
}
341342
else {
@@ -376,7 +377,7 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p)
376377

377378
// unlocked by another thread while sleeping
378379
if (!mutex->fiber_serial) {
379-
mutex_set_owner(mutex, th, fiber);
380+
mutex_set_owner(mutex, th, fiber_serial);
380381
}
381382

382383
rb_ractor_sleeper_threads_dec(th->ractor);
@@ -389,13 +390,13 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p)
389390
if (interruptible_p) {
390391
/* release mutex before checking for interrupts...as interrupt checking
391392
* code might call rb_raise() */
392-
if (mutex->fiber_serial == rb_fiber_serial(fiber)) {
393+
if (mutex->fiber_serial == fiber_serial) {
393394
mutex->thread = Qfalse;
394395
mutex->fiber_serial = 0;
395396
}
396397
RUBY_VM_CHECK_INTS_BLOCKING(th->ec); /* may release mutex */
397398
if (!mutex->fiber_serial) {
398-
mutex_set_owner(mutex, th, fiber);
399+
mutex_set_owner(mutex, th, fiber_serial);
399400
}
400401
}
401402
else {
@@ -414,13 +415,13 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p)
414415
}
415416

416417
if (saved_ints) th->ec->interrupt_flag = saved_ints;
417-
if (mutex->fiber_serial == rb_fiber_serial(fiber)) mutex_locked(mutex, th, fiber);
418+
if (mutex->fiber_serial == fiber_serial) mutex_locked(mutex, th, fiber_serial);
418419
}
419420

420421
RUBY_DEBUG_LOG("%p locked", mutex);
421422

422423
// assertion
423-
if (mutex_owned_p(fiber, mutex) == Qfalse) rb_bug("do_mutex_lock: mutex is not owned.");
424+
if (mutex_owned_p(fiber_serial, mutex) == Qfalse) rb_bug("do_mutex_lock: mutex is not owned.");
424425

425426
return self;
426427
}
@@ -455,7 +456,7 @@ rb_mutex_lock(VALUE self)
455456
static VALUE
456457
rb_mut_owned_p(rb_execution_context_t *ec, VALUE self)
457458
{
458-
return mutex_owned_p(ec->fiber_ptr, mutex_ptr(self));
459+
return mutex_owned_p(rb_ec_fiber_serial(ec), mutex_ptr(self));
459460
}
460461

461462
VALUE
@@ -465,14 +466,14 @@ rb_mutex_owned_p(VALUE self)
465466
}
466467

467468
static const char *
468-
rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber)
469+
rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial)
469470
{
470471
RUBY_DEBUG_LOG("%p", mutex);
471472

472473
if (mutex->fiber_serial == 0) {
473474
return "Attempt to unlock a mutex which is not locked";
474475
}
475-
else if (fiber && mutex->fiber_serial != rb_fiber_serial(fiber)) {
476+
else if (fiber_serial && mutex->fiber_serial != fiber_serial) {
476477
return "Attempt to unlock a mutex which is locked by another thread/fiber";
477478
}
478479

@@ -516,7 +517,7 @@ do_mutex_unlock(struct mutex_args *args)
516517
rb_mutex_t *mutex = args->mutex;
517518
rb_thread_t *th = rb_ec_thread_ptr(args->ec);
518519

519-
err = rb_mutex_unlock_th(mutex, th, args->ec->fiber_ptr);
520+
err = rb_mutex_unlock_th(mutex, th, rb_ec_fiber_serial(args->ec));
520521
if (err) rb_raise(rb_eThreadError, "%s", err);
521522
}
522523

vm_core.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,6 +1041,7 @@ struct rb_execution_context_struct {
10411041

10421042
rb_fiber_t *fiber_ptr;
10431043
struct rb_thread_struct *thread_ptr;
1044+
rb_serial_t fiber_serial;
10441045

10451046
/* storage (ec (fiber) local) */
10461047
struct rb_id_table *local_storage;

0 commit comments

Comments
 (0)