8075223: revert multithreaded deoptimizing compilation livelock prevention
authorattila
Mon, 16 Mar 2015 18:13:38 +0100
changeset 29538 a1bfbabf475c
parent 29537 e6615d669728
child 29539 b2a8fb583979
8075223: revert multithreaded deoptimizing compilation livelock prevention Reviewed-by: hannesw, sundar
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CompiledFunction.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CompiledFunction.java	Mon Mar 16 16:17:19 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CompiledFunction.java	Mon Mar 16 18:13:38 2015 +0100
@@ -27,7 +27,6 @@
 import static jdk.nashorn.internal.lookup.Lookup.MH;
 import static jdk.nashorn.internal.runtime.UnwarrantedOptimismException.INVALID_PROGRAM_POINT;
 import static jdk.nashorn.internal.runtime.UnwarrantedOptimismException.isValid;
-
 import java.lang.invoke.CallSite;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
@@ -595,7 +594,7 @@
      * switchpoint has been invalidated by a {@code RewriteException} triggered on another thread for this function.
      * This is not a problem, though, as these switch points are always used to produce call sites that fall back to
      * relinking when they are invalidated, and in this case the execution will end up here again. What this method
-     * basically does is reduce such busy-loop relinking while the function is being recompiled on a different thread.
+     * basically does is minimize such busy-loop relinking while the function is being recompiled on a different thread.
      * @param invocationSupplier the supplier that constructs the actual invocation method handle; should use the
      * {@code CompiledFunction} method itself in some capacity.
      * @return a tuple object containing the method handle as created by the supplier and an optimistic assumptions
@@ -603,27 +602,20 @@
      * function can't be further deoptimized).
      */
     private synchronized HandleAndAssumptions getValidOptimisticInvocation(final Supplier<MethodHandle> invocationSupplier) {
-        for(int i = 0; i < 2; ++i) {
+        for(;;) {
             final MethodHandle handle = invocationSupplier.get();
             final SwitchPoint assumptions = canBeDeoptimized() ? optimismInfo.optimisticAssumptions : null;
-            if(i == 0 && assumptions != null && assumptions.hasBeenInvalidated()) {
+            if(assumptions != null && assumptions.hasBeenInvalidated()) {
                 // We can be in a situation where one thread is in the middle of a deoptimizing compilation when we hit
                 // this and thus, it has invalidated the old switch point, but hasn't created the new one yet. Note that
                 // the behavior of invalidating the old switch point before recompilation, and only creating the new one
-                // after recompilation is by design. If we didn't wait here, we would be busy looping through the
-                // fallback path of the invalidated switch point, relinking the call site again with the same
-                // invalidated switch point, invoking the fallback, etc. stealing CPU cycles from the recompilation
-                // task we're dependent on. This can still happen if the switch point gets invalidated after we grabbed
-                // it here, in which case we'll indeed do one busy relink immediately.
-                // On the other hand, in order to avoid a rare livelock, we aren't doing an infinite loop, and we
-                // aren't wait()-ing indefinitely. We'll do at most one, at most 1000ms long wait after which we'll
-                // return the current handle even if it's invalidated (and which'll then trigger one loop through the
-                // relink mechanism). We therefore strike a balance between busy looping and a livelock risk by making
-                // sure that there's at most one iteration of busy loop per second. It is theoretically possible to
-                // correctly implement this without ever risking a livelock, but using this heuristic we eliminate the
-                // chance of the livelock, while still maintaining a good enough busy-looping prevention.
+                // after recompilation is by design. If we didn't wait here for the recompilation to complete, we would
+                // be busy looping through the fallback path of the invalidated switch point, relinking the call site
+                // again with the same invalidated switch point, invoking the fallback, etc. stealing CPU cycles from
+                // the recompilation task we're dependent on. This can still happen if the switch point gets invalidated
+                // after we grabbed it here, in which case we'll indeed do one busy relink immediately.
                 try {
-                    wait(1000L);
+                    wait();
                 } catch (final InterruptedException e) {
                     // Intentionally ignored. There's nothing meaningful we can do if we're interrupted
                 }
@@ -631,7 +623,6 @@
                 return new HandleAndAssumptions(handle, assumptions);
             }
         }
-        throw new AssertionError(); // never reached
     }
 
     private static void relinkComposableInvoker(final CallSite cs, final CompiledFunction inv, final boolean constructor) {