Skip to content

Commit b9a2905

Browse files
committed
Remove the --fiber-leave-context option
* It is too hard to maintain and handle both possibilities. * Performance-wise it seems very similar, and the default of leaving the context is much better to avoid triggering Truffle multithreading. * This means SafepointPredicate.ALL_THREADS_AND_FIBERS will need special handling so it can work even when Fibers leave the context.
1 parent 66ce8e4 commit b9a2905

File tree

6 files changed

+13
-59
lines changed

6 files changed

+13
-59
lines changed

src/main/java/org/truffleruby/core/fiber/FiberManager.java

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,7 @@ public static void waitForInitialization(RubyContext context, RubyFiber fiber, N
8989

9090
private void fiberMain(RubyContext context, RubyFiber fiber, RubyProc block, Node currentNode) {
9191
assert !fiber.isRootFiber() : "Root Fibers execute threadMain() and not fiberMain()";
92-
93-
final boolean entered = !context.getOptions().FIBER_LEAVE_CONTEXT;
94-
assert entered == context.getEnv().getContext().isEntered();
92+
assertNotEntered("Fibers should start unentered to avoid triggering multithreading");
9593

9694
final Thread thread = Thread.currentThread();
9795
final SourceSection sourceSection = block.sharedMethodInfo.getSourceSection();
@@ -104,14 +102,9 @@ private void fiberMain(RubyContext context, RubyFiber fiber, RubyProc block, Nod
104102
fiber.initializedLatch.countDown();
105103

106104
final FiberMessage message = waitMessage(fiber, currentNode);
107-
final TruffleContext truffleContext = entered ? null : context.getEnv().getContext();
105+
final TruffleContext truffleContext = context.getEnv().getContext();
108106

109-
final Object prev;
110-
if (!entered) {
111-
prev = truffleContext.enter(currentNode);
112-
} else {
113-
prev = null;
114-
}
107+
final Object prev = truffleContext.enter(currentNode);
115108
language.setupCurrentThread(thread, fiber.rubyThread);
116109

117110
FiberMessage lastMessage = null;
@@ -146,9 +139,7 @@ private void fiberMain(RubyContext context, RubyFiber fiber, RubyProc block, Nod
146139
fiber.alive = false;
147140
// Leave context before addToMessageQueue() -> parent Fiber starts executing
148141
language.setupCurrentThread(thread, null);
149-
if (!entered) {
150-
truffleContext.leave(currentNode, prev);
151-
}
142+
truffleContext.leave(currentNode, prev);
152143
cleanup(fiber, thread);
153144
thread.setName(oldName);
154145

@@ -205,7 +196,7 @@ class State {
205196
}
206197

207198
private void assertNotEntered(String reason) {
208-
assert !context.getOptions().FIBER_LEAVE_CONTEXT || !context.getEnv().getContext().isEntered() : reason;
199+
assert !context.getEnv().getContext().isEntered() : reason;
209200
}
210201

211202
@TruffleBoundary

src/main/java/org/truffleruby/core/thread/ThreadManager.java

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ public class ThreadManager {
7878
private final Set<RubyThread> runningRubyThreads = ConcurrentHashMap.newKeySet();
7979

8080
/** The set of Java threads TruffleRuby created, and is responsible to exit in {@link #killAndWaitOtherThreads()}.
81-
* Needs to be weak because {@link RubyLanguage#disposeThread} is called late for Fibers with --fiber-leave-context
82-
* as that uses a "host thread" (disposeThread is only called on Context#close for those), and there might be
83-
* multiple Fibers per such thread with the pool. If a Thread is unreachable we do not need to wait for it in
84-
* {@link #killAndWaitOtherThreads()}, but otherwise we need to and we can never remove from this Set as otherwise
85-
* we cannot guarantee we wait until the Thread truly finishes execution. */
81+
* Needs to be weak because {@link RubyLanguage#disposeThread} is called late as Fibers use a "host thread"
82+
* (disposeThread is only called on Context#close for those), and there might be multiple Fibers per such thread
83+
* with the pool. If a Thread is unreachable we do not need to wait for it in {@link #killAndWaitOtherThreads()},
84+
* but otherwise we need to and we can never remove from this Set as otherwise we cannot guarantee we wait until the
85+
* Thread truly finishes execution. */
8686
private final Set<Thread> rubyManagedThreads = Collections
8787
.newSetFromMap(Collections.synchronizedMap(new WeakHashMap<>()));
8888

@@ -155,12 +155,7 @@ private Thread createFiberJavaThread(Runnable runnable) {
155155
throw new UnsupportedOperationException("fibers should not be created while pre-initializing the context");
156156
}
157157

158-
final Thread thread;
159-
if (context.getOptions().FIBER_LEAVE_CONTEXT) {
160-
thread = new Thread(runnable); // context.getEnv().createUnenteredThread(runnable);
161-
} else {
162-
thread = context.getEnv().createThread(runnable);
163-
}
158+
final Thread thread = new Thread(runnable); // context.getEnv().createUnenteredThread(runnable);
164159
rubyManagedThreads.add(thread); // need to be set before initializeThread()
165160
thread.setUncaughtExceptionHandler((javaThread, throwable) -> {
166161
System.err.println("Throwable escaped Fiber pool thread:");
@@ -472,25 +467,14 @@ public interface BlockingAction<T> {
472467
T block() throws InterruptedException;
473468
}
474469

475-
/** Only leaves the context if FIBER_LEAVE_CONTEXT is true */
476470
public <T> T leaveAndEnter(TruffleContext truffleContext, Node currentNode, Supplier<T> runWhileOutsideContext) {
477471
assert truffleContext.isEntered();
478-
479-
if (context.getOptions().FIBER_LEAVE_CONTEXT) {
480-
return truffleContext.leaveAndEnter(currentNode, runWhileOutsideContext);
481-
} else {
482-
return runWhileOutsideContext.get();
483-
}
472+
return truffleContext.leaveAndEnter(currentNode, runWhileOutsideContext);
484473
}
485474

486475
/** Only use when the context is not entered. */
487476
@TruffleBoundary
488477
public <T> void retryWhileInterrupted(Node currentNode, TruffleSafepoint.Interruptible<T> interruptible, T object) {
489-
if (!context.getOptions().FIBER_LEAVE_CONTEXT) {
490-
runUntilResultKeepStatus(currentNode, interruptible, object);
491-
return;
492-
}
493-
494478
assert !context.getEnv().getContext().isEntered() : "Use runUntilResult*() when entered";
495479
boolean interrupted = false;
496480
try {

src/main/java/org/truffleruby/options/Options.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,6 @@ public class Options {
6969
public final boolean PATCHING;
7070
/** --hashing-deterministic=false */
7171
public final boolean HASHING_DETERMINISTIC;
72-
/** --fiber-leave-context=true */
73-
public final boolean FIBER_LEAVE_CONTEXT;
7472
/** --fiber-pool=true */
7573
public final boolean FIBER_POOL;
7674
/** --log-subprocess=false */
@@ -228,7 +226,6 @@ public Options(Env env, OptionValues options, LanguageOptions languageOptions) {
228226
PATTERN_MATCHING = options.get(OptionsCatalog.PATTERN_MATCHING_KEY);
229227
PATCHING = options.get(OptionsCatalog.PATCHING_KEY);
230228
HASHING_DETERMINISTIC = options.get(OptionsCatalog.HASHING_DETERMINISTIC_KEY);
231-
FIBER_LEAVE_CONTEXT = options.get(OptionsCatalog.FIBER_LEAVE_CONTEXT_KEY);
232229
FIBER_POOL = options.get(OptionsCatalog.FIBER_POOL_KEY);
233230
LOG_SUBPROCESS = options.get(OptionsCatalog.LOG_SUBPROCESS_KEY);
234231
WARN_LOCALE = options.get(OptionsCatalog.WARN_LOCALE_KEY);
@@ -345,8 +342,6 @@ public Object fromDescriptor(OptionDescriptor descriptor) {
345342
return PATCHING;
346343
case "ruby.hashing-deterministic":
347344
return HASHING_DETERMINISTIC;
348-
case "ruby.fiber-leave-context":
349-
return FIBER_LEAVE_CONTEXT;
350345
case "ruby.fiber-pool":
351346
return FIBER_POOL;
352347
case "ruby.log-subprocess":

src/main/ruby/truffleruby/core/main.rb

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,7 @@ def to_s
5656
end
5757

5858
show_backtraces = -> {
59-
if Truffle::Boot.get_option('fiber-leave-context')
60-
$stderr.puts 'All Thread backtraces (use --fiber-leave-context=false to see all Fiber backtraces):'
61-
else
62-
$stderr.puts 'All Fiber backtraces:'
63-
end
59+
$stderr.puts 'All Thread backtraces:'
6460
Primitive.all_fibers_backtraces.each do |fiber, backtrace|
6561
$stderr.puts "#{fiber} of #{Primitive.fiber_thread(fiber)}", backtrace, nil
6662
end

src/options.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ EXPERT:
102102
LAZY_TRANSLATION_USER: [lazy-translation-user, boolean, LAZY_CALLTARGETS, 'Lazily translation of stdlib, gem and user source files']
103103
PATCHING: [patching, boolean, true, Use patching]
104104
HASHING_DETERMINISTIC: [hashing-deterministic, boolean, false, Produce deterministic hash values]
105-
FIBER_LEAVE_CONTEXT: [fiber-leave-context, boolean, true, 'Leave the TruffleContext when suspending a Fiber (avoids triggering multithreading)']
106105
FIBER_POOL: [fiber-pool, boolean, true, 'Use a thread pool to speed up creating Fibers']
107106
LOG_SUBPROCESS: [log-subprocess, boolean, false, 'Log whenever a subprocess is created']
108107
WARN_LOCALE: [warn-locale, boolean, true, 'Warn when the system locale is not set properly']

src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ public class OptionsCatalog {
5252
public static final OptionKey<Boolean> LAZY_TRANSLATION_USER_KEY = new OptionKey<>(LAZY_CALLTARGETS_KEY.getDefaultValue());
5353
public static final OptionKey<Boolean> PATCHING_KEY = new OptionKey<>(true);
5454
public static final OptionKey<Boolean> HASHING_DETERMINISTIC_KEY = new OptionKey<>(false);
55-
public static final OptionKey<Boolean> FIBER_LEAVE_CONTEXT_KEY = new OptionKey<>(true);
5655
public static final OptionKey<Boolean> FIBER_POOL_KEY = new OptionKey<>(true);
5756
public static final OptionKey<Boolean> LOG_SUBPROCESS_KEY = new OptionKey<>(false);
5857
public static final OptionKey<Boolean> WARN_LOCALE_KEY = new OptionKey<>(true);
@@ -387,13 +386,6 @@ public class OptionsCatalog {
387386
.stability(OptionStability.EXPERIMENTAL)
388387
.build();
389388

390-
public static final OptionDescriptor FIBER_LEAVE_CONTEXT = OptionDescriptor
391-
.newBuilder(FIBER_LEAVE_CONTEXT_KEY, "ruby.fiber-leave-context")
392-
.help("Leave the TruffleContext when suspending a Fiber (avoids triggering multithreading)")
393-
.category(OptionCategory.EXPERT)
394-
.stability(OptionStability.EXPERIMENTAL)
395-
.build();
396-
397389
public static final OptionDescriptor FIBER_POOL = OptionDescriptor
398390
.newBuilder(FIBER_POOL_KEY, "ruby.fiber-pool")
399391
.help("Use a thread pool to speed up creating Fibers")
@@ -1223,8 +1215,6 @@ public static OptionDescriptor fromName(String name) {
12231215
return PATCHING;
12241216
case "ruby.hashing-deterministic":
12251217
return HASHING_DETERMINISTIC;
1226-
case "ruby.fiber-leave-context":
1227-
return FIBER_LEAVE_CONTEXT;
12281218
case "ruby.fiber-pool":
12291219
return FIBER_POOL;
12301220
case "ruby.log-subprocess":
@@ -1482,7 +1472,6 @@ public static OptionDescriptor[] allDescriptors() {
14821472
LAZY_TRANSLATION_USER,
14831473
PATCHING,
14841474
HASHING_DETERMINISTIC,
1485-
FIBER_LEAVE_CONTEXT,
14861475
FIBER_POOL,
14871476
LOG_SUBPROCESS,
14881477
WARN_LOCALE,

0 commit comments

Comments
 (0)