From 927d11c39181d6f2e0b817c59de6905f93bd09fd Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Wed, 8 Jun 2016 11:51:18 +0300 Subject: [PATCH] rewritten to explicit classesProcessed sets instead of global class user data, extracted LazyConcurrentCollection into separate class --- .../search/JavaClassInheritorsSearcher.java | 197 ++------------- .../impl/search/LazyConcurrentCollection.java | 231 ++++++++++++++++++ 2 files changed, 250 insertions(+), 178 deletions(-) create mode 100644 java/java-indexing-impl/src/com/intellij/psi/impl/search/LazyConcurrentCollection.java diff --git a/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaClassInheritorsSearcher.java b/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaClassInheritorsSearcher.java index 45bce6b792c0..63c7cfee74e4 100644 --- a/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaClassInheritorsSearcher.java +++ b/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaClassInheritorsSearcher.java @@ -21,7 +21,7 @@ import com.intellij.openapi.progress.ProgressIndicator; import com.intellij.openapi.progress.ProgressIndicatorProvider; import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.*; +import com.intellij.openapi.util.Computable; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; @@ -33,14 +33,12 @@ import com.intellij.psi.search.searches.ClassInheritorsSearch; import com.intellij.psi.search.searches.DirectClassInheritorsSearch; import com.intellij.psi.util.PsiUtilCore; import com.intellij.util.ConcurrencyUtil; +import com.intellij.util.Function; import com.intellij.util.Processor; -import com.intellij.util.concurrency.Semaphore; -import com.intellij.util.containers.HashSetQueue; +import com.intellij.util.containers.Predicate; import org.jetbrains.annotations.NotNull; -import java.util.Iterator; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.ForkJoinPool; public class JavaClassInheritorsSearcher extends QueryExecutorBase { @Override @@ -106,7 +104,22 @@ public class JavaClassInheritorsSearcher extends QueryExecutorBase cached = map.get(baseClass); if (cached == null) { // returns lazy collection of subclasses. Each call to next() leads to calculation of next batch of subclasses. - Iterable computed = new AllSubClassesLazyCollection(project, baseClass); // it's empty now, no big deal + Function converter = + anchor -> ApplicationManager.getApplication().runReadAction((Computable)() -> (PsiClass)anchor.retrieve()); + Predicate applicableFilter = + candidate -> !(candidate instanceof PsiAnonymousClass) && candidate != null && !candidate.hasModifierProperty(PsiModifier.FINAL); + LazyConcurrentCollection.MoreElementsGenerator generator = (candidate, processor) -> { + DirectClassInheritorsSearch.search(candidate, GlobalSearchScope.allScope(project)).forEach(subClass -> { + ProgressManager.checkCanceled(); + PsiAnchor pointer = PsiAnchor.create(subClass); + // append found result to subClasses as early as possible to allow other waiting threads to continue + processor.consume(pointer); + return true; + }); + }; + PsiAnchor seed = ApplicationManager.getApplication().runReadAction((Computable)() -> PsiAnchor.create(baseClass)); + // lazy collection: store underlying queue as PsiAnchors, generate new elements by running direct inheritors + Iterable computed = new LazyConcurrentCollection<>(seed, converter, applicableFilter, generator); // for non-physical elements ignore the cache completely because non-physical elements created so often/unpredictably so I can't figure out when to clear caches in this case boolean isPhysical = ApplicationManager.getApplication().runReadAction((Computable)baseClass::isPhysical); // make sure concurrent calls of this method always return the same collection to avoid expensive duplicate work @@ -183,176 +196,4 @@ public class JavaClassInheritorsSearcher extends QueryExecutorBase)() -> baseClass.hasModifierProperty(PsiModifier.FINAL)); } - - private static class AllSubClassesLazyCollection implements Iterable { - // Computes all sub classes of the 'baseClass' transitively by calling DirectClassInheritorsSearch repeatedly. - // Already computed subclasses are stored in this collection. - // There are two iterators maintained for this collection: - // - 'candidatesToFindSubclassesIterator' points to the next element for which direct inheritors haven't been searched yet. - // - 'subClassIterator' created in AllSubClassesLazyCollection.iterator() maintains state of the AllSubClassesLazyCollection iterator in a lazy fashion. - // If more elements requested for this iterator, the processMoreSubclasses() is called which tries to populate 'subClasses' with more inheritors. - private final HashSetQueue subClasses = new HashSetQueue<>(); // guarded by lock - private final Object lock = new Object(); // MUST NOT acquire read action inside this lock - private final GlobalSearchScope projectScope; - private final Semaphore currentlyProcessingClasses = new Semaphore(); - - private final HashSetQueue.PositionalIterator candidatesToFindSubclassesIterator = subClasses.iterator(); // guarded by lock - - AllSubClassesLazyCollection(@NotNull Project project, @NotNull PsiClass baseClass) { - projectScope = GlobalSearchScope.allScope(project); - subClasses.add(ApplicationManager.getApplication().runReadAction((Computable)() -> PsiAnchor.create(baseClass))); - } - - @NotNull - @Override - public Iterator iterator() { - return new Iterator() { - private final Iterator subClassIterator = subClasses.iterator(); // guarded by lock - { - synchronized (lock) { - subClassIterator.next(); //skip the baseClass which stored in the subClasses first element - } - } - @Override - public boolean hasNext() { - synchronized (lock) { - if (subClassIterator.hasNext()) return true; - } - - processMoreSubclasses(subClassIterator); - - synchronized (lock) { - return subClassIterator.hasNext(); - } - } - - @Override - public PsiClass next() { - PsiAnchor next; - synchronized (lock) { - next = subClassIterator.next(); - } - return ApplicationManager.getApplication().runReadAction((Computable)() -> (PsiClass)next.retrieve()); - } - }; - } - - private PsiClass findNextClassInQueue(@NotNull HashSetQueue.PositionalIterator.IteratorPosition position) { - // find the first class which is fit (not anonymous and not final and retrievable from PsiAnchor) and not already processed (flag PROCESSING_SUBCLASSES_STATUS in class user data) - PsiClass candidate = null; - boolean foundClassBeingProcessed = false; - // couldn't call iterator.next() until class is processed, so use position.peek()/position.next() which don't advance iterator - while (position != null) { - ProgressManager.checkCanceled(); - PsiAnchor anchor = position.peek(); - candidate = (PsiClass)anchor.retrieve(); - if (candidate instanceof PsiAnonymousClass || candidate != null && candidate.hasModifierProperty(PsiModifier.FINAL)) { - candidate = null; - } - - if (candidate != null) { - ClassProcessingStatus status = candidate.getUserData(PROCESSING_SUBCLASSES_STATUS); - if (status == null) { - candidate.putUserData(PROCESSING_SUBCLASSES_STATUS, ClassProcessingStatus.PROCESSING_SUBCLASSES); - break; - } - foundClassBeingProcessed |= status == ClassProcessingStatus.PROCESSING_SUBCLASSES; - } - if (!foundClassBeingProcessed) { - candidatesToFindSubclassesIterator.next(); // this class and all previous are either unfit (anonymous or final or un-retrievable) or already processed, skip iterator to help other threads - if (candidate != null) { - candidate.putUserData(PROCESSING_SUBCLASSES_STATUS, null); // this flag isn't needed anymore, free some memory - } - } - // the candidate is already being processed in the other thread, try the next one (not advancing iterator!) - candidate = null; - position = position.next(); - } - return candidate; - } - - enum ClassProcessingStatus { - PROCESSING_SUBCLASSES, PROCESSING_FINISHED - } - private static final Key PROCESSING_SUBCLASSES_STATUS = Key.create("PROCESSING_SUBCLASSES_STATUS"); - - // polls 'subClasses' for more sub classes and call DirectClassInheritorsSearch for them - private void processMoreSubclasses(@NotNull Iterator subClassIterator) { - while (true) { - ProgressManager.checkCanceled(); - - PsiClass candidate = ApplicationManager.getApplication().runReadAction(new Computable() { - @Override - public PsiClass compute() { - synchronized (lock) { - // Find the classes in subClasses collection to operate on - // (without advancing the candidatesToFindSubclassesIterator iterator - it will be moved after the class successfully handled - to protect against PCE, INRE, etc) - // The found class will be marked as being analyzed - with PROCESSING_SUBCLASSES_STATUS flag in its user data - HashSetQueue.PositionalIterator.IteratorPosition startPosition = candidatesToFindSubclassesIterator.position().next(); - PsiClass candidate = startPosition == null ? null : findNextClassInQueue(startPosition); - if (candidate != null) { - currentlyProcessingClasses.down(); - } - return candidate; - } - } - }); - if (candidate == null) { - // no candidates left in queue, exit - // but first, wait for other threads to process their candidates - break; - } - - try { - DirectClassInheritorsSearch.search(candidate, projectScope).forEach(subClass -> { - ProgressManager.checkCanceled(); - PsiAnchor pointer = PsiAnchor.create(subClass); - // append found result to subClasses as early as possible to allow other waiting threads to continue - synchronized (lock) { - subClasses.add(pointer); - } - return true; - }); - } - finally { - candidate.putUserData(PROCESSING_SUBCLASSES_STATUS, ClassProcessingStatus.PROCESSING_FINISHED); - currentlyProcessingClasses.up(); - } - - synchronized (lock) { - if (subClassIterator.hasNext()) { - // we've added something to subClasses so we can return and the iterator can move forward at least once; - // more elements will be added on the subsequent call to .next() - return; - } - } - } - - // Found nothing, have to wait for other threads because: - // The first thread comes and takes a class off the queue to search for inheritors, - // the second thread comes and sees there is no classes in the queue. - // The second thread should not return nothing, it should wait for the first thread to finish. - // - // Wait within managedBlock to signal FJP this thread is locked (to avoid thread starvation and deadlocks) - try { - ForkJoinPool.managedBlock(new ForkJoinPool.ManagedBlocker() { - @Override - public boolean block() throws InterruptedException { - currentlyProcessingClasses.waitFor(); // wait until other threads process their classes before giving up - return isReleasable(); - } - - @Override - public boolean isReleasable() { - synchronized (lock) { - return !currentlyProcessingClasses.isDown() || subClassIterator.hasNext(); - } - } - }); - } - catch (InterruptedException e) { - throw new RuntimeException(e); - } - } - } } diff --git a/java/java-indexing-impl/src/com/intellij/psi/impl/search/LazyConcurrentCollection.java b/java/java-indexing-impl/src/com/intellij/psi/impl/search/LazyConcurrentCollection.java new file mode 100644 index 000000000000..8ee55b3beffc --- /dev/null +++ b/java/java-indexing-impl/src/com/intellij/psi/impl/search/LazyConcurrentCollection.java @@ -0,0 +1,231 @@ +/* + * Copyright 2000-2016 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.psi.impl.search; + +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.progress.ProgressManager; +import com.intellij.openapi.util.Computable; +import com.intellij.openapi.util.Pair; +import com.intellij.util.Consumer; +import com.intellij.util.Function; +import com.intellij.util.concurrency.Semaphore; +import com.intellij.util.containers.HashSetQueue; +import com.intellij.util.containers.Predicate; +import gnu.trove.THashSet; +import org.jetbrains.annotations.NotNull; + +import java.util.Iterator; +import java.util.Set; +import java.util.concurrent.ForkJoinPool; + +/** + * Collection of elements of type V which is + * - lazy (computes its elements on demand, when the corresponding .iterator().next() method is called) + * - thread safe (multiple threads can iterate this collection concurrently. Already computed elements are shared between these threads. If more elements need to be computed, these computations happen concurrently; different threads can process different elements helping each other) + * - uses some other type T internally to store already computed elements, e.g. for reducing memory. + * + * When more elements needed, this collection iterates not yet processed elements, calls applicable filter on each, calls generator on applicable elements, + * adds generated elements back to the collection. + * Clients must provide: + * -- convertor T->V + * -- filter on V to use to find the applicable elements in the collection to compute more elements + * -- generator which for applicable element:V produces more elements:T + */ +class LazyConcurrentCollection implements Iterable { + // Computes all sub classes of the 'baseClass' transitively by calling DirectClassInheritorsSearch repeatedly. + // Already computed subclasses are stored in this collection. + // There are two iterators maintained for this collection: + // - 'candidatesToFindSubclassesIterator' points to the next element for which direct inheritors haven't been searched yet. + // - 'subClassIterator' created in AllSubClassesLazyCollection.iterator() maintains state of the AllSubClassesLazyCollection iterator in a lazy fashion. + // If more elements requested for this iterator, the processMoreSubclasses() is called which tries to populate 'subClasses' with more inheritors. + private final HashSetQueue subClasses; // guarded by lock + private final Object lock = new Object(); // MUST NOT acquire read action inside this lock + @NotNull private final Function myConvertor; + @NotNull private final MoreElementsGenerator myGenerator; + @NotNull private final Predicate myApplicableFilter; + private final Semaphore currentlyProcessingClasses = new Semaphore(); + + private final HashSetQueue.PositionalIterator candidatesToFindSubclassesIterator; // guarded by lock + // classes for which DirectClassInheritorsSearch is running + private final Set classesBeingProcessed = new THashSet<>(); // guarded by lock + // Classes for which DirectClassInheritorsSearch has already run (maybe in the other thread), + // but candidatesToFindSubclassesIterator hasn't caught them up yet. Elements from this set are removed as the iterator moves. + private final Set classesProcessed = new THashSet<>(); // guarded by lock + + LazyConcurrentCollection(@NotNull T seedElement, + @NotNull Function convertor, + @NotNull Predicate applicableFilter, + @NotNull MoreElementsGenerator generator) { + subClasses = new HashSetQueue<>(); + subClasses.add(seedElement); + myConvertor = convertor; + myGenerator = generator; + myApplicableFilter = applicableFilter; + candidatesToFindSubclassesIterator = subClasses.iterator(); + } + + @FunctionalInterface + interface MoreElementsGenerator { + void generateMoreElementsFor(@NotNull V element, @NotNull Consumer processor); + } + + @NotNull + @Override + public Iterator iterator() { + return new Iterator() { + private final Iterator subClassIterator = subClasses.iterator(); // guarded by lock + { + synchronized (lock) { + subClassIterator.next(); //skip the baseClass which stored in the subClasses first element + } + } + @Override + public boolean hasNext() { + synchronized (lock) { + if (subClassIterator.hasNext()) return true; + } + + processMoreSubclasses(subClassIterator); + + synchronized (lock) { + return subClassIterator.hasNext(); + } + } + + @Override + public V next() { + T next; + synchronized (lock) { + next = subClassIterator.next(); + } + return myConvertor.fun(next); + } + }; + } + + private Pair.NonNull findNextClassInQueue(@NotNull HashSetQueue.PositionalIterator.IteratorPosition position) { + // find the first class which is fit (not anonymous and not final and retrievable from PsiAnchor) and not already processed (flag PROCESSING_SUBCLASSES_STATUS in class user data) + // couldn't call iterator.next() until class is processed, so use position.peek()/position.next() which don't advance iterator + while (position != null) { + ProgressManager.checkCanceled(); + T anchor = position.peek(); + V value = myConvertor.fun(anchor); + boolean isAccepted = value != null && myApplicableFilter.apply(value); + + if (isAccepted && !classesProcessed.contains(anchor) && classesBeingProcessed.add(anchor)) { + return Pair.createNonNull(anchor, value); + } + // the candidate is already being processed in the other thread, try the next one (not advancing iterator!) + position = position.next(); + } + return null; + } + + // polls 'subClasses' for more sub classes and call DirectClassInheritorsSearch for them + private void processMoreSubclasses(@NotNull Iterator subClassIterator) { + while (true) { + ProgressManager.checkCanceled(); + + Pair.NonNull pair = + ApplicationManager.getApplication().runReadAction(new Computable>() { + @Override + public Pair.NonNull compute() { + synchronized (lock) { + // Find the classes in subClasses collection to operate on + // (without advancing the candidatesToFindSubclassesIterator iterator - it will be moved after the class successfully handled - to protect against PCE, INRE, etc) + // The found class will be marked as being analyzed - placed in classesBeingProcessed collection + HashSetQueue.PositionalIterator.IteratorPosition startPosition = candidatesToFindSubclassesIterator.position().next(); + Pair.NonNull pair = startPosition == null ? null : findNextClassInQueue(startPosition); + if (pair != null) { + currentlyProcessingClasses.down(); + } + return pair; + } + } + }); + if (pair == null) { + // no candidates left in queue, exit + // but first, wait for other threads to process their candidates + break; + } + + V candidate = pair.getSecond(); + T anchor = pair.getFirst(); + try { + myGenerator.generateMoreElementsFor(candidate, generatedElement -> { + synchronized (lock) { + subClasses.add(generatedElement); + } + }); + } + finally { + currentlyProcessingClasses.up(); + } + + synchronized (lock) { + classesBeingProcessed.remove(anchor); + classesProcessed.add(anchor); + advanceIteratorOnSuccess(); + if (subClassIterator.hasNext()) { + // we've added something to subClasses so we can return and the iterator can move forward at least once; + // more elements will be added on the subsequent call to .next() + return; + } + } + } + + // Found nothing, have to wait for other threads because: + // The first thread comes and takes a class off the queue to search for inheritors, + // the second thread comes and sees there is no classes in the queue. + // The second thread should not return nothing, it should wait for the first thread to finish. + // + // Wait within managedBlock to signal FJP this thread is locked (to avoid thread starvation and deadlocks) + try { + ForkJoinPool.managedBlock(new ForkJoinPool.ManagedBlocker() { + @Override + public boolean block() throws InterruptedException { + currentlyProcessingClasses.waitFor(); // wait until other threads process their classes before giving up + return isReleasable(); + } + + @Override + public boolean isReleasable() { + synchronized (lock) { + return !currentlyProcessingClasses.isDown() || subClassIterator.hasNext(); + } + } + }); + } + catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + private void advanceIteratorOnSuccess() { + HashSetQueue.PositionalIterator.IteratorPosition position = candidatesToFindSubclassesIterator.position().next(); + while (position != null) { + T next = position.peek(); + if (classesProcessed.contains(next)) { + candidatesToFindSubclassesIterator.next(); + classesProcessed.remove(next); + } + else { + break; + } + position = position.next(); + } + } +}