From 5225bfde704a2e7765bd41dc6314c63519e0fa7d Mon Sep 17 00:00:00 2001 From: Eugene Zhuravlev Date: Fri, 3 Nov 2023 17:11:09 +0100 Subject: [PATCH] JPS mappings for incremental compilation refactoring: support "incremental decision" while differentiating GitOrigin-RevId: 2f736a169aa63ed486f099edfbb68f4c58cd6cc0 --- .../java/dependencyView/Mappings.java | 15 ++++-- .../java/JavaDifferentiateStrategy.java | 45 ++++++++++++---- .../jetbrains/jps/dependency/java/Utils.java | 54 ++----------------- 3 files changed, 51 insertions(+), 63 deletions(-) diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java index 9f0dc9b29156..e3766756ca96 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java @@ -973,8 +973,8 @@ public final class Mappings { final Collection affectedFiles, final Collection currentlyCompiled, final @Nullable DependentFilesFilter filter) { - final boolean isField = member instanceof FieldRepr; final Util self = new Util(); + final int classname = member instanceof ClassRepr? member.name : owner; // Public branch --- hopeless if (member.isPublic()) { @@ -987,9 +987,16 @@ public final class Mappings { // Protected branch if (member.isProtected()) { debug("Protected access, softening non-incremental decision: adding all relevant subclasses for a recompilation"); - debug("Root class: ", owner); + debug("Root class: ", classname); - final IntSet propagated = self.propagateFieldAccess(isField ? member.name : myEmptyName, owner); + final IntSet propagated; + if (member instanceof FieldRepr) { + propagated = self.propagateFieldAccess(member.name, classname); + } + else { + propagated = getAllSubclasses(classname); + propagated.remove(classname); + } propagated.forEach(className -> { final Iterable fileNames = classToSourceFileGet(className); if (fileNames != null) { @@ -1003,7 +1010,7 @@ public final class Mappings { }); } - final String cName = myContext.getValue(isField ? owner : member.name); + final String cName = myContext.getValue(classname); if (cName != null) { final String packageName = ClassRepr.getPackageName(cName); diff --git a/jps/jps-builders/src/org/jetbrains/jps/dependency/java/JavaDifferentiateStrategy.java b/jps/jps-builders/src/org/jetbrains/jps/dependency/java/JavaDifferentiateStrategy.java index ba6780372cf9..383fece4912e 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/dependency/java/JavaDifferentiateStrategy.java +++ b/jps/jps-builders/src/org/jetbrains/jps/dependency/java/JavaDifferentiateStrategy.java @@ -148,7 +148,10 @@ public final class JavaDifferentiateStrategy implements DifferentiateStrategy { if (changedClass.isAnnotation() && changedClass.getRetentionPolicy() == RetentionPolicy.SOURCE) { debug("Annotation, retention policy = SOURCE => a switch to non-incremental mode requested"); - return false; + if (!affectOnNonIncrementalChange(context, changedClass.getReferenceID(), changedClass, present)) { + debug("End of Differentiate, returning false"); + return false; + } } if (addedFlags.isProtected()) { @@ -193,7 +196,7 @@ public final class JavaDifferentiateStrategy implements DifferentiateStrategy { if (removedTargets.contains(ElemType.LOCAL_VARIABLE)) { debug("Removed target contains LOCAL_VARIABLE => a switch to non-incremental mode requested"); - if (!present.incrementalDecision(context, changedClass, null)) { + if (!affectOnNonIncrementalChange(context, changedClass.getReferenceID(), changedClass, present)) { debug("End of Differentiate, returning false"); return false; } @@ -652,11 +655,10 @@ public final class JavaDifferentiateStrategy implements DifferentiateStrategy { if (!context.getParams().isProcessConstantsIncrementally() && !removedField.isPrivate() && removedField.isInlinable() && removedField.getValue() != null) { debug("Field had value and was (non-private) final => a switch to non-incremental mode requested"); - // todo: need support incremental decision? - //if (!incrementalDecision(it.name, f, myAffectedFiles, myFilesToCompile, myFilter)) { + if (!affectOnNonIncrementalChange(context, changedClass.getReferenceID(), removedField, present)) { debug("End of Differentiate, returning false"); return false; - //} + } } Set propagated = present.collectSubclassesWithoutField(changedClass.getReferenceID(), removedField.getName()); @@ -701,11 +703,10 @@ public final class JavaDifferentiateStrategy implements DifferentiateStrategy { } else { debug("Potentially inlined field changed its access or value => a switch to non-incremental mode requested"); - // todo - //if (!incrementalDecision(it.name, field, myAffectedFiles, myFilesToCompile, myFilter)) { + if (!affectOnNonIncrementalChange(context, changedClass.getReferenceID(), changedField, present)) { debug("End of Differentiate, returning false"); return false; - //} + } } } } @@ -916,13 +917,39 @@ public final class JavaDifferentiateStrategy implements DifferentiateStrategy { } } + // todo: probably support a file filter over a module structure + private static boolean affectOnNonIncrementalChange(DifferentiateContext context, JvmNodeReferenceID owner, Proto proto, Utils utils) { + if (proto.isPublic()) { + debug("Public access, switching to a non-incremental mode"); + return false; + } + + if (proto.isProtected()) { + debug("Protected access, softening non-incremental decision: adding all relevant subclasses for a recompilation"); + debug("Root class: ", owner); + for (ReferenceID id : proto instanceof JvmField? utils.collectSubclassesWithoutField(owner, proto.getName()) : utils.allSubclasses(owner)) { + affectNodeSources(context, id, "Adding "); + } + } + + String packageName = JvmClass.getPackageName(owner.getNodeName()); + debug("Softening non-incremental decision: adding all package classes for a recompilation"); + debug("Package name: ", packageName); + for (ReferenceID nodeWithinPackage : Iterators.filter(context.getGraph().getRegisteredNodes(), id -> id instanceof JvmNodeReferenceID && packageName.equals(JvmClass.getPackageName(((JvmNodeReferenceID)id).getNodeName())))) { + affectNodeSources(context, nodeWithinPackage, "Adding "); + } + + return true; + } + private static void affectNodeSources(DifferentiateContext context, ReferenceID clsId, String affectReason) { affectNodeSources(context, clsId, affectReason, false); } private static void affectNodeSources(DifferentiateContext context, ReferenceID clsId, String affectReason, boolean forceAffect) { + Set deletedSources = context.getDelta().getDeletedSources(); for (NodeSource source : context.getGraph().getSources(clsId)) { - if (forceAffect || !context.isCompiled(source)) { + if (forceAffect || !context.isCompiled(source) && !deletedSources.contains(source)) { context.affectNodeSource(source); debug(affectReason, source.getPath()); } diff --git a/jps/jps-builders/src/org/jetbrains/jps/dependency/java/Utils.java b/jps/jps-builders/src/org/jetbrains/jps/dependency/java/Utils.java index 6b608a419fd7..db8d3b4ae028 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/dependency/java/Utils.java +++ b/jps/jps-builders/src/org/jetbrains/jps/dependency/java/Utils.java @@ -5,7 +5,10 @@ import com.intellij.openapi.util.Pair; import com.intellij.util.SmartList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.jps.dependency.*; +import org.jetbrains.jps.dependency.BackDependencyIndex; +import org.jetbrains.jps.dependency.Graph; +import org.jetbrains.jps.dependency.NodeSource; +import org.jetbrains.jps.dependency.ReferenceID; import org.jetbrains.jps.javac.Iterators; import java.util.*; @@ -289,55 +292,6 @@ public final class Utils { } - public boolean incrementalDecision(DifferentiateContext context, JvmClass owner, @Nullable JvmField field) { - // Public branch --- hopeless - - if ((field != null? field : owner).isPublic()) { - debug("Public access, switching to a non-incremental mode"); - return false; - } - - // Protected branch - - Set toRecompile = new HashSet<>(); - if ((field != null? field : owner).isProtected()) { - debug("Protected access, softening non-incremental decision: adding all relevant subclasses for a recompilation"); - debug("Root class: " + owner.getName()); - - Set propagated; - if (field != null) { - propagated = collectSubclassesWithoutField(owner.getReferenceID(), field.getName()); - } - else { - JvmNodeReferenceID ownerID = owner.getReferenceID(); - propagated = new HashSet<>(); - for (ReferenceID id : withAllSubclasses(ownerID)) { - if (id instanceof JvmNodeReferenceID && !id.equals(ownerID)) { - propagated.add((JvmNodeReferenceID)id); - } - } - } - Iterators.collect(Iterators.flat(Iterators.map(propagated, id -> myGraph.getSources(id))), toRecompile); - } - - // Package-local branch - - String packageName = owner.getPackageName(); - debug("Softening non-incremental decision: adding all package classes for a recompilation"); - debug("Package name: " + packageName); - - Iterators.collect(Iterators.flat(Iterators.map( - Iterators.filter(myGraph.getRegisteredNodes(), id -> id instanceof JvmNodeReferenceID && packageName.equals(JvmClass.getPackageName(((JvmNodeReferenceID)id).getNodeName()))), - id -> myGraph.getSources(id) - )), toRecompile); - - for (NodeSource source : Iterators.filter(toRecompile, s -> !context.isCompiled(s) && !context.getDelta().getDeletedSources().contains(s))) { - context.affectNodeSource(source); - } - - return true; - } - private static Function cachingFunction(Function f) { return new Function<>() { private final Map cache = new HashMap<>();