files compiled on first round may require additional recompilation on the next round (second part for IDEA-116914)

constant search on the IDE side rewritten: only direct dependencies are returned for each search round; all transitive dependencies will be additionally handled by make
This commit is contained in:
Eugene Zhuravlev
2013-12-17 11:26:21 +01:00
parent eebad4857d
commit fbbb85580b
17 changed files with 193 additions and 137 deletions
@@ -15,7 +15,6 @@
*/
package com.intellij.compiler.server;
import com.intellij.compiler.make.CachingSearcher;
import com.intellij.lang.java.JavaLanguage;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.diagnostic.Logger;
@@ -24,11 +23,9 @@ import com.intellij.openapi.project.DumbService;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Computable;
import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.registry.Registry;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.psi.*;
import com.intellij.psi.search.*;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.SmartList;
import com.intellij.util.cls.ClsUtil;
import com.intellij.util.concurrency.SequentialTaskExecutor;
@@ -47,15 +44,11 @@ import java.util.*;
*/
public abstract class DefaultMessageHandler implements BuilderMessageHandler {
private static final Logger LOG = Logger.getInstance("#com.intellij.compiler.server.DefaultMessageHandler");
private final int MAX_CONSTANT_SEARCHES = Registry.intValue("compiler.max.static.constants.searches");
private final Project myProject;
private int myConstantSearchesCount = 0;
private final CachingSearcher mySearcher;
private final SequentialTaskExecutor myTaskExecutor = new SequentialTaskExecutor(PooledThreadExecutor.INSTANCE);
protected DefaultMessageHandler(Project project) {
myProject = project;
mySearcher = new CachingSearcher(project);
}
@Override
@@ -64,6 +57,7 @@ public abstract class DefaultMessageHandler implements BuilderMessageHandler {
@Override
public final void handleBuildMessage(final Channel channel, final UUID sessionId, final CmdlineRemoteProto.Message.BuilderMessage msg) {
//noinspection EnumSwitchStatementWhichMissesCases
switch (msg.getType()) {
case BUILD_EVENT:
handleBuildEvent(sessionId, msg.getBuildEvent());
@@ -153,11 +147,11 @@ public abstract class DefaultMessageHandler implements BuilderMessageHandler {
}
else {
for (final PsiField changedField : changedFields) {
final boolean success = performChangedConstantSearch(changedField, accessFlags, accessChanged, affectedPaths);
if (!success) {
isSuccess.set(Boolean.FALSE);
break;
if (!accessChanged && ClsUtil.isPrivate(accessFlags)) {
// optimization: don't need to search, cause may be used only in this class
continue;
}
affectDirectUsages(changedField, accessFlags, accessChanged, affectedPaths);
}
}
}
@@ -199,6 +193,7 @@ public abstract class DefaultMessageHandler implements BuilderMessageHandler {
// wait some time
for (int idx = 0; idx < 5; idx++) {
try {
//noinspection BusyWait
Thread.sleep(10L);
}
catch (InterruptedException ignored) {
@@ -212,40 +207,13 @@ public abstract class DefaultMessageHandler implements BuilderMessageHandler {
return isDumb;
}
private boolean performChangedConstantSearch(PsiField field, int accessFlags, boolean isAccessibilityChange, final Set<String> affectedPaths) {
if (!isAccessibilityChange && ClsUtil.isPrivate(accessFlags)) {
return true; // optimization: don't need to search, cause may be used only in this class
}
final Set<PsiElement> usages = new HashSet<PsiElement>();
try {
addUsages(field, usages, accessFlags, isAccessibilityChange);
ApplicationManager.getApplication().runReadAction(new Runnable() {
public void run() {
for (final PsiElement usage : usages) {
if (usage.isValid()) {
// if usage is invalid the file should be changed anyway and thus compiled later
affect(usage, affectedPaths);
}
}
}
});
}
catch (PsiInvalidElementAccessException ignored) {
LOG.debug("Constant search task: PIEAE thrown while searching of usages of changed constant");
return false;
}
catch (ProcessCanceledException ignored) {
LOG.debug("Constant search task: PCE thrown while searching of usages of changed constant");
return false;
}
return true;
}
private boolean performRemovedConstantSearch(@Nullable final PsiClass aClass, String fieldName, int fieldAccessFlags, final Set<String> affectedPaths) {
final PsiSearchHelper psiSearchHelper = PsiSearchHelper.SERVICE.getInstance(myProject);
final Ref<Boolean> result = new Ref<Boolean>(Boolean.TRUE);
final PsiFile fieldContainingFile = aClass != null? aClass.getContainingFile() : null;
processIdentifiers(psiSearchHelper, new PsiElementProcessor<PsiIdentifier>() {
@Override
public boolean execute(@NotNull PsiIdentifier identifier) {
@@ -253,9 +221,13 @@ public abstract class DefaultMessageHandler implements BuilderMessageHandler {
final PsiElement parent = identifier.getParent();
if (parent instanceof PsiReferenceExpression) {
final PsiClass ownerClass = getOwnerClass(parent);
if (ownerClass != null /*&& !ownerClass.equals(aClass)*/) {
if (ownerClass.getQualifiedName() != null) {
affect(ownerClass, affectedPaths);
if (ownerClass != null && ownerClass.getQualifiedName() != null) {
final PsiFile usageFile = ownerClass.getContainingFile();
if (usageFile != null && !usageFile.equals(fieldContainingFile)) {
final VirtualFile vFile = usageFile.getOriginalFile().getVirtualFile();
if (vFile != null) {
affectedPaths.add(vFile.getPath());
}
}
}
}
@@ -288,16 +260,6 @@ public abstract class DefaultMessageHandler implements BuilderMessageHandler {
return searchScope;
}
private static void affect(PsiElement ownerClass, Set<String> affectedPaths) {
final PsiFile containingPsi = ownerClass.getContainingFile();
if (containingPsi != null) {
final VirtualFile vFile = containingPsi.getOriginalFile().getVirtualFile();
if (vFile != null) {
affectedPaths.add(vFile.getPath());
}
}
}
private static boolean processIdentifiers(PsiSearchHelper helper, @NotNull final PsiElementProcessor<PsiIdentifier> processor, @NotNull final String identifier, @NotNull SearchScope searchScope, short searchContext) {
TextOccurenceProcessor processor1 = new TextOccurenceProcessor() {
@Override
@@ -308,45 +270,35 @@ public abstract class DefaultMessageHandler implements BuilderMessageHandler {
return helper.processElementsWithWord(processor1, searchScope, identifier, searchContext, true, false);
}
private void addUsages(final PsiField psiField, final Collection<PsiElement> usages, final int fieldAccessFlags, final boolean ignoreAccessScope) throws ProcessCanceledException {
final Queue<PsiField> fieldsToProcess = new ArrayDeque<PsiField>();
fieldsToProcess.add(psiField);
for (PsiField field = fieldsToProcess.poll(); field != null; field = fieldsToProcess.poll()) {
final int count = myConstantSearchesCount;
if (count > MAX_CONSTANT_SEARCHES) {
throw new ProcessCanceledException();
}
final PsiField fieldToCheck = field;
ApplicationManager.getApplication().runReadAction(new Runnable() {
public void run() {
if (!fieldToCheck.isValid()) {
// if field is invalid, the file might be changed, so next time it is compiled,
// the constant value change, if any, will be processed
return;
private void affectDirectUsages(final PsiField psiField, final int fieldAccessFlags, final boolean ignoreAccessScope, final Set<String> affectedPaths) throws ProcessCanceledException {
ApplicationManager.getApplication().runReadAction(new Runnable() {
public void run() {
if (psiField.isValid()) {
final PsiFile fieldContainingFile = psiField.getContainingFile();
final Set<PsiFile> processedFiles = new HashSet<PsiFile>();
if (fieldContainingFile != null) {
processedFiles.add(fieldContainingFile);
}
final Collection<PsiReferenceExpression> references = doFindReferences(fieldToCheck, fieldAccessFlags, ignoreAccessScope);
myConstantSearchesCount++;
// if field is invalid, the file might be changed, so next time it is compiled,
// the constant value change, if any, will be processed
final Collection<PsiReferenceExpression> references = doFindReferences(psiField, fieldAccessFlags, ignoreAccessScope);
for (final PsiReferenceExpression ref : references) {
PsiElement e = ref.getElement();
usages.add(e);
PsiField ownerField = getOwnerField(e);
if (ownerField != null && ownerField.hasModifierProperty(PsiModifier.FINAL)) {
final PsiExpression initializer = ownerField.getInitializer();
if (initializer != null && PsiUtil.isConstantExpression(initializer)) {
// if the field depends on the compile-time-constant expression and is itself final
fieldsToProcess.add(ownerField);
final PsiElement usage = ref.getElement();
final PsiFile containingPsi = usage.getContainingFile();
if (containingPsi != null && processedFiles.add(containingPsi)) {
final VirtualFile vFile = containingPsi.getOriginalFile().getVirtualFile();
if (vFile != null) {
affectedPaths.add(vFile.getPath());
}
}
}
}
});
}
}
});
}
private Collection<PsiReferenceExpression> doFindReferences(final PsiField psiField, int fieldAccessFlags, boolean ignoreAccessScope) {
final Collection<PsiReferenceExpression> result = Collections.synchronizedList(new SmartList<PsiReferenceExpression>());
final SmartList<PsiReferenceExpression> result = new SmartList<PsiReferenceExpression>();
final SearchScope searchScope = ignoreAccessScope? GlobalSearchScope.projectScope(myProject) : getSearchScope(psiField.getContainingClass(), fieldAccessFlags);
@@ -357,7 +309,10 @@ public abstract class DefaultMessageHandler implements BuilderMessageHandler {
if (parent instanceof PsiReferenceExpression) {
final PsiReferenceExpression refExpression = (PsiReferenceExpression)parent;
if (refExpression.isReferenceTo(psiField)) {
result.add(refExpression);
synchronized (result) {
// processor's code may be invoked from multiple threads
result.add(refExpression);
}
}
}
return true;
@@ -386,18 +341,4 @@ public abstract class DefaultMessageHandler implements BuilderMessageHandler {
return null;
}
@Nullable
private static PsiField getOwnerField(PsiElement element) {
while (!(element instanceof PsiFile)) {
if (element instanceof PsiClass) {
break;
}
if (element instanceof PsiField) { // top-level class
return (PsiField)element;
}
element = element.getParent();
}
return null;
}
}
@@ -0,0 +1,15 @@
Cleaning output files:
out/production/AddClassHidingImportedClass2/package3/C.class
End of files
Compiling files:
src/package2/A.java
src/package3/C.java
End of files
Cleaning output files:
out/production/AddClassHidingImportedClass2/package2/B.class
out/production/AddClassHidingImportedClass2/package3/C.class
End of files
Compiling files:
src/package2/B.java
src/package3/C.java
End of files
@@ -0,0 +1,7 @@
package package1;
public class A {
public static class D {
public final String s = new String("package1");
}
}
@@ -0,0 +1,7 @@
package package2;
public class A {
public static class D {
public final String s = new String("package2");
}
}
@@ -0,0 +1,6 @@
package package2;
import package1.*;
public class B extends A{
}
@@ -0,0 +1,15 @@
package package3;
import package2.B;
public class C {
public B.D p;
public String get() {
p = new B.D();
return p.s;
}
public void dummy() {}
}
@@ -0,0 +1,16 @@
package package3;
import package2.B;
public class C {
public B.D p;
public String get() {
p = new B.D();
return p.s;
}
// commenting dummy method => causing class change => compilation on the first round
// public void dummy() {}
}
@@ -12,3 +12,9 @@ Compiling files:
src/Client.java
src/ServerClient.java
End of files
Cleaning output files:
out/production/ConstantChain/Server.class
End of files
Compiling files:
src/Server.java
End of files
@@ -0,0 +1,16 @@
Cleaning output files:
out/production/MutualConstants/constants/B.class
End of files
Compiling files:
src/constants/B.java
End of files
Cleaning output files:
out/production/MutualConstants/constants/A.class
out/production/MutualConstants/constants/B.class
out/production/MutualConstants/constants/PrintConst.class
End of files
Compiling files:
src/constants/A.java
src/constants/B.java
src/constants/PrintConst.java
End of files
@@ -0,0 +1,5 @@
package constants;
public class A {
public static final int CONST_A = B.CONST_B_1 + 1;
}
@@ -0,0 +1,5 @@
package constants;
public class B {
public static final int CONST_B_1 = 0;
public static final int CONST_B_2 = A.CONST_A + 1;
}
@@ -0,0 +1,5 @@
package constants;
public class B {
public static final int CONST_B_1 = 1;
public static final int CONST_B_2 = A.CONST_A + 1;
}
@@ -0,0 +1,7 @@
package constants;
public class PrintConst {
public static void main(String[] args) {
System.out.println(B.CONST_B_2);
}
}
@@ -114,7 +114,6 @@ public class JavaBuilderUtil {
if (incremental) {
final Set<File> newlyAffectedFiles = new HashSet<File>(allAffectedFiles);
newlyAffectedFiles.removeAll(affectedBeforeDif);
newlyAffectedFiles.removeAll(allCompiledFiles); // the diff operation may have affected the class already compiled in thic compilation round
final String infoMessage = "Dependency analysis found " + newlyAffectedFiles.size() + " affected files";
LOG.info(infoMessage);
@@ -544,7 +544,7 @@ public class Mappings {
}
}
void affectSubclasses(final int className, final Collection<File> affectedFiles, final Collection<UsageRepr.Usage> affectedUsages, final TIntHashSet dependants, final boolean usages) {
void affectSubclasses(final int className, final Collection<File> affectedFiles, final Collection<UsageRepr.Usage> affectedUsages, final TIntHashSet dependants, final boolean usages, final Collection<File> alreadyCompiledFiles) {
debug("Affecting subclasses of class: ", className);
final File fileName = myClassToSourceFile.get(className);
@@ -570,14 +570,16 @@ public class Mappings {
if (depClasses != null) {
addAll(dependants, depClasses);
}
affectedFiles.add(fileName);
if (!alreadyCompiledFiles.contains(fileName)) {
affectedFiles.add(fileName);
}
final TIntHashSet directSubclasses = myClassToSubclasses.get(className);
if (directSubclasses != null) {
directSubclasses.forEach(new TIntProcedure() {
@Override
public boolean execute(int subClass) {
affectSubclasses(subClass, affectedFiles, affectedUsages, dependants, usages);
affectSubclasses(subClass, affectedFiles, affectedUsages, dependants, usages, alreadyCompiledFiles);
return true;
}
});
@@ -739,7 +741,7 @@ public class Mappings {
return acc;
}
private boolean incrementalDecision(final int owner, final Proto member, final Collection<File> affectedFiles, @Nullable final DependentFilesFilter filter) {
private boolean incrementalDecision(final int owner, final Proto member, final Collection<File> affectedFiles, final Collection<File> currentlyCompiled, @Nullable final DependentFilesFilter filter) {
final boolean isField = member instanceof FieldRepr;
final Util self = new Util();
@@ -759,7 +761,7 @@ public class Mappings {
@Override
public boolean execute(int className) {
final File fileName = myClassToSourceFile.get(className);
if (fileName != null) {
if (fileName != null && !currentlyCompiled.contains(fileName)) {
debug("Adding ", fileName);
affectedFiles.add(fileName);
}
@@ -778,7 +780,7 @@ public class Mappings {
@Override
public boolean execute(int className, File fileName) {
if (ClassRepr.getPackageName(myContext.getValue(className)).equals(packageName)) {
if (filter == null || filter.accept(fileName)) {
if ((filter == null || filter.accept(fileName)) && !currentlyCompiled.contains(fileName)) {
debug("Adding: ", fileName);
affectedFiles.add(fileName);
}
@@ -881,7 +883,7 @@ public class Mappings {
debug("Constant search service not available.");
}
debug("Trying to soften non-incremental decision.");
if (!incrementalDecision(t.owner, t.field, affectedFiles, myFilter)) {
if (!incrementalDecision(t.owner, t.field, affectedFiles, myFilesToCompile, myFilter)) {
debug("No luck.");
debug("End of delayed work, returning false.");
return false;
@@ -1028,7 +1030,7 @@ public class Mappings {
debug("Method: ", m.name);
if (it.isInterface() || it.isAbstract() || m.isAbstract()) {
debug("Class is abstract, or is interface, or added method in abstract => affecting all subclasses");
myFuture.affectSubclasses(it.name, myAffectedFiles, state.myAffectedUsages, state.myDependants, false);
myFuture.affectSubclasses(it.name, myAffectedFiles, state.myAffectedUsages, state.myDependants, false, myCompiledFiles);
}
TIntHashSet propagated = null;
@@ -1115,26 +1117,23 @@ public class Mappings {
}
final TIntHashSet subClasses = getAllSubclasses(it.name);
if (subClasses != null) {
subClasses.forEach(new TIntProcedure() {
@Override
public boolean execute(int subClass) {
final ClassRepr r = myFuture.reprByName(subClass);
if (r != null) {
final File sourceFileName = myClassToSourceFile.get(subClass);
if (sourceFileName != null) {
final int outerClass = r.getOuterClassName();
if (!isEmpty(outerClass) && myFuture.isMethodVisible(outerClass, m)) {
myAffectedFiles.add(sourceFileName);
debug("Affecting file due to local overriding: ", sourceFileName);
}
subClasses.forEach(new TIntProcedure() {
@Override
public boolean execute(int subClass) {
final ClassRepr r = myFuture.reprByName(subClass);
if (r != null) {
final File sourceFileName = myClassToSourceFile.get(subClass);
if (sourceFileName != null && !myCompiledFiles.contains(sourceFileName)) {
final int outerClass = r.getOuterClassName();
if (!isEmpty(outerClass) && myFuture.isMethodVisible(outerClass, m)) {
myAffectedFiles.add(sourceFileName);
debug("Affecting file due to local overriding: ", sourceFileName);
}
}
return true;
}
});
}
return true;
}
});
}
}
debug("End of added methods processing");
@@ -1225,9 +1224,9 @@ public class Mappings {
if (allAbstract && visited) {
final File source = myClassToSourceFile.get(p);
if (source != null) {
if (source != null && !myCompiledFiles.contains(source)) {
myAffectedFiles.add(source);
debug("Removed method is not abstract & overrides some abstract method which is not then over-overriden in subclass ", p);
debug("Removed method is not abstract & overrides some abstract method which is not then over-overridden in subclass ", p);
debug("Affecting subclass source file ", source);
}
}
@@ -1320,7 +1319,7 @@ public class Mappings {
if ((d.addedModifiers() & Opcodes.ACC_STATIC) > 0) {
debug("Added static specifier --- affecting subclasses");
myFuture.affectSubclasses(it.name, myAffectedFiles, state.myAffectedUsages, state.myDependants, false);
myFuture.affectSubclasses(it.name, myAffectedFiles, state.myAffectedUsages, state.myDependants, false, myCompiledFiles);
}
}
else {
@@ -1328,7 +1327,7 @@ public class Mappings {
(d.addedModifiers() & Opcodes.ACC_PUBLIC) > 0 ||
(d.addedModifiers() & Opcodes.ACC_ABSTRACT) > 0) {
debug("Added final, public or abstract specifier --- affecting subclasses");
myFuture.affectSubclasses(it.name, myAffectedFiles, state.myAffectedUsages, state.myDependants, false);
myFuture.affectSubclasses(it.name, myAffectedFiles, state.myAffectedUsages, state.myDependants, false, myCompiledFiles);
}
if ((d.addedModifiers() & Opcodes.ACC_PROTECTED) > 0 && !((d.removedModifiers() & Opcodes.ACC_PRIVATE) > 0)) {
@@ -1369,7 +1368,7 @@ public class Mappings {
final ClassRepr r = myFuture.reprByName(subClass);
if (r != null) {
final File sourceFileName = myClassToSourceFile.get(subClass);
if (sourceFileName != null) {
if (sourceFileName != null && !myCompiledFiles.contains(sourceFileName)) {
if (r.isLocal()) {
debug("Affecting local subclass (introduced field can potentially hide surrounding method parameters/local variables): ", sourceFileName);
myAffectedFiles.add(sourceFileName);
@@ -1461,7 +1460,7 @@ public class Mappings {
myDelayedWorks.addConstantWork(it.name, f, true, false);
}
else {
if (!incrementalDecision(it.name, f, myAffectedFiles, myFilter)) {
if (!incrementalDecision(it.name, f, myAffectedFiles, myFilesToCompile, myFilter)) {
debug("End of Differentiate, returning false");
return false;
}
@@ -1501,7 +1500,7 @@ public class Mappings {
myDelayedWorks.addConstantWork(it.name, field, false, accessChanged);
}
else {
if (!incrementalDecision(it.name, field, myAffectedFiles, myFilter)) {
if (!incrementalDecision(it.name, field, myAffectedFiles, myFilesToCompile, myFilter)) {
debug("End of Differentiate, returning false");
return false;
}
@@ -1618,7 +1617,7 @@ public class Mappings {
debug("Extends changed: ", extendsChanged);
debug("Interfaces removed: ", interfacesRemoved);
myFuture.affectSubclasses(changedClass.name, myAffectedFiles, state.myAffectedUsages, state.myDependants, extendsChanged || interfacesRemoved || signatureChanged);
myFuture.affectSubclasses(changedClass.name, myAffectedFiles, state.myAffectedUsages, state.myDependants, extendsChanged || interfacesRemoved || signatureChanged, myCompiledFiles);
if (!changedClass.isAnonymous()) {
final TIntHashSet parents = new TIntHashSet();
@@ -1652,7 +1651,7 @@ public class Mappings {
if (changedClass.isAnnotation() && changedClass.getRetentionPolicy() == RetentionPolicy.SOURCE) {
debug("Annotation, retention policy = SOURCE => a switch to non-incremental mode requested");
if (!incrementalDecision(changedClass.getOuterClassName(), changedClass, myAffectedFiles, myFilter)) {
if (!incrementalDecision(changedClass.getOuterClassName(), changedClass, myAffectedFiles, myFilesToCompile, myFilter)) {
debug("End of Differentiate, returning false");
return false;
}
@@ -1696,7 +1695,7 @@ public class Mappings {
if (removedtargets.contains(ElemType.LOCAL_VARIABLE)) {
debug("Removed target contains LOCAL_VARIABLE => a switch to non-incremental mode requested");
if (!incrementalDecision(changedClass.getOuterClassName(), changedClass, myAffectedFiles, myFilter)) {
if (!incrementalDecision(changedClass.getOuterClassName(), changedClass, myAffectedFiles, myFilesToCompile, myFilter)) {
debug("End of Differentiate, returning false");
return false;
}
@@ -1788,7 +1787,6 @@ public class Mappings {
debug("Scheduling for recompilation duplicated sources: ", currentlyMappedTo.getPath() + "; " + srcFile.getPath());
myAffectedFiles.add(currentlyMappedTo);
myAffectedFiles.add(srcFile);
myCompiledFiles.remove(srcFile); // this will force sending the file to compilation again
return; // do not process this file because it should not be integrated
}
break;
@@ -118,4 +118,8 @@ public class CommonTest extends IncrementalTestCase {
doTest();
}
public void testAddClassHidingImportedClass2() throws Exception {
doTest();
}
}
@@ -104,6 +104,10 @@ public class FieldPropertyTest extends IncrementalTestCase {
// }
public void testNonIncremental4() throws Exception {
doTest();
}
doTest();
}
public void testMutualConstants() throws Exception {
doTest();
}
}