Skip to content

Commit

Permalink
improve performance for checking if type is subtype
Browse files Browse the repository at this point in the history
  • Loading branch information
Luro02 committed Jun 29, 2024
1 parent 8710f81 commit 41725cc
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import de.firemage.autograder.core.integrated.IntegratedCheck;
import de.firemage.autograder.core.integrated.SpoonUtil;
import de.firemage.autograder.core.integrated.StaticAnalysis;
import spoon.processing.FactoryAccessor;
import spoon.reflect.code.BinaryOperatorKind;
import spoon.reflect.code.CtArrayAccess;
import spoon.reflect.code.CtBinaryOperator;
Expand All @@ -28,7 +27,6 @@
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.declaration.CtParameter;
import spoon.reflect.declaration.CtRecord;
import spoon.reflect.declaration.CtTypeInformation;
import spoon.reflect.declaration.CtVariable;
import spoon.reflect.path.CtRole;
import spoon.reflect.reference.CtArrayTypeReference;
Expand All @@ -52,10 +50,9 @@ public class ConcreteCollectionCheck extends IntegratedCheck {
java.util.EnumSet.class
);

private <T extends CtTypeInformation & FactoryAccessor> boolean isConcreteCollectionType(T ctType) {
return Stream.of(java.util.Collection.class, java.util.Map.class)
.map(ty -> ctType.getFactory().Type().createReference(ty, false))
.anyMatch(ctType::isSubtypeOf) && !ctType.isInterface();
private boolean isConcreteCollectionType(CtTypeReference<?> ctType) {
return !ctType.isInterface() && Stream.of(java.util.Collection.class, java.util.Map.class)
.anyMatch(ctClass -> SpoonUtil.isSubtypeOf(ctType, ctClass));
}

private boolean isAllowedType(CtTypeReference<?> ctTypeReference) {
Expand Down Expand Up @@ -116,6 +113,7 @@ private boolean isInAllowedContext(CtTypeReference<?> ctTypeReference) {
if (ctFieldRead != null) {
CtFieldReference<?> ctFieldReference = ctFieldRead.getVariable();
return ctFieldReference.getDeclaringType().equals(ctTypeReference)
// TODO: use SpoonUtil.isTypeEqualTo
&& ctFieldReference.getType()
.equals(ctTypeReference.getFactory().Type().createReference(Class.class, false));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.stream.Collectors;

public class IntegratedAnalysis {
private static final boolean IS_IN_DEBUG_MODE = SpoonUtil.isInJunitTest();
private static final String INITIAL_INTEGRITY_CHECK_NAME = "StaticAnalysis-Constructor";
private static final boolean ENSURE_NO_ORPHANS = false;
private static final boolean ENSURE_NO_MODEL_CHANGES = false;
Expand All @@ -39,10 +40,14 @@ public IntegratedAnalysis(UploadedFile file) {
this.file = file;

// create a copy of the model to later check if a check changed the model
this.originalModel = file.copy().getModel().getModel();
if (IS_IN_DEBUG_MODE || ENSURE_NO_MODEL_CHANGES || ENSURE_NO_ORPHANS) {
this.originalModel = file.copy().getModel().getModel();
} else {
this.originalModel = null;
}

this.staticAnalysis = new StaticAnalysis(file.getModel(), file.getCompilationResult());
if (this.originalModel == this.staticAnalysis.getModel()) {
if (IS_IN_DEBUG_MODE && this.originalModel == this.staticAnalysis.getModel()) {
throw new IllegalStateException("The model was not cloned");
}

Expand Down Expand Up @@ -79,11 +84,11 @@ public void lint(Iterable<? extends IntegratedCheck> checks, Consumer<? super Li
private void assertModelIntegrity(String checkName) {
CtModel linterModel = this.staticAnalysis.getModel();

if ((ENSURE_NO_MODEL_CHANGES || SpoonUtil.isInJunitTest())) {
if (ENSURE_NO_MODEL_CHANGES || IS_IN_DEBUG_MODE) {
Collection<ParentChecker.InvalidElement> invalidElements = ParentChecker.checkConsistency(linterModel.getUnnamedModule());

if (checkName.equals(INITIAL_INTEGRITY_CHECK_NAME)) {
alreadyInvalidElements.addAll(invalidElements.stream().map(ParentChecker.InvalidElement::element).toList());
invalidElements.stream().map(ParentChecker.InvalidElement::element).forEach(alreadyInvalidElements::add);
}

invalidElements.removeIf(elem -> alreadyInvalidElements.contains(elem.element()));
Expand All @@ -96,11 +101,11 @@ private void assertModelIntegrity(String checkName) {
}
}

if ((ENSURE_NO_MODEL_CHANGES || SpoonUtil.isInJunitTest()) && !this.isModelEqualTo(this.originalModel, linterModel)) {
if ((ENSURE_NO_MODEL_CHANGES || IS_IN_DEBUG_MODE) && !this.isModelEqualTo(this.originalModel, linterModel)) {
throw new IllegalStateException("The model was changed by the check: %s".formatted(checkName));
}

if (ENSURE_NO_ORPHANS || SpoonUtil.isInJunitTest()) {
if (ENSURE_NO_ORPHANS || IS_IN_DEBUG_MODE) {
List<CtElement> orphans = findOrphans(linterModel);
if (!orphans.isEmpty()) {
throw new IllegalStateException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ public static <T, R> CtLiteral<R> castLiteral(CtTypeReference<R> type, CtLiteral
CtTypeReference<?> targetType = type.unbox();
if (targetType.isPrimitive()) {
// the FoldUtils.convert method only works for Number -> Number conversions
if (targetType.box().isSubtypeOf(type.getFactory().createCtTypeReference(Number.class))) {
if (SpoonUtil.isSubtypeOf(targetType.box(), Number.class)) {
// for instances of Number, one can use the convert method:
if (literal.getValue() instanceof Number number) {
result.setValue(FoldUtils.convert(type, number));
Expand Down Expand Up @@ -960,8 +960,19 @@ public static boolean isTypeEqualTo(CtTypeReference<?> ctType, CtTypeReference<?

public static boolean isSubtypeOf(CtTypeReference<?> ctTypeReference, Class<?> expected) {
// NOTE: calling isSubtypeOf on CtTypeParameterReference will result in a crash
return !(ctTypeReference instanceof CtTypeParameterReference)
&& ctTypeReference.isSubtypeOf(ctTypeReference.getFactory().Type().get(expected).getReference());
CtType<?> expectedType = ctTypeReference.getFactory().Type().get(expected);

boolean result = !(ctTypeReference instanceof CtTypeParameterReference)
&& UsesFinder.isSubtypeOf(ctTypeReference.getTypeDeclaration(), expectedType);

if (SpoonUtil.isInJunitTest() && result != ctTypeReference.isSubtypeOf(expectedType.getReference())) {
throw new IllegalStateException("UsesFinder.isSubtypeOf(%s, %s) does not match spoon implementation".formatted(
ctTypeReference.getQualifiedName(),
expectedType.getQualifiedName()
));
}

return result;
}

public static boolean isMainMethod(CtMethod<?> method) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,18 @@ public static CtElementStream<CtTypeReference<?>> typeUses(CtType<?> type) {
return CtElementStream.of(UsesFinder.getFor(type).scanner.typeUses.getOrDefault(type, List.of())).assumeElementType();
}

private static CtElementStream<CtType<?>> supertypesOf(CtType<?> type) {
return CtElementStream.of(UsesFinder.getFor(type).scanner.supertypes.getOrDefault(type, new LinkedHashSet<>())).assumeElementType();
}

public static boolean isSubtypeOf(CtType<?> parentType, CtType<?> potentialSubtype) {
return parentType == potentialSubtype
// this only caches classes declared in the model
|| UsesFinder.supertypesOf(parentType).anyMatch(type -> potentialSubtype == type)
// that is why we have to call the original method
|| parentType.isSubtypeOf(potentialSubtype.getReference());
}

public static CtElementStream<CtType<?>> subtypesOf(CtType<?> type, boolean includeSelf) {
Stream<CtType<?>> selfStream = includeSelf ? Stream.of(type) : Stream.empty();
return CtElementStream.concat(
Expand Down Expand Up @@ -150,6 +162,7 @@ private static class UsesScanner extends CtScanner {
private final Map<CtExecutable, List<CtElement>> executableUses = new IdentityHashMap<>();
private final Map<CtType, List<CtTypeReference>> typeUses = new IdentityHashMap<>();
private final Map<CtType, SequencedSet<CtType>> subtypes = new IdentityHashMap<>();
private final Map<CtType, SequencedSet<CtType>> supertypes = new IdentityHashMap<>();

// Caches the current instanceof pattern variables, since Spoon doesn't track them yet
// We are conservative: A pattern introduces a variable until the end of the current block
Expand Down Expand Up @@ -308,6 +321,7 @@ private void recordCtType(CtType<?> ctType) {
CtTypeReference<?> superType = ctType.getSuperclass();
while (superType != null && superType.getTypeDeclaration() != null) {
this.subtypes.computeIfAbsent(superType.getTypeDeclaration(), k -> new LinkedHashSet<>()).add(ctType);
this.supertypes.computeIfAbsent(ctType, k -> new LinkedHashSet<>()).add(superType.getTypeDeclaration());
superType = superType.getSuperclass();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package de.firemage.autograder.core.integrated.evaluator;

import de.firemage.autograder.core.integrated.SpoonUtil;
import de.firemage.autograder.core.integrated.UsesFinder;
import de.firemage.autograder.core.integrated.evaluator.fold.ApplyCasts;
import de.firemage.autograder.core.integrated.evaluator.fold.ChainedFold;
import de.firemage.autograder.core.integrated.evaluator.fold.DeduplicateOperatorApplication;
Expand Down Expand Up @@ -71,6 +72,7 @@ private static <T> CtExpression<T> createExpression(String expression, String ar
environment.setIgnoreSyntaxErrors(false);

CtModel ctModel = launcher.buildModel();
UsesFinder.buildFor(ctModel);

CtMethod<?> ctMethod = new ArrayList<>(ctModel.getAllTypes()).get(0).getMethodsByName("t").get(0);
CtAbstractInvocation<?> ctInvocation = (CtInvocation<?>) ctMethod.getBody().getStatements().get(0);
Expand Down

0 comments on commit 41725cc

Please sign in to comment.