Mercurial > hg > truffle
changeset 10056:a323a9e20f9d
Fixed a few race conditions in the compilation queue.
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/CompilationTask.java Fri Jun 14 15:52:59 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/CompilationTask.java Fri Jun 14 19:12:56 2013 +0200 @@ -28,6 +28,7 @@ import java.lang.reflect.*; import java.util.concurrent.*; +import java.util.concurrent.atomic.*; import com.oracle.graal.api.code.*; import com.oracle.graal.api.code.CallingConvention.Type; @@ -53,8 +54,9 @@ } }; - private volatile boolean cancelled; - private volatile boolean inProgress; + private enum CompilationStatus { + Queued, Running, Canceled + } private final HotSpotGraalRuntime graalRuntime; private final PhasePlan plan; @@ -64,6 +66,7 @@ private final int entryBCI; private final int id; private final int priority; + private final AtomicInteger status; private StructuredGraph graph; @@ -80,6 +83,7 @@ this.entryBCI = entryBCI; this.id = id; this.priority = priority; + this.status = new AtomicInteger(); } public ResolvedJavaMethod getMethod() { @@ -90,16 +94,8 @@ return priority; } - public void cancel() { - cancelled = true; - } - - public boolean isInProgress() { - return inProgress; - } - - public boolean isCancelled() { - return cancelled; + public boolean tryToCancel() { + return tryToChangeStatus(CompilationStatus.Queued, CompilationStatus.Canceled); } public int getEntryBCI() { @@ -109,10 +105,9 @@ public void run() { withinEnqueue.set(Boolean.FALSE); try { - if (cancelled) { + if (!tryToChangeStatus(CompilationStatus.Queued, CompilationStatus.Running)) { return; } - inProgress = true; if (DynamicCompilePriority.getValue()) { int threadPriority = priority < VMToCompilerImpl.SlowQueueCutoff.getValue() ? Thread.NORM_PRIORITY : Thread.MIN_PRIORITY; if (Thread.currentThread().getPriority() != threadPriority) { @@ -124,8 +119,6 @@ if (method.currentTask() == this) { method.setCurrentTask(null); } - graalRuntime.getCompilerToVM().clearQueuedForCompilation(method); - inProgress = false; withinEnqueue.set(Boolean.TRUE); } } @@ -196,6 +189,9 @@ if (ExitVMOnException.getValue()) { System.exit(-1); } + } finally { + assert method.isQueuedForCompilation(); + method.clearQueuedForCompilation(); } stats.finish(method); } @@ -227,6 +223,10 @@ }); } + private boolean tryToChangeStatus(CompilationStatus from, CompilationStatus to) { + return status.compareAndSet(from.ordinal(), to.ordinal()); + } + @Override public int compareTo(CompilationTask o) { if (priority < o.priority) {
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotVMConfig.java Fri Jun 14 15:52:59 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotVMConfig.java Fri Jun 14 19:12:56 2013 +0200 @@ -213,6 +213,12 @@ public int methodAccessFlagsOffset; /** + * JVM_ACC_QUEUED defined in accessFlags.hpp and used for marking a Method object as queued for + * compilation. + */ + public int methodQueuedForCompilationBit; + + /** * Offset of _intrinsic_id in a metaspace Method object. */ public int methodIntrinsicIdOffset;
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/CompilerToVM.java Fri Jun 14 15:52:59 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/CompilerToVM.java Fri Jun 14 19:12:56 2013 +0200 @@ -222,8 +222,6 @@ String getFileName(HotSpotResolvedJavaType method); - void clearQueuedForCompilation(HotSpotResolvedJavaMethod method); - /** * Invalidates the profiling information and restarts profiling upon the next invocation. *
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/CompilerToVMImpl.java Fri Jun 14 15:52:59 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/CompilerToVMImpl.java Fri Jun 14 19:12:56 2013 +0200 @@ -164,9 +164,6 @@ public native String getFileName(HotSpotResolvedJavaType method); @Override - public native void clearQueuedForCompilation(HotSpotResolvedJavaMethod method); - - @Override public native void reprofile(long metaspaceMethod); @Override
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompiler.java Fri Jun 14 15:52:59 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompiler.java Fri Jun 14 19:12:56 2013 +0200 @@ -37,19 +37,13 @@ /** * Compiles a method to machine code. This method is called from the VM * (VMToCompiler::compileMethod). - * - * @return true if the method is in the queue (either added to the queue or already in the - * queue) */ - boolean compileMethod(long metaspaceMethod, HotSpotResolvedObjectType holder, int entryBCI, boolean blocking, int priority) throws Throwable; + void compileMethod(long metaspaceMethod, HotSpotResolvedObjectType holder, int entryBCI, boolean blocking, int priority) throws Throwable; /** * Compiles a method to machine code. - * - * @return true if the method is in the queue (either added to the queue or already in the - * queue) */ - boolean compileMethod(HotSpotResolvedJavaMethod method, int entryBCI, boolean blocking, int priority) throws Throwable; + void compileMethod(HotSpotResolvedJavaMethod method, int entryBCI, boolean blocking, int priority) throws Throwable; void shutdownCompiler() throws Throwable;
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompilerImpl.java Fri Jun 14 15:52:59 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompilerImpl.java Fri Jun 14 19:12:56 2013 +0200 @@ -576,24 +576,20 @@ } @Override - public boolean compileMethod(long metaspaceMethod, final HotSpotResolvedObjectType holder, final int entryBCI, boolean blocking, int priority) throws Throwable { + public void compileMethod(long metaspaceMethod, final HotSpotResolvedObjectType holder, final int entryBCI, boolean blocking, int priority) throws Throwable { HotSpotResolvedJavaMethod method = holder.createMethod(metaspaceMethod); - return compileMethod(method, entryBCI, blocking, priority); + compileMethod(method, entryBCI, blocking, priority); } /** - * Compiles a method to machine code. - * - * @return true if the method is in the queue (either added to the queue or already in the - * queue) + * Compiled a method to machine code. */ - public boolean compileMethod(final HotSpotResolvedJavaMethod method, final int entryBCI, boolean blocking, int priority) throws Throwable { - CompilationTask current = method.currentTask(); + public void compileMethod(final HotSpotResolvedJavaMethod method, final int entryBCI, boolean blocking, int priority) throws Throwable { boolean osrCompilation = entryBCI != StructuredGraph.INVOCATION_ENTRY_BCI; if (osrCompilation && bootstrapRunning) { // no OSR compilations during bootstrap - the compiler is just too slow at this point, // and we know that there are no endless loops - return current != null && (current.isInProgress() || !current.isCancelled()); + return; } if (CompilationTask.withinEnqueue.get()) { @@ -601,39 +597,32 @@ // java.util.concurrent.BlockingQueue is used to implement the compilation worker // queues. If a compiler thread triggers a compilation, then it may be blocked trying // to add something to its own queue. - return current != null && (current.isInProgress() || !current.isCancelled()); + return; } - CompilationTask.withinEnqueue.set(Boolean.TRUE); try { - if (!blocking && current != null) { - if (current.isInProgress()) { - if (current.getEntryBCI() == entryBCI) { - // a compilation with the correct bci is already in progress, so just return - // true - return true; - } - } else { - if (PriorityCompileQueue.getValue()) { - // normally compilation tasks will only be re-queued when they get a - // priority boost, so cancel the old task and add a new one - current.cancel(); - } else if (!current.isCancelled()) { - // without a prioritizing compile queue it makes no sense to re-queue the - // compilation task - return true; - } + CompilationTask.withinEnqueue.set(Boolean.TRUE); + if (!method.tryToQueueForCompilation()) { + // method is already queued + CompilationTask current = method.currentTask(); + if (!PriorityCompileQueue.getValue() || current == null || !current.tryToCancel()) { + // normally compilation tasks will only be re-queued when they get a + // priority boost, so cancel the old task and add a new one + // without a prioritizing compile queue it makes no sense to re-queue the + // compilation task + return; } } + assert method.isQueuedForCompilation(); final OptimisticOptimizations optimisticOpts = new OptimisticOptimizations(method); int id = compileTaskIds.incrementAndGet(); // OSR compilations need to be finished quickly, so they get max priority int queuePriority = osrCompilation ? -1 : priority; CompilationTask task = CompilationTask.create(graalRuntime, createPhasePlan(optimisticOpts, osrCompilation), optimisticOpts, method, entryBCI, id, queuePriority); + if (blocking) { task.runCompilation(); - return false; } else { try { method.setCurrentTask(task); @@ -642,10 +631,8 @@ } else { compileQueue.execute(task); } - return true; } catch (RejectedExecutionException e) { // The compile queue was already shut down. - return false; } } } finally {
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaMethod.java Fri Jun 14 15:52:59 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaMethod.java Fri Jun 14 19:12:56 2013 +0200 @@ -504,4 +504,43 @@ throw new IllegalArgumentException(ex); } } + + public boolean tryToQueueForCompilation() { + // other threads may update certain bits of the access flags field concurrently. So, the + // loop ensures that this method only returns false when another thread has set the + // queuedForCompilation bit. + do { + long address = getAccessFlagsAddress(); + int actualValue = unsafe.getInt(address); + int expectedValue = actualValue & ~graalRuntime().getConfig().methodQueuedForCompilationBit; + if (actualValue != expectedValue) { + return false; + } else { + int newValue = expectedValue | graalRuntime().getConfig().methodQueuedForCompilationBit; + boolean success = unsafe.compareAndSwapInt(null, address, expectedValue, newValue); + if (success) { + return true; + } + } + } while (true); + } + + public void clearQueuedForCompilation() { + long address = getAccessFlagsAddress(); + boolean success; + do { + int actualValue = unsafe.getInt(address); + int newValue = actualValue & ~graalRuntime().getConfig().methodQueuedForCompilationBit; + assert isQueuedForCompilation() : "queued for compilation must be set"; + success = unsafe.compareAndSwapInt(null, address, actualValue, newValue); + } while (!success); + } + + public boolean isQueuedForCompilation() { + return (unsafe.getInt(getAccessFlagsAddress()) & graalRuntime().getConfig().methodQueuedForCompilationBit) != 0; + } + + private long getAccessFlagsAddress() { + return metaspaceMethod + graalRuntime().getConfig().methodAccessFlagsOffset; + } }
--- a/src/share/vm/classfile/vmSymbols.hpp Fri Jun 14 15:52:59 2013 +0200 +++ b/src/share/vm/classfile/vmSymbols.hpp Fri Jun 14 19:12:56 2013 +0200 @@ -352,7 +352,7 @@ template(bootstrap_name, "bootstrap") \ template(shutdownCompiler_name, "shutdownCompiler") \ template(compileMethod_name, "compileMethod") \ - template(compileMethod_signature, "(JLcom/oracle/graal/hotspot/meta/HotSpotResolvedObjectType;IZI)Z") \ + template(compileMethod_signature, "(JLcom/oracle/graal/hotspot/meta/HotSpotResolvedObjectType;IZI)V") \ template(setOption_name, "setOption") \ template(setOption_signature, "(Ljava/lang/String;)Z") \ template(createUnresolvedJavaMethod_name, "createUnresolvedJavaMethod") \
--- a/src/share/vm/compiler/compileBroker.cpp Fri Jun 14 15:52:59 2013 +0200 +++ b/src/share/vm/compiler/compileBroker.cpp Fri Jun 14 19:12:56 2013 +0200 @@ -1123,7 +1123,6 @@ } #ifdef GRAALVM if (!JavaThread::current()->is_compiling()) { - method->set_queued_for_compilation(); GraalCompiler::instance()->compile_method(method, osr_bci, is_compile_blocking(method, osr_bci)); } else { // Recursive compile request => ignore.
--- a/src/share/vm/graal/graalCompiler.cpp Fri Jun 14 15:52:59 2013 +0200 +++ b/src/share/vm/graal/graalCompiler.cpp Fri Jun 14 19:12:56 2013 +0200 @@ -163,9 +163,7 @@ void GraalCompiler::compile_method(methodHandle method, int entry_bci, jboolean blocking) { GRAAL_EXCEPTION_CONTEXT if (!_initialized) { - method->clear_queued_for_compilation(); - method->invocation_counter()->reset(); - method->backedge_counter()->reset(); + CompilationPolicy::policy()->delay_compilation(method()); return; } @@ -173,12 +171,8 @@ ResourceMark rm; JavaThread::current()->set_is_compiling(true); Handle holder = GraalCompiler::createHotSpotResolvedObjectType(method, CHECK); - jboolean success = VMToCompiler::compileMethod(method(), holder, entry_bci, blocking, method->graal_priority()); + VMToCompiler::compileMethod(method(), holder, entry_bci, blocking, method->graal_priority()); JavaThread::current()->set_is_compiling(false); - if (success != JNI_TRUE) { - method->clear_queued_for_compilation(); - CompilationPolicy::policy()->delay_compilation(method()); - } } // Compilation entry point for methods
--- a/src/share/vm/graal/graalCompilerToVM.cpp Fri Jun 14 15:52:59 2013 +0200 +++ b/src/share/vm/graal/graalCompilerToVM.cpp Fri Jun 14 19:12:56 2013 +0200 @@ -714,6 +714,7 @@ set_int("constantPoolHolderOffset", ConstantPool::pool_holder_offset_in_bytes()); set_int("extraStackEntries", Method::extra_stack_entries()); set_int("methodAccessFlagsOffset", in_bytes(Method::access_flags_offset())); + set_int("methodQueuedForCompilationBit", (int) JVM_ACC_QUEUED); set_int("methodIntrinsicIdOffset", Method::intrinsic_id_offset_in_bytes()); set_int("klassHasFinalizerFlag", JVM_ACC_HAS_FINALIZER); set_int("threadExceptionOopOffset", in_bytes(JavaThread::exception_oop_offset())); @@ -941,11 +942,6 @@ return result; C2V_END -C2V_VMENTRY(void, clearQueuedForCompilation, (JNIEnv *jniEnv, jobject, jobject resolvedMethod)) - methodHandle method = getMethodFromHotSpotMethod(JNIHandles::resolve(resolvedMethod)); - method->clear_queued_for_compilation(); -C2V_END - C2V_VMENTRY(jobject, getCode, (JNIEnv *jniEnv, jobject, jlong codeBlob)) ResourceMark rm; HandleMark hm; @@ -1247,7 +1243,6 @@ {CC"getLineNumberTable", CC"("HS_RESOLVED_METHOD")[J", FN_PTR(getLineNumberTable)}, {CC"getLocalVariableTable", CC"("HS_RESOLVED_METHOD")["LOCAL, FN_PTR(getLocalVariableTable)}, {CC"getFileName", CC"("HS_RESOLVED_JAVA_TYPE")"STRING, FN_PTR(getFileName)}, - {CC"clearQueuedForCompilation", CC"("HS_RESOLVED_METHOD")V", FN_PTR(clearQueuedForCompilation)}, {CC"reprofile", CC"("METASPACE_METHOD")V", FN_PTR(reprofile)}, {CC"invalidateInstalledCode", CC"(J)V", FN_PTR(invalidateInstalledCode)}, {CC"isInstalledCodeValid", CC"(J)Z", FN_PTR(isInstalledCodeValid)},
--- a/src/share/vm/graal/graalVMToCompiler.cpp Fri Jun 14 15:52:59 2013 +0200 +++ b/src/share/vm/graal/graalVMToCompiler.cpp Fri Jun 14 19:12:56 2013 +0200 @@ -96,11 +96,11 @@ return result.get_jboolean(); } -jboolean VMToCompiler::compileMethod(Method* method, Handle holder, int entry_bci, jboolean blocking, int priority) { +void VMToCompiler::compileMethod(Method* method, Handle holder, int entry_bci, jboolean blocking, int priority) { assert(method != NULL, "just checking"); assert(!holder.is_null(), "just checking"); Thread* THREAD = Thread::current(); - JavaValue result(T_BOOLEAN); + JavaValue result(T_VOID); JavaCallArguments args; args.push_oop(instance()); args.push_long((jlong) (address) method); @@ -110,7 +110,6 @@ args.push_int(priority); JavaCalls::call_interface(&result, vmToCompilerKlass(), vmSymbols::compileMethod_name(), vmSymbols::compileMethod_signature(), &args, THREAD); check_pending_exception("Error while calling compileMethod"); - return result.get_jboolean(); } void VMToCompiler::shutdownCompiler() {
--- a/src/share/vm/graal/graalVMToCompiler.hpp Fri Jun 14 15:52:59 2013 +0200 +++ b/src/share/vm/graal/graalVMToCompiler.hpp Fri Jun 14 19:12:56 2013 +0200 @@ -57,7 +57,7 @@ static jboolean setOption(Handle option); // public abstract boolean compileMethod(long vmId, String name, int entry_bci, boolean blocking); - static jboolean compileMethod(Method* method, Handle holder, int entry_bci, jboolean blocking, int priority); + static void compileMethod(Method* method, Handle holder, int entry_bci, jboolean blocking, int priority); // public abstract void shutdownCompiler(); static void shutdownCompiler();