Skip to content

Commit

Permalink
JPS mappings for incremental compilation refactoring: support affecte…
Browse files Browse the repository at this point in the history
…d files filter; add duplicate classes check

GitOrigin-RevId: 3513e6903ea8e490acf6c48641f926e84fa8ba73
  • Loading branch information
Eugene Zhuravlev authored and intellij-monorepo-bot committed Nov 6, 2023
1 parent 3294414 commit 8f1258a
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import org.jetbrains.jps.builders.storage.BuildDataCorruptedException;
import org.jetbrains.jps.dependency.Delta;
import org.jetbrains.jps.dependency.DependencyGraph;
import org.jetbrains.jps.dependency.DifferentiateParameters;
import org.jetbrains.jps.dependency.DifferentiateResult;
import org.jetbrains.jps.dependency.impl.DifferentiateParametersBuilder;
import org.jetbrains.jps.dependency.impl.FileSource;
import org.jetbrains.jps.incremental.*;
import org.jetbrains.jps.incremental.fs.CompilationRound;
Expand Down Expand Up @@ -420,11 +420,14 @@ private static boolean updateDependencyGraph(CompileContext context, Delta delta
final boolean errorsDetected = Utils.errorsDetected(context);
BuildDataManager dataManager = context.getProjectDescriptor().dataManager;
DependencyGraph graph = dataManager.getDependencyGraph();
boolean calculateAffected = context.shouldDifferentiate(chunk) && !isForcedRecompilationAllJavaModules(context);
boolean processConstantsIncrementally = dataManager.isProcessConstantsIncrementally();
DifferentiateResult diffResult = graph.differentiate(delta, new DifferentiateParameters(calculateAffected, processConstantsIncrementally));

final ModulesBasedFileFilter moduleBasedFilter = new ModulesBasedFileFilter(context, chunk);
DifferentiateParametersBuilder params = DifferentiateParametersBuilder.create()
.calculateAffected(context.shouldDifferentiate(chunk) && !isForcedRecompilationAllJavaModules(context))
.processConstantsIncrementally(dataManager.isProcessConstantsIncrementally())
.withAffectionFilter(s -> moduleBasedFilter.accept(s.getPath().toFile()))
.withChunkStructureFilter(s -> moduleBasedFilter.belongsToCurrentTargetChunk(s.getPath().toFile()));
DifferentiateResult diffResult = graph.differentiate(delta, params.get());

final boolean compilingIncrementally = isCompileJavaIncrementally(context);

if (diffResult.isIncremental()) {
Expand Down Expand Up @@ -677,13 +680,18 @@ public boolean accept(File file) {
if (rd == null) {
return true;
}
final ModuleBuildTarget targetOfFile = rd.target;
final BuildTarget<?> targetOfFile = rd.target;
if (myChunkTargets.contains(targetOfFile)) {
return true;
}
Set<BuildTarget<?>> targetOfFileWithDependencies = myCache.get(targetOfFile);
if (targetOfFileWithDependencies == null) {
targetOfFileWithDependencies = myBuildTargetIndex.getDependenciesRecursively(targetOfFile, myContext);
targetOfFileWithDependencies = Iterators.collect(Iterators.recurseDepth(targetOfFile, new Iterators.Function<BuildTarget<?>, Iterable<? extends BuildTarget<?>>>() {
@Override
public Iterable<? extends BuildTarget<?>> fun(BuildTarget<?> t) {
return myBuildTargetIndex.getDependencies(t, myContext);
}
}, false), new LinkedHashSet<>());
myCache.put(targetOfFile, targetOfFileWithDependencies);
}
return ContainerUtil.intersects(targetOfFileWithDependencies, myChunkTargets);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,36 +1,19 @@
// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package org.jetbrains.jps.dependency;

public final class DifferentiateParameters {
private final boolean calculateAffected;
private final boolean processConstantsIncrementally;
import org.jetbrains.annotations.NotNull;

public DifferentiateParameters() {
this(true, true);
}
import java.util.function.Predicate;

public boolean isCalculateAffected() {
return calculateAffected;
}
public interface DifferentiateParameters {

public boolean isProcessConstantsIncrementally() {
return processConstantsIncrementally;
}
boolean isCalculateAffected();

public DifferentiateParameters(boolean calculateAffected, boolean processConstantsIncrementally) {
this.calculateAffected = calculateAffected;
this.processConstantsIncrementally = processConstantsIncrementally;
}
boolean isProcessConstantsIncrementally();

public static DifferentiateParameters byDefault() {
return new DifferentiateParameters();
}
@NotNull
Predicate<? super NodeSource> affectionFilter();

public static DifferentiateParameters withoutAffectedCalculation() {
return new DifferentiateParameters(false, true);
}

public static DifferentiateParameters processConstantsNonIncremental() {
return new DifferentiateParameters(true, false);
}
@NotNull
Predicate<? super NodeSource> belongsToCurrentCompilationChunk();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package org.jetbrains.jps.dependency.impl;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.jps.dependency.DifferentiateParameters;
import org.jetbrains.jps.dependency.NodeSource;

import java.util.function.Predicate;

public final class DifferentiateParametersBuilder implements DifferentiateParameters {
private boolean calculateAffected = true;
private boolean processConstantsIncrementally = true;
private Predicate<? super NodeSource> myAffectionFilter = s -> true;
private Predicate<? super NodeSource> myCurrentChunkFilter = s -> true;

private DifferentiateParametersBuilder() {
}

@Override
public boolean isCalculateAffected() {
return calculateAffected;
}

@Override
public boolean isProcessConstantsIncrementally() {
return processConstantsIncrementally;
}

@Override
public @NotNull Predicate<? super NodeSource> affectionFilter() {
return myAffectionFilter;
}

@Override
public @NotNull Predicate<? super NodeSource> belongsToCurrentCompilationChunk() {
return myCurrentChunkFilter;
}

public DifferentiateParameters get() {
return this;
}

public static DifferentiateParametersBuilder create() {
return new DifferentiateParametersBuilder();
}

public static DifferentiateParameters withDefaultSettings() {
return create().get();
}

public DifferentiateParametersBuilder calculateAffected(boolean value) {
calculateAffected = value;
return this;
}

public DifferentiateParametersBuilder processConstantsIncrementally(boolean value) {
processConstantsIncrementally = value;
return this;
}

public DifferentiateParametersBuilder withAffectionFilter(Predicate<? super NodeSource> filter) {
myAffectionFilter = filter;
return this;
}

public DifferentiateParametersBuilder withChunkStructureFilter(Predicate<? super NodeSource> filter) {
myCurrentChunkFilter = filter;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,29 @@ public boolean processAddedClasses(DifferentiateContext context, Difference.Spec
Set<ReferenceID> affectedNodes = new HashSet<>();

for (JvmClass addedClass : addedClasses){
// todo: consider if we need to perform a duplicate-class check, when the newly added class is of the same name with the existing one in the same module chunk
debug("Class name: ", addedClass.getName());

// class duplication checks
if (!addedClass.isAnonymous() && !addedClass.isLocal() && addedClass.getOuterFqName().isEmpty()) {
Set<NodeSource> deletedSources = context.getDelta().getDeletedSources();
Predicate<? super NodeSource> belongsToChunk = context.getParams().belongsToCurrentCompilationChunk();
Set<NodeSource> candidates = Iterators.collect(
Iterators.filter(present.getNodeSources(addedClass.getReferenceID()), s -> !deletedSources.contains(s) && belongsToChunk.test(s)), new HashSet<>()
);

if (!Iterators.isEmpty(Iterators.filter(candidates, src -> !context.isCompiled(src)))) {
Iterators.collect(context.getDelta().getSources(addedClass.getReferenceID()), candidates);
final StringBuilder msg = new StringBuilder();
msg.append("Possibly duplicated classes in the same compilation chunk; Scheduling for recompilation sources: ");
for (NodeSource candidate : candidates) {
context.affectNodeSource(candidate);
msg.append(candidate.getPath()).append("; ");
}
debug(msg.toString());
continue; // if duplicates are found, do not perform further checks for classes with the same short name
}
}

if (!addedClass.isAnonymous() && !addedClass.isLocal()) {
Iterators.collect(index.getDependencies(new JvmNodeReferenceID(addedClass.getShortName())), affectedNodes);
affectedNodes.add(addedClass.getReferenceID());
Expand Down Expand Up @@ -552,7 +572,7 @@ private void processAddedMethods(DifferentiateContext context, JvmClass changedC
if (!Iterators.isEmpty(Iterators.filter(sources, s -> !context.isCompiled(s)))) { // has non-compiled sources
for (JvmClass outerClass : Iterators.flat(Iterators.map(future.getNodes(subClassId, JvmClass.class), cl -> future.getNodes(new JvmNodeReferenceID(cl.getOuterFqName()), JvmClass.class)))) {
if (future.isMethodVisible(outerClass, addedMethod) || future.inheritsFromLibraryClass(outerClass)) {
for (NodeSource source : sources) {
for (NodeSource source : Iterators.filter(sources, context.getParams().affectionFilter()::test)) {
debug("Affecting file due to local overriding: ", source.getPath());
context.affectNodeSource(source);
}
Expand Down Expand Up @@ -949,7 +969,8 @@ private static void affectNodeSources(DifferentiateContext context, ReferenceID

private static void affectNodeSources(DifferentiateContext context, ReferenceID clsId, String affectReason, boolean forceAffect) {
Set<NodeSource> deletedSources = context.getDelta().getDeletedSources();
for (NodeSource source : context.getGraph().getSources(clsId)) {
Predicate<? super NodeSource> affectionFilter = context.getParams().affectionFilter();
for (NodeSource source : Iterators.filter(context.getGraph().getSources(clsId), affectionFilter::test)) {
if (forceAffect || !context.isCompiled(source) && !deletedSources.contains(source)) {
context.affectNodeSource(source);
debug(affectReason, source.getPath());
Expand All @@ -959,7 +980,7 @@ private static void affectNodeSources(DifferentiateContext context, ReferenceID

private static void affectModule(DifferentiateContext context, Utils utils, JvmModule mod) {
debug("Affecting module ", mod.getName());
for (NodeSource source : utils.getNodeSources(mod.getReferenceID())) {
for (NodeSource source : Iterators.filter(utils.getNodeSources(mod.getReferenceID()), context.getParams().affectionFilter()::test)) {
context.affectNodeSource(source);
debug("Affected source ", source.getPath());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.intellij.testFramework.fixtures.BasePlatformTestCase;
import org.jetbrains.jps.dependency.impl.Containers;
import org.jetbrains.jps.dependency.impl.DependencyGraphImpl;
import org.jetbrains.jps.dependency.impl.DifferentiateParametersBuilder;
import org.jetbrains.jps.dependency.impl.FileSource;
import org.jetbrains.jps.dependency.java.JvmClass;
import org.jetbrains.jps.dependency.serializer.JvmClassTestUtil;
Expand All @@ -31,7 +32,7 @@ public void testPersistentNodeGraph() throws IOException {
delta.associate(jvmClassNode, Arrays.asList(aSrc, bSrc));

// After each round, not after each builder
DifferentiateResult differentiateResult = graph.differentiate(delta, DifferentiateParameters.byDefault());
DifferentiateResult differentiateResult = graph.differentiate(delta, DifferentiateParametersBuilder.withDefaultSettings());
graph.integrate(differentiateResult);

// Check graph
Expand Down

0 comments on commit 8f1258a

Please sign in to comment.