Skip to content

Commit c5608ab

Browse files
committed
Monitor: avoid repeated calls to rb_fiber_current()
That call is surprisingly expensive, so trying doing it once in `#synchronize` and then passing the fiber to enter and exit saves quite a few cycles.
1 parent ef4490d commit c5608ab

File tree

2 files changed

+95
-59
lines changed

2 files changed

+95
-59
lines changed

ext/monitor/monitor.c

Lines changed: 71 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,10 @@ monitor_ptr(VALUE monitor)
5050
return mc;
5151
}
5252

53-
static int
54-
mc_owner_p(struct rb_monitor *mc)
53+
static bool
54+
mc_owner_p(struct rb_monitor *mc, VALUE current_fiber)
5555
{
56-
return mc->owner == rb_fiber_current();
56+
return mc->owner == current_fiber;
5757
}
5858

5959
/*
@@ -67,17 +67,44 @@ monitor_try_enter(VALUE monitor)
6767
{
6868
struct rb_monitor *mc = monitor_ptr(monitor);
6969

70-
if (!mc_owner_p(mc)) {
70+
VALUE current_fiber = rb_fiber_current();
71+
if (!mc_owner_p(mc, current_fiber)) {
7172
if (!rb_mutex_trylock(mc->mutex)) {
7273
return Qfalse;
7374
}
74-
RB_OBJ_WRITE(monitor, &mc->owner, rb_fiber_current());
75+
RB_OBJ_WRITE(monitor, &mc->owner, current_fiber);
7576
mc->count = 0;
7677
}
7778
mc->count += 1;
7879
return Qtrue;
7980
}
8081

82+
83+
struct monitor_args {
84+
VALUE monitor;
85+
struct rb_monitor *mc;
86+
VALUE current_fiber;
87+
};
88+
89+
static inline void
90+
monitor_args_init(struct monitor_args *args, VALUE monitor)
91+
{
92+
args->monitor = monitor;
93+
args->mc = monitor_ptr(monitor);
94+
args->current_fiber = rb_fiber_current();
95+
}
96+
97+
static void
98+
monitor_enter0(struct monitor_args *args)
99+
{
100+
if (!mc_owner_p(args->mc, args->current_fiber)) {
101+
rb_mutex_lock(args->mc->mutex);
102+
RB_OBJ_WRITE(args->monitor, &args->mc->owner, args->current_fiber);
103+
args->mc->count = 0;
104+
}
105+
args->mc->count++;
106+
}
107+
81108
/*
82109
* call-seq:
83110
* enter -> nil
@@ -87,27 +114,44 @@ monitor_try_enter(VALUE monitor)
87114
static VALUE
88115
monitor_enter(VALUE monitor)
89116
{
90-
struct rb_monitor *mc = monitor_ptr(monitor);
91-
if (!mc_owner_p(mc)) {
92-
rb_mutex_lock(mc->mutex);
93-
RB_OBJ_WRITE(monitor, &mc->owner, rb_fiber_current());
94-
mc->count = 0;
95-
}
96-
mc->count++;
117+
struct monitor_args args;
118+
monitor_args_init(&args, monitor);
119+
monitor_enter0(&args);
97120
return Qnil;
98121
}
99122

123+
static inline void
124+
monitor_check_owner0(struct monitor_args *args)
125+
{
126+
if (!mc_owner_p(args->mc, args->current_fiber)) {
127+
rb_raise(rb_eThreadError, "current fiber not owner");
128+
}
129+
}
130+
100131
/* :nodoc: */
101132
static VALUE
102133
monitor_check_owner(VALUE monitor)
103134
{
104-
struct rb_monitor *mc = monitor_ptr(monitor);
105-
if (!mc_owner_p(mc)) {
106-
rb_raise(rb_eThreadError, "current fiber not owner");
107-
}
135+
struct monitor_args args;
136+
monitor_args_init(&args, monitor);
137+
monitor_check_owner0(&args);
108138
return Qnil;
109139
}
110140

141+
static void
142+
monitor_exit0(struct monitor_args *args)
143+
{
144+
monitor_check_owner(args->monitor);
145+
146+
if (args->mc->count <= 0) rb_bug("monitor_exit: count:%d", (int)args->mc->count);
147+
args->mc->count--;
148+
149+
if (args->mc->count == 0) {
150+
RB_OBJ_WRITE(args->monitor, &args->mc->owner, Qnil);
151+
rb_mutex_unlock(args->mc->mutex);
152+
}
153+
}
154+
111155
/*
112156
* call-seq:
113157
* exit -> nil
@@ -117,17 +161,9 @@ monitor_check_owner(VALUE monitor)
117161
static VALUE
118162
monitor_exit(VALUE monitor)
119163
{
120-
monitor_check_owner(monitor);
121-
122-
struct rb_monitor *mc = monitor_ptr(monitor);
123-
124-
if (mc->count <= 0) rb_bug("monitor_exit: count:%d", (int)mc->count);
125-
mc->count--;
126-
127-
if (mc->count == 0) {
128-
RB_OBJ_WRITE(monitor, &mc->owner, Qnil);
129-
rb_mutex_unlock(mc->mutex);
130-
}
164+
struct monitor_args args;
165+
monitor_args_init(&args, monitor);
166+
monitor_exit0(&args);
131167
return Qnil;
132168
}
133169

@@ -144,7 +180,7 @@ static VALUE
144180
monitor_owned_p(VALUE monitor)
145181
{
146182
struct rb_monitor *mc = monitor_ptr(monitor);
147-
return (rb_mutex_locked_p(mc->mutex) && mc_owner_p(mc)) ? Qtrue : Qfalse;
183+
return rb_mutex_locked_p(mc->mutex) && mc_owner_p(mc, rb_fiber_current()) ? Qtrue : Qfalse;
148184
}
149185

150186
static VALUE
@@ -210,9 +246,10 @@ monitor_sync_body(VALUE monitor)
210246
}
211247

212248
static VALUE
213-
monitor_sync_ensure(VALUE monitor)
249+
monitor_sync_ensure(VALUE v_args)
214250
{
215-
return monitor_exit(monitor);
251+
monitor_exit0((struct monitor_args *)v_args);
252+
return Qnil;
216253
}
217254

218255
/*
@@ -226,8 +263,10 @@ monitor_sync_ensure(VALUE monitor)
226263
static VALUE
227264
monitor_synchronize(VALUE monitor)
228265
{
229-
monitor_enter(monitor);
230-
return rb_ensure(monitor_sync_body, monitor, monitor_sync_ensure, monitor);
266+
struct monitor_args args;
267+
monitor_args_init(&args, monitor);
268+
monitor_enter0(&args);
269+
return rb_ensure(monitor_sync_body, (VALUE)&args, monitor_sync_ensure, (VALUE)&args);
231270
}
232271

233272
void

thread_sync.c

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -239,23 +239,34 @@ thread_mutex_remove(rb_thread_t *thread, rb_mutex_t *mutex)
239239
}
240240

241241
static void
242-
mutex_set_owner(VALUE self, rb_thread_t *th, rb_fiber_t *fiber)
242+
mutex_set_owner(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber)
243243
{
244-
rb_mutex_t *mutex = mutex_ptr(self);
245-
246244
mutex->thread = th->self;
247245
mutex->fiber_serial = rb_fiber_serial(fiber);
248246
}
249247

250248
static void
251-
mutex_locked(rb_thread_t *th, rb_fiber_t *fiber, VALUE self)
249+
mutex_locked(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber)
252250
{
253-
rb_mutex_t *mutex = mutex_ptr(self);
254-
255-
mutex_set_owner(self, th, fiber);
251+
mutex_set_owner(mutex, th, fiber);
256252
thread_mutex_insert(th, mutex);
257253
}
258254

255+
static inline bool
256+
mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber)
257+
{
258+
if (mutex->fiber_serial == 0) {
259+
RUBY_DEBUG_LOG("%p ok", mutex);
260+
261+
mutex_locked(mutex, th, fiber);
262+
return true;
263+
}
264+
else {
265+
RUBY_DEBUG_LOG("%p ng", mutex);
266+
return false;
267+
}
268+
}
269+
259270
/*
260271
* call-seq:
261272
* mutex.try_lock -> true or false
@@ -266,21 +277,7 @@ mutex_locked(rb_thread_t *th, rb_fiber_t *fiber, VALUE self)
266277
VALUE
267278
rb_mutex_trylock(VALUE self)
268279
{
269-
rb_mutex_t *mutex = mutex_ptr(self);
270-
271-
if (mutex->fiber_serial == 0) {
272-
RUBY_DEBUG_LOG("%p ok", mutex);
273-
274-
rb_fiber_t *fiber = GET_EC()->fiber_ptr;
275-
rb_thread_t *th = GET_THREAD();
276-
277-
mutex_locked(th, fiber, self);
278-
return Qtrue;
279-
}
280-
else {
281-
RUBY_DEBUG_LOG("%p ng", mutex);
282-
return Qfalse;
283-
}
280+
return RBOOL(mutex_trylock(mutex_ptr(self), GET_THREAD(), GET_EC()->fiber_ptr));
284281
}
285282

286283
static VALUE
@@ -321,7 +318,7 @@ do_mutex_lock(VALUE self, int interruptible_p)
321318
rb_raise(rb_eThreadError, "can't be called from trap context");
322319
}
323320

324-
if (rb_mutex_trylock(self) == Qfalse) {
321+
if (!mutex_trylock(mutex, th, fiber)) {
325322
if (mutex->fiber_serial == rb_fiber_serial(fiber)) {
326323
rb_raise(rb_eThreadError, "deadlock; recursive locking");
327324
}
@@ -342,7 +339,7 @@ do_mutex_lock(VALUE self, int interruptible_p)
342339
rb_ensure(call_rb_fiber_scheduler_block, self, delete_from_waitq, (VALUE)&sync_waiter);
343340

344341
if (!mutex->fiber_serial) {
345-
mutex_set_owner(self, th, fiber);
342+
mutex_set_owner(mutex, th, fiber);
346343
}
347344
}
348345
else {
@@ -383,7 +380,7 @@ do_mutex_lock(VALUE self, int interruptible_p)
383380

384381
// unlocked by another thread while sleeping
385382
if (!mutex->fiber_serial) {
386-
mutex_set_owner(self, th, fiber);
383+
mutex_set_owner(mutex, th, fiber);
387384
}
388385

389386
rb_ractor_sleeper_threads_dec(th->ractor);
@@ -402,7 +399,7 @@ do_mutex_lock(VALUE self, int interruptible_p)
402399
}
403400
RUBY_VM_CHECK_INTS_BLOCKING(th->ec); /* may release mutex */
404401
if (!mutex->fiber_serial) {
405-
mutex_set_owner(self, th, fiber);
402+
mutex_set_owner(mutex, th, fiber);
406403
}
407404
}
408405
else {
@@ -421,7 +418,7 @@ do_mutex_lock(VALUE self, int interruptible_p)
421418
}
422419

423420
if (saved_ints) th->ec->interrupt_flag = saved_ints;
424-
if (mutex->fiber_serial == rb_fiber_serial(fiber)) mutex_locked(th, fiber, self);
421+
if (mutex->fiber_serial == rb_fiber_serial(fiber)) mutex_locked(mutex, th, fiber);
425422
}
426423

427424
RUBY_DEBUG_LOG("%p locked", mutex);

0 commit comments

Comments
 (0)