Skip to content

Commit 5b3d0c9

Browse files
committed
Refactor and extract logic for ModuleFields#setConstantInternal()
1 parent d0cf1f1 commit 5b3d0c9

File tree

2 files changed

+26
-12
lines changed

2 files changed

+26
-12
lines changed

src/main/java/org/truffleruby/collections/ConcurrentOperations.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public abstract class ConcurrentOperations {
2222
* if they are no collisions in the bucket.
2323
* This method might execute the function multiple times, in contrast to computeIfAbsent().
2424
*/
25-
public static <V, K> V getOrCompute(Map<K, V> map, K key, Function<? super K, ? extends V> compute) {
25+
public static <K, V> V getOrCompute(Map<K, V> map, K key, Function<? super K, ? extends V> compute) {
2626
V value = map.get(key);
2727
if (value != null) {
2828
return value;
@@ -33,4 +33,13 @@ public static <V, K> V getOrCompute(Map<K, V> map, K key, Function<? super K, ?
3333
}
3434
}
3535

36+
/**
37+
* Similar to {@link Map#replace(Object, Object, Object)} except that the old value can also be
38+
* null (missing). Returns true if the replace succeeded, or false if it should be retried
39+
* because <code>map.get(key)</code> is no longer associated to <code>oldValue</code>.
40+
*/
41+
public static <K, V> boolean replace(Map<K, V> map, K key, V oldValue, V newValue) {
42+
return oldValue == null ? map.putIfAbsent(key, newValue) == null : map.replace(key, oldValue, newValue);
43+
}
44+
3645
}

src/main/java/org/truffleruby/core/module/ModuleFields.java

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import org.truffleruby.Layouts;
2222
import org.truffleruby.RubyLanguage;
23+
import org.truffleruby.collections.ConcurrentOperations;
2324
import org.truffleruby.RubyContext;
2425
import org.truffleruby.core.klass.ClassNodes;
2526
import org.truffleruby.core.method.MethodFilter;
@@ -333,18 +334,22 @@ private RubyConstant setConstantInternal(RubyContext context, Node currentNode,
333334

334335
SharedObjects.propagate(context, rubyModuleObject, value);
335336

336-
while (true) {
337-
final RubyConstant previous = constants.get(name);
338-
final boolean isPrivate = previous != null && previous.isPrivate();
339-
final boolean isDeprecated = previous != null && previous.isDeprecated();
340-
final SourceSection sourceSection = currentNode != null ? currentNode.getSourceSection() : null;
341-
final RubyConstant newValue = new RubyConstant(rubyModuleObject, value, isPrivate, autoload, isDeprecated, sourceSection);
337+
RubyConstant previous;
338+
RubyConstant newConstant;
339+
do {
340+
previous = constants.get(name);
341+
newConstant = newConstant(currentNode, value, autoload, previous);
342+
} while (!ConcurrentOperations.replace(constants, name, previous, newConstant));
342343

343-
if (previous == null ? constants.putIfAbsent(name, newValue) == null : constants.replace(name, previous, newValue)) {
344-
newConstantsVersion();
345-
return previous;
346-
}
347-
}
344+
newConstantsVersion();
345+
return previous;
346+
}
347+
348+
private RubyConstant newConstant(Node currentNode, Object value, boolean autoload, RubyConstant previous) {
349+
final boolean isPrivate = previous != null && previous.isPrivate();
350+
final boolean isDeprecated = previous != null && previous.isDeprecated();
351+
final SourceSection sourceSection = currentNode != null ? currentNode.getSourceSection() : null;
352+
return new RubyConstant(rubyModuleObject, value, isPrivate, autoload, isDeprecated, sourceSection);
348353
}
349354

350355
@TruffleBoundary

0 commit comments

Comments
 (0)