# HG changeset patch # User fzhinkin # Date 1419926982 -10800 # Node ID 6d382dc493e5db91e93a9b2dfc28da87168f40cc # Parent 18701d7781e7252d2e37f54c26d3c9046d57baa1 8050486: compiler/rtm/ tests fail due to monitor deflation at safepoint synchronization Reviewed-by: kvn, iignatyev diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/rtm/locking/TestRTMAbortRatio.java --- a/hotspot/test/compiler/rtm/locking/TestRTMAbortRatio.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/rtm/locking/TestRTMAbortRatio.java Tue Dec 30 11:09:42 2014 +0300 @@ -127,10 +127,7 @@ @Override public String[] getMethodsToCompileNames() { - return new String[] { - getMethodWithLockName(), - Unsafe.class.getName() + "::addressSize" - }; + return new String[] { getMethodWithLockName() }; } public void lock(boolean abort) { @@ -148,10 +145,12 @@ public static void main(String args[]) throws Throwable { Asserts.assertGTE(args.length, 1, "One argument required."); Test t = new Test(); - if (Boolean.valueOf(args[0])) { + boolean shouldBeInflated = Boolean.valueOf(args[0]); + if (shouldBeInflated) { AbortProvoker.inflateMonitor(t.monitor); } for (int i = 0; i < Test.TOTAL_ITERATIONS; i++) { + AbortProvoker.verifyMonitorState(t.monitor, shouldBeInflated); t.lock(i >= Test.WARMUP_ITERATIONS); } } diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java --- a/hotspot/test/compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java Tue Dec 30 11:09:42 2014 +0300 @@ -157,10 +157,7 @@ @Override public String[] getMethodsToCompileNames() { - return new String[] { - getMethodWithLockName(), - sun.misc.Unsafe.class.getName() + "::forceAbort" - }; + return new String[] { getMethodWithLockName() }; } public void forceAbort(int a[], boolean abort) { @@ -183,13 +180,15 @@ public static void main(String args[]) throws Throwable { Test t = new Test(); - if (Boolean.valueOf(args[0])) { + boolean shouldBeInflated = Boolean.valueOf(args[0]); + if (shouldBeInflated) { AbortProvoker.inflateMonitor(t.monitor); } int tmp[] = new int[1]; for (int i = 0; i < Test.ITERATIONS; i++ ) { + AbortProvoker.verifyMonitorState(t.monitor, shouldBeInflated); if (i == Test.RANGE_CHECK_AT) { t.forceAbort(new int[0], false); } else { diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java --- a/hotspot/test/compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java Tue Dec 30 11:09:42 2014 +0300 @@ -130,10 +130,7 @@ @Override public String[] getMethodsToCompileNames() { - return new String[] { - getMethodWithLockName(), - sun.misc.Unsafe.class.getName() + "::addressSize" - }; + return new String[] { getMethodWithLockName() }; } public void forceAbort(boolean abort) { @@ -151,11 +148,12 @@ public static void main(String args[]) throws Throwable { Asserts.assertGTE(args.length, 1, "One argument required."); Test t = new Test(); - - if (Boolean.valueOf(args[0])) { + boolean shouldBeInflated = Boolean.valueOf(args[0]); + if (shouldBeInflated) { AbortProvoker.inflateMonitor(t.monitor); } for (int i = 0; i < AbortProvoker.DEFAULT_ITERATIONS; i++) { + AbortProvoker.verifyMonitorState(t.monitor, shouldBeInflated); t.forceAbort( i == TestRTMDeoptOnLowAbortRatio.LOCKING_THRESHOLD); } diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/rtm/locking/TestRTMLockingThreshold.java --- a/hotspot/test/compiler/rtm/locking/TestRTMLockingThreshold.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/rtm/locking/TestRTMLockingThreshold.java Tue Dec 30 11:09:42 2014 +0300 @@ -143,10 +143,7 @@ @Override public String[] getMethodsToCompileNames() { - return new String[] { - getMethodWithLockName(), - sun.misc.Unsafe.class.getName() + "::addressSize" - }; + return new String[] { getMethodWithLockName() }; } public void lock(boolean abort) { @@ -164,11 +161,12 @@ public static void main(String args[]) throws Throwable { Asserts.assertGTE(args.length, 1, "One argument required."); Test t = new Test(); - - if (Boolean.valueOf(args[0])) { + boolean shouldBeInflated = Boolean.valueOf(args[0]); + if (shouldBeInflated) { AbortProvoker.inflateMonitor(t.monitor); } for (int i = 0; i < Test.TOTAL_ITERATIONS; i++) { + AbortProvoker.verifyMonitorState(t.monitor, shouldBeInflated); t.lock(i % 2 == 1); } } diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/rtm/locking/TestRTMTotalCountIncrRate.java --- a/hotspot/test/compiler/rtm/locking/TestRTMTotalCountIncrRate.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/rtm/locking/TestRTMTotalCountIncrRate.java Tue Dec 30 11:09:42 2014 +0300 @@ -117,9 +117,7 @@ @Override public String[] getMethodsToCompileNames() { - return new String[] { - getMethodWithLockName() - }; + return new String[] { getMethodWithLockName() }; } public void lock() { @@ -135,11 +133,13 @@ public static void main(String args[]) throws Throwable { Asserts.assertGTE(args.length, 1, "One argument required."); Test test = new Test(); - - if (Boolean.valueOf(args[0])) { + boolean shouldBeInflated = Boolean.valueOf(args[0]); + if (shouldBeInflated) { AbortProvoker.inflateMonitor(test.monitor); } for (long i = 0L; i < Test.TOTAL_ITERATIONS; i++) { + AbortProvoker.verifyMonitorState(test.monitor, + shouldBeInflated); test.lock(); } } diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/rtm/locking/TestUseRTMAfterLockInflation.java --- a/hotspot/test/compiler/rtm/locking/TestUseRTMAfterLockInflation.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/rtm/locking/TestUseRTMAfterLockInflation.java Tue Dec 30 11:09:42 2014 +0300 @@ -52,7 +52,7 @@ * Compiled method invoked {@code AbortProvoker.DEFAULT_ITERATIONS} times before * lock inflation and the same amount of times after inflation. * As a result total locks count should be equal to - * {@code 2*AbortProvoker.DEFAULT_ITERATIONS}. + * {@code 2 * AbortProvoker.DEFAULT_ITERATIONS}. * It is a pretty strict assertion which could fail if some retriable abort * happened: it could be {@code AbortType.RETRIABLE} or * {@code AbortType.MEM_CONFLICT}, but unfortunately abort can has both these @@ -101,7 +101,6 @@ } public static class Test { - /** * Usage: * Test <provoker type> @@ -113,10 +112,12 @@ AbortProvoker provoker = AbortType.lookup(Integer.valueOf(args[0])).provoker(); for (int i = 0; i < AbortProvoker.DEFAULT_ITERATIONS; i++) { + AbortProvoker.verifyMonitorState(provoker, false /*deflated*/); provoker.forceAbort(); } provoker.inflateMonitor(); for (int i = 0; i < AbortProvoker.DEFAULT_ITERATIONS; i++) { + AbortProvoker.verifyMonitorState(provoker, true /*inflated*/); provoker.forceAbort(); } } diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/testlibrary/rtm/AbortProvoker.java --- a/hotspot/test/compiler/testlibrary/rtm/AbortProvoker.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/testlibrary/rtm/AbortProvoker.java Tue Dec 30 11:09:42 2014 +0300 @@ -29,8 +29,7 @@ import java.util.concurrent.CyclicBarrier; import com.oracle.java.testlibrary.Asserts; -import com.oracle.java.testlibrary.Utils; -import sun.misc.Unsafe; +import sun.hotspot.WhiteBox; /** * Base class for different transactional execution abortion @@ -38,6 +37,9 @@ */ public abstract class AbortProvoker implements CompilableTest { public static final long DEFAULT_ITERATIONS = 10000L; + private static final WhiteBox WHITE_BOX = WhiteBox.getWhiteBox(); + @SuppressWarnings("unused") + private static int sharedState = 0; /** * Inflates monitor associated with object {@code monitor}. * Inflation is forced by entering the same monitor from @@ -48,36 +50,76 @@ * @throws Exception if something went wrong. */ public static Object inflateMonitor(Object monitor) throws Exception { - Unsafe unsafe = Utils.getUnsafe(); CyclicBarrier barrier = new CyclicBarrier(2); Runnable inflatingRunnable = () -> { - unsafe.monitorEnter(monitor); - try { - barrier.await(); - barrier.await(); - } catch (InterruptedException | BrokenBarrierException e) { - throw new RuntimeException( - "Synchronization issue occurred.", e); - } finally { - unsafe.monitorExit(monitor); + synchronized (monitor) { + try { + barrier.await(); + } catch (BrokenBarrierException | InterruptedException e) { + throw new RuntimeException( + "Synchronization issue occurred.", e); + } + try { + monitor.wait(); + } catch (InterruptedException e) { + throw new AssertionError("The thread waiting on an" + + " inflated monitor was interrupted, thus test" + + " results may be incorrect.", e); + } } }; Thread t = new Thread(inflatingRunnable); + t.setDaemon(true); t.start(); // Wait until thread t enters the monitor. barrier.await(); - // At this point monitor will be owned by thread t, - // so our attempt to enter the same monitor will force - // monitor inflation. - Asserts.assertFalse(unsafe.tryMonitorEnter(monitor), - "Not supposed to enter the monitor first"); - barrier.await(); - t.join(); + synchronized (monitor) { + // At this point thread t is already waiting on the monitor. + // Modifying static field just to avoid lock's elimination. + sharedState++; + } + verifyMonitorState(monitor, true /* inflated */); return monitor; } + /** + * Verifies that {@code monitor} is a stack-lock or inflated lock depending + * on {@code shouldBeInflated} value. If {@code monitor} is inflated while + * it is expected that it should be a stack-lock, then this method attempts + * to deflate it by forcing a safepoint and then verifies the state once + * again. + * + * @param monitor monitor to be verified. + * @param shouldBeInflated flag indicating whether or not monitor is + * expected to be inflated. + * @throws RuntimeException if the {@code monitor} in a wrong state. + */ + public static void verifyMonitorState(Object monitor, + boolean shouldBeInflated) { + if (!shouldBeInflated && WHITE_BOX.isMonitorInflated(monitor)) { + WHITE_BOX.forceSafepoint(); + } + Asserts.assertEQ(WHITE_BOX.isMonitorInflated(monitor), shouldBeInflated, + "Monitor in a wrong state."); + } + /** + * Verifies that monitor used by the {@code provoker} is a stack-lock or + * inflated lock depending on {@code shouldBeInflated} value. If such + * monitor is inflated while it is expected that it should be a stack-lock, + * then this method attempts to deflate it by forcing a safepoint and then + * verifies the state once again. + * + * @param provoker AbortProvoker whose monitor's state should be verified. + * @param shouldBeInflated flag indicating whether or not monitor is + * expected to be inflated. + * @throws RuntimeException if the {@code monitor} in a wrong state. + */ + public static void verifyMonitorState(AbortProvoker provoker, + boolean shouldBeInflated) { + verifyMonitorState(provoker.monitor, shouldBeInflated); + } /** * Get instance of specified AbortProvoker, inflate associated monitor @@ -120,6 +162,7 @@ } for (long i = 0; i < iterations; i++) { + AbortProvoker.verifyMonitorState(provoker, monitorShouldBeInflated); provoker.forceAbort(); } } diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/testlibrary/rtm/BusyLock.java --- a/hotspot/test/compiler/testlibrary/rtm/BusyLock.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/testlibrary/rtm/BusyLock.java Tue Dec 30 11:09:42 2014 +0300 @@ -77,7 +77,7 @@ } } - public void test() { + public void syncAndTest() { try { barrier.await(); // wait until monitor is locked by a ::run method @@ -85,6 +85,10 @@ } catch (InterruptedException | BrokenBarrierException e) { throw new RuntimeException("Synchronization error happened.", e); } + test(); + } + + public void test() { synchronized(monitor) { BusyLock.field++; } @@ -130,7 +134,7 @@ Thread t = new Thread(busyLock); t.start(); - busyLock.test(); + busyLock.syncAndTest(); t.join(); } } diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/testlibrary/rtm/MemoryConflictProvoker.java --- a/hotspot/test/compiler/testlibrary/rtm/MemoryConflictProvoker.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/testlibrary/rtm/MemoryConflictProvoker.java Tue Dec 30 11:09:42 2014 +0300 @@ -69,11 +69,6 @@ * Accesses and modifies memory region from within the transaction. */ public void transactionalRegion() { - try { - barrier.await(); - } catch (InterruptedException | BrokenBarrierException e) { - throw new RuntimeException(e); - } for (int i = 0; i < MemoryConflictProvoker.INNER_ITERATIONS; i++) { synchronized(monitor) { MemoryConflictProvoker.field--; @@ -86,6 +81,11 @@ try { Thread t = new Thread(conflictingThread); t.start(); + try { + barrier.await(); + } catch (InterruptedException | BrokenBarrierException e) { + throw new RuntimeException(e); + } transactionalRegion(); t.join(); } catch (Exception e) { diff -r 18701d7781e7 -r 6d382dc493e5 hotspot/test/compiler/testlibrary/rtm/RTMTestBase.java --- a/hotspot/test/compiler/testlibrary/rtm/RTMTestBase.java Tue Dec 30 11:07:49 2014 +0300 +++ b/hotspot/test/compiler/testlibrary/rtm/RTMTestBase.java Tue Dec 30 11:09:42 2014 +0300 @@ -240,7 +240,8 @@ Collections.addAll(finalVMOpts, "-Xcomp", "-server", "-XX:-TieredCompilation", "-XX:+UseRTMLocking", CommandLineOptionTest.UNLOCK_DIAGNOSTIC_VM_OPTIONS, - CommandLineOptionTest.UNLOCK_EXPERIMENTAL_VM_OPTIONS); + CommandLineOptionTest.UNLOCK_EXPERIMENTAL_VM_OPTIONS, + "-Xbootclasspath/a:.", "-XX:+WhiteBoxAPI"); if (test != null) { for (String method : test.getMethodsToCompileNames()) {