From 1cc7204197eebc321b31c431e360dcf4e8426a51 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Sun, 20 Dec 2020 16:07:57 +0100 Subject: [PATCH] RWLock performance: make unnecessarily volatile fields final GitOrigin-RevId: 23781a6883d170d350627a3b340046a012494c28 --- .../application/impl/ApplicationImpl.java | 5 ++++- .../application/impl/ReadMostlyRWLock.java | 21 ++++--------------- .../application/impl/ApplicationImplTest.java | 4 ++-- 3 files changed, 10 insertions(+), 20 deletions(-) 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 314422a0b8a2..70233b1a1aac 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 @@ -58,6 +58,7 @@ import java.awt.*; import java.util.List; import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; public class ApplicationImpl extends ComponentManagerImpl implements ApplicationEx { @@ -142,6 +143,7 @@ public class ApplicationImpl extends ComponentManagerImpl implements Application gatherStatistics = LOG.isDebugEnabled() || isUnitTestMode() || isInternal(); Activity activity = StartUpMeasurer.startActivity("AppDelayQueue instantiation"); + AtomicReference edtThread = new AtomicReference<>(); Runnable runnable = () -> { // instantiate AppDelayQueue which starts "Periodic task thread" which we'll mark busy to prevent this EDT to die // that thread was chosen because we know for sure it's running @@ -151,9 +153,10 @@ public class ApplicationImpl extends ComponentManagerImpl implements Application Disposer.register(this, () -> { AWTAutoShutdown.getInstance().notifyThreadFree(thread); // allow for EDT to exit - needed for Upsource }); + edtThread.set(Thread.currentThread()); }; EdtInvocationManager.invokeAndWaitIfNeeded(runnable); - myLock = new ReadMostlyRWLock(); + myLock = new ReadMostlyRWLock(edtThread.get()); // Acquire IW lock on EDT indefinitely in legacy mode if (!USE_SEPARATE_WRITE_THREAD || isUnitTestMode) { EdtInvocationManager.invokeAndWaitIfNeeded(() -> acquireWriteIntentLock(getClass())); diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/ReadMostlyRWLock.java b/platform/platform-impl/src/com/intellij/openapi/application/impl/ReadMostlyRWLock.java index be69c8e0c289..90d4449ac3df 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/impl/ReadMostlyRWLock.java +++ b/platform/platform-impl/src/com/intellij/openapi/application/impl/ReadMostlyRWLock.java @@ -23,7 +23,6 @@ import com.intellij.util.containers.ConcurrentList; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.TestOnly; import org.jetbrains.annotations.VisibleForTesting; import java.util.ArrayList; @@ -45,8 +44,7 @@ import java.util.concurrent.locks.LockSupport; * Write lock: sets global {@link #writeRequested} bit and waits for all readers (in global {@link #readers} list) to release their locks by checking {@link Reader#readRequested} for all readers. */ class ReadMostlyRWLock { - volatile Thread writeThread; - private volatile Thread writeIntendedThread; + final Thread writeThread; @VisibleForTesting volatile boolean writeRequested; // this writer is requesting or obtained the write access private final AtomicBoolean writeIntent = new AtomicBoolean(false); @@ -59,7 +57,8 @@ class ReadMostlyRWLock { // (we have to reduce frequency of this "dead readers GC" activity because Thread.isAlive() turned out to be too expensive) private volatile long deadReadersGCStamp; - ReadMostlyRWLock() { + ReadMostlyRWLock(@NotNull Thread writeThread) { + this.writeThread = writeThread; } // Each reader thread has instance of this struct in its thread local. it's also added to global "readers" list. @@ -91,15 +90,6 @@ class ReadMostlyRWLock { return status; }); - @TestOnly - void setWriteThread(@NotNull Thread thread) { - assert !writeAcquired; - assert !writeRequested; - assert writeThread == null; - - writeThread = thread; - } - boolean isWriteThread() { return Thread.currentThread() == writeThread; } @@ -212,13 +202,11 @@ class ReadMostlyRWLock { void writeIntentLock() { //checkWriteThreadAccess(); - writeIntendedThread = Thread.currentThread(); for (int iter=0; ;iter++) { if (writeIntent.compareAndSet(false, true)) { assert !writeRequested; assert !writeAcquired; - writeThread = Thread.currentThread(); break; } @@ -237,9 +225,8 @@ class ReadMostlyRWLock { assert !writeAcquired; assert !writeRequested; - writeThread = null; writeIntent.set(false); - LockSupport.unpark(writeIntendedThread); + LockSupport.unpark(writeThread); } void writeLock() { diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/ApplicationImplTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/ApplicationImplTest.java index f565a221bc66..b2c49d05af1b 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/ApplicationImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/ApplicationImplTest.java @@ -525,8 +525,8 @@ public class ApplicationImplTest extends LightPlatformTestCase { UIUtil.dispatchAllInvocationEvents(); } int readIterations = 200_000_000; - ReadMostlyRWLock lock = new ReadMostlyRWLock(); - lock.setWriteThread(Thread.currentThread()); + ApplicationManager.getApplication().assertIsDispatchThread(); + ReadMostlyRWLock lock = new ReadMostlyRWLock(Thread.currentThread()); final int numOfThreads = JobSchedulerImpl.getJobPoolParallelism(); final Field myThreadLocalsField = Objects.requireNonNull(ReflectionUtil.getDeclaredField(Thread.class, "threadLocals")); //noinspection Convert2Lambda