-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Truffle compiler control based on HotSpot's CompileBroker compilation activity #10135
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -359,4 +359,31 @@ default ResolvedJavaType resolveType(MetaAccessProvider metaAccess, String class | |
*/ | ||
boolean isSuppressedFailure(TruffleCompilable compilable, Supplier<String> serializedException); | ||
|
||
/** | ||
* Represents HotSpot's compilation activity mode which is one of: | ||
* {@code stop_compilation = 0}, {@code run_compilation = 1} or {@code shutdown_compilation = 2} | ||
* Should be in sync with the {@code CompilerActivity} enum in {@code hotspot/share/compiler/compileBroker.hpp} | ||
*/ | ||
enum CompilationActivityMode { | ||
STOP_COMPILATION, | ||
RUN_COMPILATION, | ||
SHUTDOWN_COMPILATION; | ||
|
||
static public CompilationActivityMode fromInteger(int i) { | ||
return switch (i) { | ||
case 0 -> STOP_COMPILATION; | ||
case 1 -> RUN_COMPILATION; | ||
case 2 -> SHUTDOWN_COMPILATION; | ||
default -> throw new RuntimeException("Invalid CompilationActivityMode " + i); | ||
}; | ||
} | ||
} | ||
|
||
/** | ||
* Returns the current host compilation activity mode which is one of: | ||
* {@code STOP_COMPILATION}, {@code RUN_COMPILATION} or {@code SHUTDOWN_COMPILATION} | ||
*/ | ||
default CompilationActivityMode getCompilationActivityMode() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to have the getCompilationActivityMode in this interface. This interface is intended for methods that get called by the Graal compiler. You can move this abstract specification directly into OptimizedTruffleRuntime and implemented more concretely in HotSpotTruffleRutnime. |
||
return CompilationActivityMode.RUN_COMPILATION; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -158,6 +158,14 @@ public ExceptionAction apply(String s) { | |
// TODO: GR-29949 | ||
public static final OptionKey<Long> CompilerIdleDelay = new OptionKey<>(10000L); | ||
|
||
@Option(help = "Before the Truffle runtime submits an OptimizedCallTarget for compilation, it checks for the " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: The help text is extensive but seems a bit implementation heavy. I think it is enough for a user of this option to know what happens that is observable to the user. In other words specific enum names like STOP_COMPILATION won't help understanding the semantics of this option. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, this option is ignored if not running on HotSpot right? Might be worth pointing that out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sorry - just saw now that @chumer already pointed this out. |
||
"compilation activity mode in the host VM. If the activity mode is 'STOP_COMPILATION' because " + | ||
"of a full code cache, no new compilation requests are submitted and the compilation queue is flushed. " + | ||
"After 'StoppedCompilationRetryDelay' milliseconds new compilations will be submitted again " + | ||
"(which might trigger a sweep of the code cache and a reset of the compilation activity mode in the host JVM).", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well this sounds like it would be supported in all runtimes. But it is not. SVM does not support this yet. So we should mention this only works for HotSpot right now. Note that OptimizedRuntimeOptions are shared across all optimizing runtimes this includes SVM. |
||
usageSyntax = "<ms>", category = OptionCategory.EXPERT) | ||
public static final OptionKey<Long> StoppedCompilationRetryDelay = new OptionKey<>(1000L); | ||
|
||
@Option(help = "Manually set the number of compiler threads. By default, the number of compiler threads is scaled with the number of available cores on the CPU.", usageSyntax = "[1, inf)", category = OptionCategory.EXPERT, // | ||
stability = OptionStability.STABLE, sandbox = SandboxPolicy.UNTRUSTED) // | ||
public static final OptionKey<Integer> CompilerThreads = new OptionKey<>(-1); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,7 @@ | |
package com.oracle.truffle.runtime; | ||
|
||
import static com.oracle.truffle.runtime.OptimizedRuntimeOptions.CompilerIdleDelay; | ||
import static com.oracle.truffle.runtime.OptimizedRuntimeOptions.StoppedCompilationRetryDelay; | ||
|
||
import java.io.CharArrayWriter; | ||
import java.io.PrintWriter; | ||
|
@@ -912,10 +913,45 @@ private void notifyCompilationFailure(OptimizedCallTarget callTarget, Throwable | |
protected void onEngineCreated(EngineData engine) { | ||
} | ||
|
||
private long stoppedCompilationTime = 0; | ||
private boolean logShutdownCompilations = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. submitForCompilation is not thread-safe. We need to handle these fields to be modified from multpile threads. So we would need to synchronize accessing these fields. |
||
|
||
@SuppressWarnings("try") | ||
public CompilationTask submitForCompilation(OptimizedCallTarget optimizedCallTarget, boolean lastTierCompilation) { | ||
Priority priority = new Priority(optimizedCallTarget.getCallAndLoopCount(), lastTierCompilation ? Priority.Tier.LAST : Priority.Tier.FIRST); | ||
return getCompileQueue().submitCompilation(priority, optimizedCallTarget); | ||
BackgroundCompileQueue queue = getCompileQueue(); | ||
CompilationActivityMode compilationActivityMode = getCompilationActivityMode(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should move this logic into OptimizedCallTarget#compile instead and have a more efficent check, similar to how we already check for OptimizedCallTarget.compilationTask? Currently we would take a lock for every OptimizedCallTarget.call that triggers a compilation if compilation is paused. We should also use the OptimizedCallTrage.EngineData class to store any state associated with the retry time. I know it most likely will be the same for all engines, but its a principle we work by to avoid that engines can affect each other. So we also avoid sharing locks. |
||
if (compilationActivityMode == CompilationActivityMode.RUN_COMPILATION || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor, but I think we could write this as a switch and easier to read retry handling (probably opinionated):
|
||
(stoppedCompilationTime != 0 && System.currentTimeMillis() - stoppedCompilationTime > optimizedCallTarget.getOptionValue(StoppedCompilationRetryDelay))) { | ||
stoppedCompilationTime = 0; | ||
Priority priority = new Priority(optimizedCallTarget.getCallAndLoopCount(), lastTierCompilation ? Priority.Tier.LAST : Priority.Tier.FIRST); | ||
return queue.submitCompilation(priority, optimizedCallTarget); | ||
} else if (compilationActivityMode == CompilationActivityMode.STOP_COMPILATION) { | ||
if (stoppedCompilationTime == 0) { | ||
stoppedCompilationTime = System.currentTimeMillis(); | ||
} | ||
// Flush the compilations queue. There's still a chance that compilation will be re-enabled | ||
// eventually, if the hosts code cache can be cleaned up. | ||
for (OptimizedCallTarget target : queue.getQueuedTargets(optimizedCallTarget.engine)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This just cancels compilation for one engine only, whereas the JVMCI indication is for all engines. Either we move this code to the engine level or we pass |
||
target.cancelCompilation("Compilation temporary disabled due to full code cache."); | ||
} | ||
} else { | ||
// Compilation was shut down permanently because the hosts code cache ran full and | ||
// the host was configured without support for code cache sweeping. | ||
assert compilationActivityMode == CompilationActivityMode.SHUTDOWN_COMPILATION; | ||
TruffleLogger logger = optimizedCallTarget.engine.getLogger("engine"); | ||
// The logger can be null if the engine is closed. | ||
if (logger != null && logShutdownCompilations) { | ||
logShutdownCompilations = false; | ||
logger.log(Level.WARNING, "Truffle host compilations permanently disabled because of full code cache. " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do not really call Truffle compilations host compilations, that is typically reserved for compilations of HotSpot directly. Maybe we should just change it to Truffle compilations to avoid confusion. |
||
"Increase the code cache size using '-XX:ReservedCodeCacheSize=' and/or run with '-XX:+UseCodeCacheFlushing -XX:+MethodFlushing'."); | ||
} | ||
try { | ||
queue.shutdownAndAwaitTermination(100 /* milliseconds */); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you describe a bit what you are trying to achieve here? why would it help to block the entire interpreter thread if this happens? |
||
} catch (RuntimeException re) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would this fail? If you want to catch InterruptedException and ignore that, that would be fine. But I do not like catching wildcard exceptions and ignore them, I would prefer to rethrow. |
||
// Best effort, ignore failure | ||
} | ||
} | ||
return null; | ||
} | ||
|
||
@SuppressWarnings("all") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not list the enum constants in this javadoc - just one more location you need to remember to update if enum constants are added (or deleted/renamed).