diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java index b8bfe9ff8765..411d22db8932 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java @@ -67,6 +67,7 @@ import com.intellij.util.io.storage.HeavyProcessLatch; import com.intellij.util.ui.UIUtil; import gnu.trove.TLongArrayList; import gnu.trove.TLongProcedure; +import jsr166e.StampedLock; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -78,22 +79,18 @@ import java.awt.*; import java.io.File; import java.io.IOException; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.locks.ReentrantReadWriteLock; public class ApplicationImpl extends PlatformComponentManagerImpl implements ApplicationEx { private static final Logger LOG = Logger.getInstance("#com.intellij.application.impl.ApplicationImpl"); private final ModalityState MODALITY_STATE_NONE = ModalityState.NON_MODAL; - // about writer preference: the way the j.u.c.l.ReentrantReadWriteLock.NonfairSync is implemented, the - // writer thread will be always at the queue head and therefore, j.u.c.l.ReentrantReadWriteLock.NonfairSync.readerShouldBlock() - // will return true if the write action is pending, exactly as we need - private final ReentrantReadWriteLock myLock = new ReentrantReadWriteLock(false); + private final StampedLock myLock = new StampedLock(); private final ModalityInvokator myInvokator = new ModalityInvokatorImpl(); @@ -126,12 +123,13 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App private static final String WAS_EVER_SHOWN = "was.ever.shown"; private static final int IS_EDT_FLAG = 1<<30; // we don't mess with sign bit since we want to do arithmetic - private static final int IS_READ_LOCK_ACQUIRED_FLAG = 1<<29; private static class Status { // higher three bits are for IS_* flags // lower bits are for edtSafe counter private int flags; + // StampedLock' acquired sequence or zero if not locked + private long stamp; } private static final ThreadLocal status = new ThreadLocal(){ @@ -145,9 +143,6 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App private static Status getStatus() { return status.get(); } - private static void setReadLockAcquired(Status status, boolean acquired) { - status.flags = BitUtil.set(status.flags, IS_READ_LOCK_ACQUIRED_FLAG, acquired); - } private static final ModalityState ANY = new ModalityState() { @Override @@ -319,7 +314,7 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App } private static boolean holdsReadLock(Status status) { - return BitUtil.isSet(status.flags, IS_READ_LOCK_ACQUIRED_FLAG); + return status.stamp != 0; } @NotNull @@ -981,8 +976,7 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App private void startRead(Status status) { assertNoPsiLock(); try { - myLock.readLock().lockInterruptibly(); - setReadLockAcquired(status, true); + status.stamp = myLock.readLockInterruptibly(); } catch (InterruptedException e) { throw new RuntimeInterruptedException(e); @@ -990,8 +984,8 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App } private void endRead(Status status) { - setReadLockAcquired(status, false); - myLock.readLock().unlock(); + myLock.unlockRead(status.stamp); + status.stamp = 0; } @Override @@ -1074,7 +1068,7 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App } private static boolean isReadAccessAllowed(Status status) { - return (status.flags & (IS_EDT_FLAG | IS_READ_LOCK_ACQUIRED_FLAG)) != 0; + return BitUtil.isSet(status.flags, IS_EDT_FLAG) || status.stamp != 0; } @Override @@ -1155,14 +1149,7 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App if (mustAcquire) { assertNoPsiLock(); - try { - // timed version of tryLock() respects fairness unlike the no-args method - if (!myLock.readLock().tryLock(0, TimeUnit.MILLISECONDS)) return false; - setReadLockAcquired(status, true); - } - catch (InterruptedException e) { - throw new RuntimeInterruptedException(e); - } + if ((status.stamp = myLock.tryReadLock()) == 0) return false; } try { @@ -1209,7 +1196,8 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App private final TLongArrayList writePauses = new TLongArrayList(); private void startWrite(@Nullable Class clazz) { - assertIsDispatchThread(getStatus(), "Write access is allowed from event dispatch thread only"); + Status status = getStatus(); + assertIsDispatchThread(status, "Write access is allowed from event dispatch thread only"); boolean writeActionPending = myWriteActionPending; myWriteActionPending = true; long start = System.currentTimeMillis(); @@ -1218,26 +1206,26 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App fireBeforeWriteActionStart(clazz); try { - if (!isWriteAccessAllowed()) { + if (!myLock.isWriteLocked()) { assertNoPsiLock(); - } - if (!myLock.writeLock().tryLock()) { - final AtomicBoolean lockAcquired = new AtomicBoolean(false); - if (ourDumpThreadsOnLongWriteActionWaiting > 0) { - executeOnPooledThread(new Runnable() { - @Override - public void run() { - while (!lockAcquired.get()) { - TimeoutUtil.sleep(ourDumpThreadsOnLongWriteActionWaiting); - if (!lockAcquired.get()) { - PerformanceWatcher.getInstance().dumpThreads("waiting", true); + if ((status.stamp = myLock.tryWriteLock()) == 0) { + final AtomicBoolean lockAcquired = new AtomicBoolean(false); + if (ourDumpThreadsOnLongWriteActionWaiting > 0) { + executeOnPooledThread(new Runnable() { + @Override + public void run() { + while (!lockAcquired.get()) { + TimeoutUtil.sleep(ourDumpThreadsOnLongWriteActionWaiting); + if (!lockAcquired.get()) { + PerformanceWatcher.getInstance().dumpThreads("waiting", true); + } } } - } - }); + }); + } + status.stamp = myLock.writeLockInterruptibly(); + lockAcquired.set(true); } - myLock.writeLock().lockInterruptibly(); - lockAcquired.set(true); } } catch (InterruptedException e) { @@ -1258,11 +1246,15 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App private void endWrite(@Nullable Class clazz) { try { - myWriteActionsStack.pop(); fireWriteActionFinished(clazz); } finally { - myLock.writeLock().unlock(); + myWriteActionsStack.pop(); + if (myWriteActionsStack.isEmpty()) { + Status status = getStatus(); + myLock.unlockWrite(status.stamp); + status.stamp = 0; + } } } @@ -1363,7 +1355,7 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App @Override public boolean isWriteAccessAllowed() { - return myLock.isWriteLockedByCurrentThread(); + return isDispatchThread() && myLock.isWriteLocked(); } // cheaper version of isWriteAccessAllowed(). must be called from EDT @@ -1494,4 +1486,16 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App (isHeadlessEnvironment() ? " (Headless)" : "") + (isCommandLine() ? " (Command line)" : ""); } + + @TestOnly + public void disableEventsUntil(@NotNull Disposable disposable) { + final List listeners = new ArrayList(myDispatcher.getListeners()); + myDispatcher.getListeners().removeAll(listeners); + Disposer.register(disposable, new Disposable() { + @Override + public void dispose() { + myDispatcher.getListeners().addAll(listeners); + } + }); + } } diff --git a/platform/platform-tests/testSrc/com/intellij/application/ApplicationImplTest.java b/platform/platform-tests/testSrc/com/intellij/application/ApplicationImplTest.java index 6ba1cceca1d4..38114c10f651 100644 --- a/platform/platform-tests/testSrc/com/intellij/application/ApplicationImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/application/ApplicationImplTest.java @@ -15,20 +15,22 @@ */ package com.intellij.application; +import com.intellij.openapi.Disposable; import com.intellij.openapi.application.AccessToken; import com.intellij.openapi.application.Application; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ex.ApplicationEx; +import com.intellij.openapi.application.impl.ApplicationImpl; import com.intellij.openapi.progress.ProgressIndicator; import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.progress.Task; import com.intellij.openapi.progress.impl.ProgressManagerImpl; +import com.intellij.openapi.util.Disposer; import com.intellij.openapi.util.EmptyRunnable; import com.intellij.openapi.util.ThrowableComputable; import com.intellij.testFramework.PlatformTestCase; import com.intellij.testFramework.PlatformTestUtil; import com.intellij.testFramework.Timings; -import com.intellij.util.ThrowableRunnable; import com.intellij.util.TimeoutUtil; import com.intellij.util.ui.UIUtil; import org.jetbrains.annotations.NotNull; @@ -58,50 +60,38 @@ public class ApplicationImplTest extends PlatformTestCase { String err = null; for (int i=0; i<4; i++) { - Callable runnable = new Callable() { - @Override - public String call() throws Exception { - try { - assertFalse(application.isReadAccessAllowed()); - long l2 = PlatformTestUtil.measure(new Runnable() { - @Override - public void run() { - for (int i = 0; i < N; i++) { - AccessToken token = application.acquireReadActionLock(); - try { - // do it - } - finally { - token.finish(); - } - } + Callable runnable = () -> { + try { + assertFalse(application.isReadAccessAllowed()); + long l2 = PlatformTestUtil.measure(() -> { + for (int i1 = 0; i1 < N; i1++) { + AccessToken token = application.acquireReadActionLock(); + try { + // do it } - }); - - long l1 = PlatformTestUtil.measure(new Runnable() { - @Override - public void run() { - for (int i=0; i 20) { - return "Suspiciously different times for acquireReadActionLock(" +l2 + "ms) vs runReadAction(" + l1 + "ms). Ratio: "+ ratioPercent + "%"; } + }); + + long l1 = PlatformTestUtil.measure(() -> { + for (int i1 = 0; i1 < N; i1++) { + application.runReadAction(() -> { + }); + } + }); + + assertFalse(application.isReadAccessAllowed()); + int ratioPercent = (int)((l1 - l2) * 100.0 / l1); + if (Math.abs(ratioPercent) > 20) { + return "Suspiciously different times for acquireReadActionLock(" +l2 + "ms) vs runReadAction(" + l1 + "ms). Ratio: "+ ratioPercent + "%"; } - catch (Throwable e) { - exception = e; - } - return null; } + catch (Throwable e) { + exception = e; + } + return null; }; err = application.executeOnPooledThread(runnable).get(); @@ -114,49 +104,61 @@ public class ApplicationImplTest extends PlatformTestCase { } - public void testLockPerformance() throws InterruptedException { - int iterations = Timings.adjustAccordingToMySpeed(3000, true); + public void testReadWriteLockPerformance() throws InterruptedException { + int iterations = Timings.adjustAccordingToMySpeed(300000, true); System.out.println("iterations = " + iterations); final int readIterations = iterations; final int writeIterations = iterations; - final int numOfThreads = 10; - PlatformTestUtil.startPerformanceTest("lock performance", 200, new ThrowableRunnable() { - @Override - public void run() throws Throwable { + runReadWrites(readIterations, writeIterations, 2000); + } + + public void testReadLockPerformance() throws InterruptedException { + int iterations = Timings.adjustAccordingToMySpeed(300000, true); + System.out.println("iterations = " + iterations); + final int readIterations = iterations; + final int writeIterations = 0; + + runReadWrites(readIterations, writeIterations, 1000); + } + + private static void runReadWrites(final int readIterations, final int writeIterations, int expectedMs) { + final ApplicationImpl application = (ApplicationImpl)ApplicationManager.getApplication(); + Disposable disposable = Disposer.newDisposable(); + application.disableEventsUntil(disposable); + + try { + final int numOfThreads = 10; + PlatformTestUtil.startPerformanceTest("lock performance", expectedMs, () -> { final CountDownLatch reads = new CountDownLatch(numOfThreads); for (int i = 0; i < numOfThreads; i++) { final String name = "stress thread " + i; - new Thread(new Runnable() { - @Override - public void run() { - System.out.println(name); - for (int i = 0; i < readIterations; i++) { - ApplicationManager.getApplication().runReadAction(new Runnable() { - @Override - public void run() { + new Thread(() -> { + System.out.println(name); + for (int i1 = 0; i1 < readIterations; i1++) { + application.runReadAction(() -> { - } - }); - } - - reads.countDown(); + }); } + + reads.countDown(); }, name).start(); } - System.out.println("write start"); - for (int i = 0; i < writeIterations; i++) { - ApplicationManager.getApplication().runWriteAction(new Runnable() { - @Override - public void run() { - } - }); + if (writeIterations > 0) { + System.out.println("write start"); + for (int i = 0; i < writeIterations; i++) { + ApplicationManager.getApplication().runWriteAction(() -> { + }); + } + System.out.println("write end"); } - System.out.println("write end"); reads.await(); - } - }).assertTiming(); + }).assertTiming(); + } + finally { + Disposer.dispose(disposable); + } } private volatile boolean tryingToStartWriteAction; @@ -170,62 +172,50 @@ public class ApplicationImplTest extends PlatformTestCase { anotherThreadStarted[i] = new AtomicBoolean(); } final StringBuffer LOG = new StringBuffer(); - new Thread(new Runnable() { - @Override - public void run() { - try { - ApplicationManager.getApplication().runReadAction(new Runnable() { - @Override - public void run() { - LOG.append("inside read action\n"); - readStarted = true; - while (!tryingToStartWriteAction); - TimeoutUtil.sleep(100); + new Thread(() -> { + try { + ApplicationManager.getApplication().runReadAction(() -> { + LOG.append("inside read action\n"); + readStarted = true; + while (!tryingToStartWriteAction); + TimeoutUtil.sleep(100); - for (int i = 0; i < anotherReadActionStarted.length; i++) { - final int finalI = i; - new Thread(new Runnable() { - @Override - public void run() { - LOG.append("\nanother thread started " + finalI); - anotherThreadStarted[finalI].set(true); - ApplicationManager.getApplication().runReadAction(new Runnable() { - @Override - public void run() { - LOG.append("\ninside another thread read action " + finalI); - anotherReadActionStarted[finalI].set(true); - try { - Thread.sleep(100); - } - catch (InterruptedException e) { - throw new RuntimeException(e); - } - anotherReadActionStarted[finalI].set(false); - LOG.append("\nfinished another thread read action " + finalI); - } - }); - LOG.append("\nanother thread finished " + finalI); - } - },"another read action "+i).start(); - } - - for (AtomicBoolean threadStarted : anotherThreadStarted) { - while (!threadStarted.get()) ; - } - // now the other threads try to get read lock. we should not let them - for (int i=0; i<10; i++) { - for (AtomicBoolean readStarted : anotherReadActionStarted) { - assertThat(!readStarted.get(), "must not start another read action while write is pending"); + for (int i = 0; i < anotherReadActionStarted.length; i++) { + final int finalI = i; + new Thread(() -> { + LOG.append("\nanother thread started " + finalI); + anotherThreadStarted[finalI].set(true); + ApplicationManager.getApplication().runReadAction(() -> { + LOG.append("\ninside another thread read action " + finalI); + anotherReadActionStarted[finalI].set(true); + try { + Thread.sleep(100); } - TimeoutUtil.sleep(20); - } - LOG.append("\nfinished read action"); + catch (InterruptedException e) { + throw new RuntimeException(e); + } + anotherReadActionStarted[finalI].set(false); + LOG.append("\nfinished another thread read action " + finalI); + }); + LOG.append("\nanother thread finished " + finalI); + }, "another read action " + i).start(); + } + + for (AtomicBoolean threadStarted : anotherThreadStarted) { + while (!threadStarted.get()) ; + } + // now the other threads try to get read lock. we should not let them + for (int i=0; i<10; i++) { + for (AtomicBoolean readStarted1 : anotherReadActionStarted) { + assertThat(!readStarted1.get(), "must not start another read action while write is pending"); } - }); - } - catch (Throwable e) { - exception = e; - } + TimeoutUtil.sleep(20); + } + LOG.append("\nfinished read action"); + }); + } + catch (Throwable e) { + exception = e; } }, "read").start(); @@ -233,15 +223,12 @@ public class ApplicationImplTest extends PlatformTestCase { while (!readStarted); tryingToStartWriteAction = true; LOG.append("\nwrite about to start"); - ApplicationManager.getApplication().runWriteAction(new Runnable() { - @Override - public void run() { - LOG.append("\ninside write action"); - for (AtomicBoolean readStarted : anotherReadActionStarted) { - assertThat(!readStarted.get(), "must not start another read action while write is running"); - } - LOG.append("\nfinished write action"); + ApplicationManager.getApplication().runWriteAction(() -> { + LOG.append("\ninside write action"); + for (AtomicBoolean readStarted1 : anotherReadActionStarted) { + assertThat(!readStarted1.get(), "must not start another read action while write is running"); } + LOG.append("\nfinished write action"); }); if (exception != null) { System.err.println(LOG); @@ -257,37 +244,23 @@ public class ApplicationImplTest extends PlatformTestCase { } public void testProgressVsReadAction() throws Throwable { - ProgressManager.getInstance().runProcessWithProgressSynchronously(new ThrowableComputable() { - @Override - public Void compute() throws Exception { - try { - assertFalse(ApplicationManager.getApplication().isReadAccessAllowed()); - assertFalse(ApplicationManager.getApplication().isDispatchThread()); - for (int i=0; i<100;i++) { - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - ApplicationManager.getApplication().runWriteAction(new Runnable() { - @Override - public void run() { - TimeoutUtil.sleep(20); - } - }); - } - }); - ApplicationManager.getApplication().runReadAction(new Runnable() { - @Override - public void run() { - TimeoutUtil.sleep(20); - } - }); - } + ProgressManager.getInstance().runProcessWithProgressSynchronously((ThrowableComputable)() -> { + try { + assertFalse(ApplicationManager.getApplication().isReadAccessAllowed()); + assertFalse(ApplicationManager.getApplication().isDispatchThread()); + for (int i=0; i<100;i++) { + SwingUtilities.invokeLater(() -> ApplicationManager.getApplication().runWriteAction(() -> { + TimeoutUtil.sleep(20); + })); + ApplicationManager.getApplication().runReadAction(() -> { + TimeoutUtil.sleep(20); + }); } - catch (Exception e) { - exception = e; - } - return null; } + catch (Exception e) { + exception = e; + } + return null; }, "cc", false, getProject()); if (exception != null) throw exception; } @@ -329,16 +302,13 @@ public class ApplicationImplTest extends PlatformTestCase { public void testRunProcessWithProgressSynchronouslyInReadAction() throws Throwable { boolean result = ((ApplicationEx)ApplicationManager.getApplication()) - .runProcessWithProgressSynchronouslyInReadAction(getProject(), "title", true, "cancel", null, new Runnable() { - @Override - public void run() { - try { - assertFalse(SwingUtilities.isEventDispatchThread()); - assertTrue(ApplicationManager.getApplication().isReadAccessAllowed()); - } - catch (Throwable e) { - exception = e; - } + .runProcessWithProgressSynchronouslyInReadAction(getProject(), "title", true, "cancel", null, () -> { + try { + assertFalse(SwingUtilities.isEventDispatchThread()); + assertTrue(ApplicationManager.getApplication().isReadAccessAllowed()); + } + catch (Throwable e) { + exception = e; } }); assertTrue(result); @@ -346,19 +316,9 @@ public class ApplicationImplTest extends PlatformTestCase { } public void testRunProcessWithProgressSynchronouslyInReadActionWithPendingWriteAction() throws Throwable { - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - ApplicationManager.getApplication().runWriteAction(EmptyRunnable.getInstance()); - } - }); + SwingUtilities.invokeLater(() -> ApplicationManager.getApplication().runWriteAction(EmptyRunnable.getInstance())); boolean result = ((ApplicationEx)ApplicationManager.getApplication()) - .runProcessWithProgressSynchronouslyInReadAction(getProject(), "title", true, "cancel", null, new Runnable() { - @Override - public void run() { - TimeoutUtil.sleep(10000); - } - }); + .runProcessWithProgressSynchronouslyInReadAction(getProject(), "title", true, "cancel", null, () -> TimeoutUtil.sleep(10000)); assertTrue(result); UIUtil.dispatchAllInvocationEvents(); if (exception != null) throw exception;