-
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?
Conversation
…ompilation activity
@@ -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 comment
The 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.
// 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 comment
The 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.
Sidenote: it would not be a problem to show this warning once per engine.
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 comment
The 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.
} | ||
// 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 comment
The 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 null
to filter for all engines in the queue.
"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 comment
The 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?
...com.oracle.truffle.runtime/src/com/oracle/truffle/runtime/hotspot/HotSpotTruffleRuntime.java
Show resolved
Hide resolved
...com.oracle.truffle.runtime/src/com/oracle/truffle/runtime/hotspot/HotSpotTruffleRuntime.java
Show resolved
Hide resolved
* 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 comment
The 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.
"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 comment
The 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.
@@ -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 comment
The 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 comment
The 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 comment
The 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.
Sorry - just saw now that @chumer already pointed this out.
|
||
/** | ||
* Returns the current host compilation activity mode which is one of: | ||
* {@code STOP_COMPILATION}, {@code RUN_COMPILATION} or {@code SHUTDOWN_COMPILATION} |
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).
@@ -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 comment
The 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.
Truffle compilations run in "hosted" mode, i.e. the Truffle runtimes triggers compilations independently of HotSpot's
CompileBroker
. But the results of Truffle compilations are still stored as ordinary nmethods in HotSpot's code cache (with the help of the JVMCI methodjdk.vm.ci.hotspot.HotSpotCodeCacheProvider::installCode()
). The regular JIT compilers are controlled by theCompileBroker
which is aware of the code cache occupancy. If the code cache runs full, theCompileBroker
temporary pauses any subsequent JIT compilations until the code cache gets swept (if running with-XX:+UseCodeCacheFlushing -XX:+MethodFlushing
which is the default) or completely shuts down the JIT compilers if running with-XX:+UseCodeCacheFlushing
.Truffle compiled methods can contribute significantly to the overall code cache occupancy and they can trigger JIT compilation stalls if they fill the code cache up. But the Truffle framework itself is neither aware of the current code cache occupancy, nor of the compilation activity of the
CompileBroker
. If Truffle tries to install a compiled method through JVMCI and the code cache is full, it will silently fail. Currently Truffle interprets such failures as transient errors and basically ignores it. Whenever the corresponding method gets hot again (usually immediately at the next invocation), Truffle will recompile it again just to fail again in the nmethod installation step, if the code cache is still full.When the code cache is tight, this can lead to situations, where Truffle is unnecessarily and repeatedly compiling methods which can't be installed in the code cache but produce a significant CPU load. Instead, Truffle should poll HotSpot's
CompileBroker
compilation activity and paus compilations for the time theCompileBroker
is pausing JIT compilations (or completely shutdown Truffle compilations if theCompileBroker
shut down the JIT compilers).The corresponding JVMCI change is tracked under JDK-8344727: [JVMCI] Export the CompileBroker compilation activity mode for Truffle compiler control.
This PR fixes the problem by checking HotSpot's compilation activity mode in
OptimizedTruffleRuntime::submitForCompilation()
before actually submitting a compilation task to a compile queue. If the compilation activity mode isRUN_COMPILATION
the task is submitted as before without any changes. However, if the compilation activity mode isSTOP_COMPILATION
(i.e. theCompileBroker
has temporarily stopped JIT compilations, we flush the current compile queue (because compiled methods can not be installed anyway) and returnnull
. We also start a timer which can be configured with the newStoppedCompilationRetryDelay
parameter (defaults to 1000ms). AfterStoppedCompilationRetryDelay
we submit a new compilation task, even if the compilation activity mode is notRUN_COMPILATION
. This can help to trigger a code cache cleanup in situations when there are no JIT compilations, because code cache sweeping is only triggered when new nmethods are installed. Finally, when the compilation activity mode isSHUTDOWN_COMPILATION
, we simply shutdown the compilation queue and issue a warning to inform users about the code cache shortage.I've manually tested the change by running an octane benchmark with a very small code cache. Before the change, we could see the following results:
As you can see, out of 2255 compilations, 1702 have been to no purpose, because they failed in the final installation step because of a full code cache (i.e.
BailoutException: Code installation failed: code cache is full: 1702
). The other interesting observation is that although the benchmark terminated after 3min11s wall clock time, it actually consumed 11m19s cpu time, because the Truffle compiler threads where continuously doing useless work.With my changes applied, the picture looks as follows:
As you can see, we compile considerably fewer methods and we only have 16 compilation failures due to a full code cache. At the same time we can see that from the 410 methods which have been enqueued for compilation, 277 have been dequeued because of
Compilation temporary disabled due to full code cache
. Again, the wall clock time of the benchmark was 3m14s but this time, the overall cpu time was just slightly higher with 3m22s because we haven't done such a huge amount of unnecessary compilations any more. Also notice how HotSpot's code cachefull_count
, which is incremented every time when a nmethod can't be installed because of a full code cache, dropped from 3200 to 33.These example were run with
-XX:+UseCodeCacheFlushing -XX:+MethodFlushing
. If the JVM is configured with-XX:-UseCodeCacheFlushing
, the benefits of this change are even higher.Fixes #10133