From fa12df795c880fe82ddc77bc125eb83daf0956ac Mon Sep 17 00:00:00 2001 From: "Maxim.Mossienko" Date: Sun, 14 May 2017 01:34:55 +0200 Subject: [PATCH] (instead of numerous stub index locks) use stub_updating_index's lock for processing stubs, flushing, clearing, disposing as side effect accessing stub indices during processing other stub indices is allowed --- .../com/intellij/index/IndexTest.groovy | 2 +- .../com/intellij/psi/stubs/StubIndexImpl.java | 59 ++++++++-------- .../intellij/psi/stubs/StubUpdatingIndex.java | 67 ++++++------------- .../util/indexing/VfsAwareMapReduceIndex.java | 39 ++++++----- .../util/indexing/impl/MapReduceIndex.java | 53 ++++++++++----- 5 files changed, 107 insertions(+), 113 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/index/IndexTest.groovy b/java/java-tests/testSrc/com/intellij/index/IndexTest.groovy index ceb4192440b0..c615e795ac3a 100644 --- a/java/java-tests/testSrc/com/intellij/index/IndexTest.groovy +++ b/java/java-tests/testSrc/com/intellij/index/IndexTest.groovy @@ -536,7 +536,7 @@ class IndexTest extends JavaCodeInsightFixtureTestCase { } assertTrue(foundClass[0]) - assertTrue(!foundMethod[0]) + assertTrue(foundMethod[0]) // allow access stub index processing other index def foundClassProcessAll = [false] def foundClassStub = [false] diff --git a/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java b/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java index 6d5c9350dd64..c8c9a8fc899e 100644 --- a/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java +++ b/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java @@ -39,10 +39,7 @@ import com.intellij.util.Processors; import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.indexing.*; -import com.intellij.util.indexing.impl.IndexStorage; -import com.intellij.util.indexing.impl.InputDataDiffBuilder; -import com.intellij.util.indexing.impl.MapInputDataDiffBuilder; -import com.intellij.util.indexing.impl.UpdateData; +import com.intellij.util.indexing.impl.*; import com.intellij.util.io.DataExternalizer; import com.intellij.util.io.DataInputOutputUtil; import com.intellij.util.io.KeyDescriptor; @@ -59,7 +56,7 @@ import java.io.IOException; import java.util.*; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantReadWriteLock; @State(name = "FileBasedIndex", storages = @Storage(value = "stubIndex.xml", roamingType = RoamingType.DISABLED)) public class StubIndexImpl extends StubIndex implements PersistentStateComponent, ApplicationComponent { @@ -214,9 +211,7 @@ public class StubIndexImpl extends StubIndex implements PersistentStateComponent if (!myInitialized) { return; } - AsyncState state = getAsyncState(); - for (StubIndexKey key : getAllStubIndexKeys()) { - final MyIndex index = state.myIndices.get(key); + for (MyIndex index : getAsyncState().myIndices.values()) { index.flush(); } } @@ -344,27 +339,29 @@ public class StubIndexImpl extends StubIndex implements PersistentStateComponent @Nullable final GlobalSearchScope scope, @NotNull StubIdListContainerAction action) { final FileBasedIndexImpl fileBasedIndex = (FileBasedIndexImpl)FileBasedIndex.getInstance(); - myAccessValidator.checkAccessingIndexDuringOtherIndexProcessing(StubUpdatingIndex.INDEX_ID); - fileBasedIndex.ensureUpToDate(StubUpdatingIndex.INDEX_ID, project, scope); + ID stubUpdatingIndexId = StubUpdatingIndex.INDEX_ID; + myAccessValidator.checkAccessingIndexDuringOtherIndexProcessing(stubUpdatingIndexId); + fileBasedIndex.ensureUpToDate(stubUpdatingIndexId, project, scope); final MyIndex index = (MyIndex)getAsyncState().myIndices.get(indexKey); + UpdatableIndex stubUpdatingIndex = fileBasedIndex.getIndex(stubUpdatingIndexId); try { - myAccessValidator.checkAccessingIndexDuringOtherIndexProcessing(indexKey); + myAccessValidator.checkAccessingIndexDuringOtherIndexProcessing(stubUpdatingIndexId); try { // disable up-to-date check to avoid locks on attempt to acquire index write lock while holding at the same time the readLock for this index FileBasedIndexImpl.disableUpToDateCheckForCurrentThread(); - index.getReadLock().lock(); + stubUpdatingIndex.getReadLock().lock(); - myAccessValidator.startedProcessingActivityForIndex(indexKey); + myAccessValidator.startedProcessingActivityForIndex(stubUpdatingIndexId); return index.getData(key).forEach(action); } finally { - myAccessValidator.stoppedProcessingActivityForIndex(indexKey); - index.getReadLock().unlock(); + myAccessValidator.stoppedProcessingActivityForIndex(stubUpdatingIndexId); + stubUpdatingIndex.getReadLock().unlock(); FileBasedIndexImpl.enableUpToDateCheckForCurrentThread(); } } @@ -505,19 +502,21 @@ public class StubIndexImpl extends StubIndex implements PersistentStateComponent } public void cleanupMemoryStorage() { - for (UpdatableIndex index : getAsyncState().myIndices.values()) { - final IndexStorage indexStorage = ((VfsAwareMapReduceIndex)index).getStorage(); - index.getWriteLock().lock(); - try { + UpdatableIndex stubUpdatingIndex = + ((FileBasedIndexImpl)FileBasedIndex.getInstance()).getIndex(StubUpdatingIndex.INDEX_ID); + stubUpdatingIndex.getWriteLock().lock(); + + try { + for (UpdatableIndex index : getAsyncState().myIndices.values()) { + final IndexStorage indexStorage = ((VfsAwareMapReduceIndex)index).getStorage(); ((MemoryIndexStorage)indexStorage).clearMemoryMap(); } - finally { - index.getWriteLock().unlock(); - } + } + finally { + stubUpdatingIndex.getWriteLock().unlock(); } } - public void clearAllIndices() { for (UpdatableIndex index : getAsyncState().myIndices.values()) { try { @@ -561,14 +560,6 @@ public class StubIndexImpl extends StubIndex implements PersistentStateComponent myPreviouslyRegistered = state; } - public final Lock getWriteLock(StubIndexKey indexKey) { - return getAsyncState().myIndices.get(indexKey).getWriteLock(); - } - - Collection getAllStubIndexKeys() { - return Collections.unmodifiableCollection(getAsyncState().myIndices.keySet()); - } - public void updateIndex(@NotNull StubIndexKey key, int fileId, @NotNull final Map oldValues, @@ -586,6 +577,12 @@ public class StubIndexImpl extends StubIndex implements PersistentStateComponent } private static class MyIndex extends VfsAwareMapReduceIndex { + @NotNull + @Override + protected ReentrantReadWriteLock createLock() { + UpdatableIndex index = ((FileBasedIndexImpl)FileBasedIndex.getInstance()).getIndex(StubUpdatingIndex.INDEX_ID); + return ((MapReduceIndex)index).getLock(); + } public MyIndex(IndexExtension extension, IndexStorage storage) throws IOException { super(extension, storage); diff --git a/platform/lang-impl/src/com/intellij/psi/stubs/StubUpdatingIndex.java b/platform/lang-impl/src/com/intellij/psi/stubs/StubUpdatingIndex.java index 6277537d5aee..1cbc95dc21ba 100644 --- a/platform/lang-impl/src/com/intellij/psi/stubs/StubUpdatingIndex.java +++ b/platform/lang-impl/src/com/intellij/psi/stubs/StubUpdatingIndex.java @@ -441,13 +441,13 @@ public class StubUpdatingIndex extends CustomImplementationFileBasedIndexExtensi } @Override - public void flush() throws StorageException { + protected void doFlush() throws IOException, StorageException { final StubIndexImpl stubIndex = getStubIndex(); try { stubIndex.flush(); } finally { - super.flush(); + super.doFlush(); } } @@ -458,36 +458,22 @@ public class StubUpdatingIndex extends CustomImplementationFileBasedIndexExtensi StubUpdatingData stubUpdatingData = (StubUpdatingData)updateData; final Map> newStubIndicesValueMap = stubUpdatingData.getNewStubIndicesValueMap(); - final StubIndexImpl stubIndex = getStubIndex(); - final Collection allStubIndices = stubIndex.getAllStubIndexKeys(); try { - // first write-lock affected stub indices to avoid deadlocks - for (StubIndexKey key : allStubIndices) { - stubIndex.getWriteLock(key).lock(); - } + getWriteLock().lock(); - try { - getWriteLock().lock(); + super.updateWithMap(inputId, updateData); - super.updateWithMap(inputId, updateData); + final Map> previousStubIndicesValueMap = stubUpdatingData.getOldStubIndicesValueMap(); - final Map> previousStubIndicesValueMap = stubUpdatingData.getOldStubIndicesValueMap(); - - updateStubIndices( - getAffectedIndices(previousStubIndicesValueMap, newStubIndicesValueMap), - inputId, - previousStubIndicesValueMap, - newStubIndicesValueMap - ); - } - finally { - getWriteLock().unlock(); - } + updateStubIndices( + getAffectedIndices(previousStubIndicesValueMap, newStubIndicesValueMap), + inputId, + previousStubIndicesValueMap, + newStubIndicesValueMap + ); } finally { - for (StubIndexKey key : allStubIndices) { - stubIndex.getWriteLock(key).unlock(); - } + getWriteLock().unlock(); } } @@ -509,34 +495,19 @@ public class StubUpdatingIndex extends CustomImplementationFileBasedIndexExtensi } @Override - public void clear() throws StorageException { + protected void doClear() throws StorageException, IOException { final StubIndexImpl stubIndex = StubIndexImpl.getInstanceOrInvalidate(); - final Collection allStubIndexKeys = stubIndex != null? stubIndex.getAllStubIndexKeys() : Collections.emptyList(); - try { - for (StubIndexKey key : allStubIndexKeys) { - //noinspection ConstantConditions - stubIndex.getWriteLock(key).lock(); - } - getWriteLock().lock(); - if (stubIndex != null) { - stubIndex.clearAllIndices(); - } - myStubVersionMap.clear(); - super.clear(); - } - finally { - getWriteLock().unlock(); - for (StubIndexKey key : allStubIndexKeys) { - //noinspection ConstantConditions - stubIndex.getWriteLock(key).unlock(); - } + if (stubIndex != null) { + stubIndex.clearAllIndices(); } + myStubVersionMap.clear(); + super.doClear(); } @Override - public void dispose() { + protected void doDispose() throws StorageException { try { - super.dispose(); + super.doDispose(); } finally { getStubIndex().dispose(); diff --git a/platform/lang-impl/src/com/intellij/util/indexing/VfsAwareMapReduceIndex.java b/platform/lang-impl/src/com/intellij/util/indexing/VfsAwareMapReduceIndex.java index b303d7e26c0b..e61d74273b27 100644 --- a/platform/lang-impl/src/com/intellij/util/indexing/VfsAwareMapReduceIndex.java +++ b/platform/lang-impl/src/com/intellij/util/indexing/VfsAwareMapReduceIndex.java @@ -57,7 +57,7 @@ public class VfsAwareMapReduceIndex extends MapReduceIndex> myInMemoryKeys = new TIntObjectHashMap>(); + private final TIntObjectHashMap> myInMemoryKeys = new TIntObjectHashMap<>(); private final SnapshotInputMappings mySnapshotInputMappings; public VfsAwareMapReduceIndex(@NotNull IndexExtension extension, @@ -179,30 +179,35 @@ public class VfsAwareMapReduceIndex extends MapReduceIndex implements InvertedIndex< protected final ForwardIndex myForwardIndex; - private final ReentrantReadWriteLock myLock = new ReentrantReadWriteLock(); + private final ReentrantReadWriteLock myLock = createLock(); private volatile boolean myDisposed; private final LowMemoryWatcher myLowMemoryFlusher = LowMemoryWatcher.register(new Runnable() { @@ -91,12 +91,20 @@ public abstract class MapReduceIndex implements InvertedIndex< return myStorage; } + @NotNull + protected ReentrantReadWriteLock createLock() { + return new ReentrantReadWriteLock(); + } + + public final ReentrantReadWriteLock getLock() { + return myLock; + } + @Override public void clear() throws StorageException { try { getWriteLock().lock(); - myStorage.clear(); - if (myForwardIndex != null) myForwardIndex.clear(); + doClear(); } catch (StorageException e) { LOG.error(e); @@ -109,12 +117,16 @@ public abstract class MapReduceIndex implements InvertedIndex< } } + protected void doClear() throws StorageException, IOException { + myStorage.clear(); + if (myForwardIndex != null) myForwardIndex.clear(); + } + @Override public void flush() throws StorageException{ try { getReadLock().lock(); - if (myForwardIndex != null) myForwardIndex.flush(); - myStorage.flush(); + doFlush(); } catch (IOException e) { throw new StorageException(e); @@ -133,23 +145,18 @@ public abstract class MapReduceIndex implements InvertedIndex< } } + protected void doFlush() throws IOException, StorageException { + if (myForwardIndex != null) myForwardIndex.flush(); + myStorage.flush(); + } + @Override public void dispose() { myLowMemoryFlusher.stop(); final Lock lock = getWriteLock(); try { lock.lock(); - try { - myStorage.close(); - } - finally { - try { - if (myForwardIndex != null) myForwardIndex.close(); - } - catch (IOException e) { - LOG.error(e); - } - } + doDispose(); } catch (StorageException e) { LOG.error(e); @@ -160,6 +167,20 @@ public abstract class MapReduceIndex implements InvertedIndex< } } + protected void doDispose() throws StorageException { + try { + myStorage.close(); + } + finally { + try { + if (myForwardIndex != null) myForwardIndex.close(); + } + catch (IOException e) { + LOG.error(e); + } + } + } + @NotNull public final Lock getReadLock() { return myLock.readLock();