Skip to content

Commit cd5758c

Browse files
committed
Refactor to not use the monitor's lock for condition variables.
1 parent caf8d05 commit cd5758c

File tree

3 files changed

+17
-52
lines changed

3 files changed

+17
-52
lines changed

lib/mri/monitor.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,7 @@ class ConditionVariable < ::ConditionVariable
107107
# even if no other thread doesn't signal.
108108
#
109109
def wait(timeout = nil)
110-
if timeout
111-
raise ArgumentError, 'Timeout must be positive' if timeout < 0
112-
timeout = timeout * 1_000_000_000
113-
timeout = Primitive.rb_num2long(timeout)
114-
end
115-
116-
Primitive.condition_variable_wait(self, nil, timeout)
110+
super(@mon_mutex, timeout)
117111
end
118112

119113
#
@@ -133,6 +127,12 @@ def wait_until
133127
wait
134128
end
135129
end
130+
131+
private
132+
133+
def initialize(mutex)
134+
@mon_mutex = mutex
135+
end
136136
end
137137

138138
def self.extend_object(obj)
@@ -192,7 +192,7 @@ def mon_synchronize(&block)
192192
# receiver.
193193
#
194194
def new_cond
195-
Primitive.mutex_linked_condition_variable(ConditionVariable, @mon_mutex)
195+
ConditionVariable.new(@mon_mutex)
196196
end
197197

198198
# Use <tt>extend MonitorMixin</tt> or <tt>include MonitorMixin</tt> instead

src/main/java/org/truffleruby/core/monitor/TruffleMonitorNodes.java

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,45 +9,24 @@
99
*/
1010
package org.truffleruby.core.monitor;
1111

12-
import java.util.concurrent.locks.Condition;
13-
import java.util.concurrent.locks.ReentrantLock;
14-
1512
import com.oracle.truffle.api.dsl.Cached;
1613
import com.oracle.truffle.api.dsl.Specialization;
17-
import com.oracle.truffle.api.object.Shape;
1814
import com.oracle.truffle.api.profiles.BranchProfile;
1915

2016
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
2117
import org.truffleruby.builtins.CoreModule;
2218
import org.truffleruby.builtins.Primitive;
23-
import org.truffleruby.core.klass.RubyClass;
2419
import org.truffleruby.core.mutex.MutexOperations;
25-
import org.truffleruby.core.mutex.RubyConditionVariable;
2620
import org.truffleruby.core.mutex.RubyMutex;
2721
import org.truffleruby.core.proc.RubyProc;
2822
import org.truffleruby.core.thread.GetCurrentRubyThreadNode;
2923
import org.truffleruby.core.thread.RubyThread;
3024
import org.truffleruby.language.control.RaiseException;
31-
import org.truffleruby.language.objects.AllocationTracing;
3225
import org.truffleruby.language.yield.CallBlockNode;
3326

3427
@CoreModule("Truffle::MonitorOperations")
3528
public abstract class TruffleMonitorNodes {
3629

37-
@Primitive(name = "mutex_linked_condition_variable")
38-
public abstract static class LinkedConditionVariableNode extends CoreMethodArrayArgumentsNode {
39-
40-
@Specialization
41-
protected RubyConditionVariable linkedConditionVariable(RubyClass rubyClass, RubyMutex mutex) {
42-
final ReentrantLock condLock = mutex.lock;
43-
final Condition condition = MutexOperations.newCondition(condLock);
44-
final Shape shape = getLanguage().conditionVariableShape;
45-
final RubyConditionVariable instance = new RubyConditionVariable(rubyClass, shape, condLock, condition);
46-
AllocationTracing.trace(instance, this);
47-
return instance;
48-
}
49-
}
50-
5130
@Primitive(name = "monitor_synchronize")
5231
public abstract static class SynchronizeNode extends CoreMethodArrayArgumentsNode {
5332

src/main/java/org/truffleruby/core/mutex/ConditionVariableNodes.java

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -82,26 +82,6 @@ protected RubyConditionVariable withTimeout(RubyConditionVariable condVar, RubyM
8282
return condVar;
8383
}
8484

85-
@Specialization
86-
protected RubyConditionVariable noMutexNoTimeout(RubyConditionVariable condVar, Nil mutex, Nil timeout,
87-
@Cached GetCurrentRubyThreadNode getCurrentRubyThreadNode,
88-
@Cached BranchProfile errorProfile) {
89-
final RubyThread thread = getCurrentRubyThreadNode.execute();
90-
91-
waitInternal(condVar, null, thread, -1);
92-
return condVar;
93-
}
94-
95-
@Specialization
96-
protected RubyConditionVariable noMutexWithTimeout(RubyConditionVariable condVar, Nil mutex, long timeout,
97-
@Cached GetCurrentRubyThreadNode getCurrentRubyThreadNode,
98-
@Cached BranchProfile errorProfile) {
99-
final RubyThread thread = getCurrentRubyThreadNode.execute();
100-
101-
waitInternal(condVar, null, thread, timeout);
102-
return condVar;
103-
}
104-
10585
@TruffleBoundary
10686
private void waitInternal(RubyConditionVariable conditionVariable, ReentrantLock mutexLock,
10787
RubyThread thread, long durationInNanos) {
@@ -123,9 +103,11 @@ private void waitInternal(RubyConditionVariable conditionVariable, ReentrantLock
123103
// If there is an interrupt, it should be consumed by condition.await() and the Ruby Thread sleep status
124104
// must imply being ready to be interrupted by Thread#{run,wakeup}.
125105
condLock.lock();
106+
int holdCount = 0;
126107
try {
127-
if (mutexLock != null) {
108+
while (mutexLock.isHeldByCurrentThread()) {
128109
mutexLock.unlock();
110+
holdCount++;
129111
}
130112

131113
conditionVariable.waiters++;
@@ -143,8 +125,12 @@ private void waitInternal(RubyConditionVariable conditionVariable, ReentrantLock
143125
}
144126
} finally {
145127
condLock.unlock();
146-
if (mutexLock != null) {
147-
MutexOperations.internalLockEvenWithException(getContext(), mutexLock, this);
128+
MutexOperations.internalLockEvenWithException(getContext(), mutexLock, this);
129+
if (holdCount > 1) {
130+
// We know we already hold the lock, so we can skip the rest of the logic at this point.
131+
for (int i = 1; i < holdCount; i++) {
132+
mutexLock.tryLock();
133+
}
148134
}
149135
}
150136
}

0 commit comments

Comments
 (0)