Merge pull request #1167 from hcoles/feature/static_analysis_performance

Improve performance of filters
hcoles · Mar 16, 2023 · c280d40 · c280d40
2 parents aef8c66 + 51a88d5
commit c280d40
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 119 deletions.
diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/RegionIndex.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/RegionIndex.java
@@ -0,0 +1,19 @@
+package org.pitest.mutationtest.build.intercept;
+
+public class RegionIndex {
+    private final int start;
+    private final int end;
+
+    public RegionIndex(int start, int end) {
+        this.start = start;
+        this.end = end;
+    }
+
+    public int start() {
+        return start;
+    }
+
+    public int end() {
+        return end;
+    }
+}
diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/RegionInterceptor.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/RegionInterceptor.java
@@ -19,7 +19,7 @@
  */
 public abstract class RegionInterceptor implements MutationInterceptor {
     private ClassTree currentClass;
-    private Map<MethodTree, List<Region>> cache;
+    private Map<MethodTree, List<RegionIndex>> cache;
 
     @Override
     public InterceptorType type() {
@@ -40,23 +40,30 @@ public Collection<MutationDetails> intercept(
                 .collect(Collectors.toList());
     }
 
+
     private Predicate<MutationDetails> buildPredicate() {
         return a -> {
             final int instruction = a.getInstructionIndex();
             final MethodTree method = this.currentClass.method(a.getId().getLocation()).get();
 
-            List<Region> regions = cache.computeIfAbsent(method, this::computeRegions);
-
+            List<RegionIndex> regions = cache.computeIfAbsent(method, this::computeRegionIndex);
             return regions.stream()
-                    .anyMatch(r -> instruction >= method.instructions().indexOf(r.start) && instruction <= method.instructions().indexOf(r.end));
+                    .anyMatch(r -> r.start() <= instruction && r.end() >= instruction);
         };
     }
 
+    private List<RegionIndex> computeRegionIndex(MethodTree method) {
+        return computeRegions(method).stream()
+                .map(r -> new RegionIndex(method.instructions().indexOf(r.start), method.instructions().indexOf(r.end)))
+                .collect(Collectors.toList());
+    }
+
     protected abstract List<Region> computeRegions(MethodTree method);
 
     @Override
     public void end() {
         currentClass = null;
         cache = null;
     }
+
 }
diff --git a/...ntry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/AssertFilter.java b/...ntry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/AssertFilter.java
@@ -54,10 +54,9 @@ private static Match<AbstractInsnNode> store(SlotWrite<AbstractInsnNode> slot) {
 
     protected List<Region> computeRegions(MethodTree method) {
         Context context = Context.start();
-        List<Region> regions = ASSERT_GET.contextMatches(method.instructions(), context).stream()
+        return ASSERT_GET.contextMatches(method.instructions(), context).stream()
                 .map(c -> new Region(c.retrieve(START.read()).get(), c.retrieve(END.read()).get()))
                 .collect(Collectors.toList());
-        return regions;
     }
 
 }
diff --git a/...in/java/org/pitest/mutationtest/build/intercept/javafeatures/ImplicitNullCheckFilter.java b/...in/java/org/pitest/mutationtest/build/intercept/javafeatures/ImplicitNullCheckFilter.java
@@ -2,41 +2,37 @@
 
 import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction;
 import static org.pitest.bytecode.analysis.InstructionMatchers.isA;
-import static org.pitest.bytecode.analysis.InstructionMatchers.isInstruction;
 import static org.pitest.bytecode.analysis.InstructionMatchers.methodCallTo;
 import static org.pitest.bytecode.analysis.InstructionMatchers.notAnInstruction;
 import static org.pitest.bytecode.analysis.OpcodeMatchers.POP;
+import static org.pitest.sequence.Result.result;
 
-import java.util.Collection;
-import java.util.function.Predicate;
+import java.util.List;
+import java.util.stream.Collectors;
 
 import org.objectweb.asm.tree.AbstractInsnNode;
 import org.objectweb.asm.tree.LabelNode;
-import org.objectweb.asm.tree.MethodInsnNode;
-import org.pitest.bytecode.analysis.ClassTree;
 import org.pitest.bytecode.analysis.MethodTree;
 import org.pitest.classinfo.ClassName;
-import org.pitest.functional.FCollection;
-import org.pitest.functional.prelude.Prelude;
-import org.pitest.mutationtest.build.InterceptorType;
-import org.pitest.mutationtest.build.MutationInterceptor;
-import org.pitest.mutationtest.engine.Mutater;
-import org.pitest.mutationtest.engine.MutationDetails;
+import org.pitest.mutationtest.build.intercept.Region;
+import org.pitest.mutationtest.build.intercept.RegionInterceptor;
 import org.pitest.sequence.Context;
+import org.pitest.sequence.Match;
 import org.pitest.sequence.QueryParams;
 import org.pitest.sequence.QueryStart;
 import org.pitest.sequence.SequenceMatcher;
 import org.pitest.sequence.Slot;
+import org.pitest.sequence.SlotWrite;
 
-public class ImplicitNullCheckFilter implements MutationInterceptor {
+public class ImplicitNullCheckFilter extends RegionInterceptor {
 
   private static final boolean DEBUG = false;
 
   private static final Slot<AbstractInsnNode> MUTATED_INSTRUCTION = Slot.create(AbstractInsnNode.class);
 
   static final SequenceMatcher<AbstractInsnNode> GET_CLASS_NULL_CHECK = QueryStart
       .any(AbstractInsnNode.class)
-      .then(methodCallTo(ClassName.fromClass(Object.class), "getClass").and(isInstruction(MUTATED_INSTRUCTION.read())))
+      .then(aGetClassCall().and(store(MUTATED_INSTRUCTION.write())))
       .then(POP) // immediate discard
       .then(isA(LabelNode.class).negate()) // use presence of a label to indicate this was a programmer call to getClass
       .zeroOrMore(QueryStart.match(anyInstruction()))
@@ -45,47 +41,20 @@ public class ImplicitNullCheckFilter implements MutationInterceptor {
           .withDebug(DEBUG)
           );
 
-
-  private ClassTree currentClass;
-
-  @Override
-  public InterceptorType type() {
-    return InterceptorType.FILTER;
-  }
-
-  @Override
-  public void begin(ClassTree clazz) {
-    this.currentClass = clazz;
+  private static Match<AbstractInsnNode> aGetClassCall() {
+    return methodCallTo(ClassName.fromClass(Object.class), "getClass");
   }
 
-  @Override
-  public Collection<MutationDetails> intercept(
-      Collection<MutationDetails> mutations, Mutater m) {
-    return FCollection.filter(mutations, Prelude.not(isAnImplicitNullCheck()));
-  }
-
-  private Predicate<MutationDetails> isAnImplicitNullCheck() {
-    return a -> {
-      final int instruction = a.getInstructionIndex();
-      final MethodTree method = ImplicitNullCheckFilter.this.currentClass.method(a.getId().getLocation())
-          .get();
-
-      final AbstractInsnNode mutatedInstruction = method.instruction(instruction);
-
-      // performance hack
-      if (!(mutatedInstruction instanceof MethodInsnNode)) {
-        return false;
-      }
-
-      Context context = Context.start(DEBUG);
-      context = context.store(MUTATED_INSTRUCTION.write(), mutatedInstruction);
-      return GET_CLASS_NULL_CHECK.matches(method.instructions(), context);
-    };
+  private static Match<AbstractInsnNode> store(SlotWrite<AbstractInsnNode> slot) {
+    return (c, n) -> result(true, c.store(slot, n));
   }
 
   @Override
-  public void end() {
-    this.currentClass = null;
+  protected List<Region> computeRegions(MethodTree method) {
+    Context context = Context.start();
+    return GET_CLASS_NULL_CHECK.contextMatches(method.instructions(), context).stream()
+            .map(c -> new Region(c.retrieve(MUTATED_INSTRUCTION.read()).get(), c.retrieve(MUTATED_INSTRUCTION.read()).get()))
+            .collect(Collectors.toList());
   }
 
 }
diff --git a/.../org/pitest/mutationtest/build/intercept/javafeatures/MethodReferenceNullCheckFilter.java b/.../org/pitest/mutationtest/build/intercept/javafeatures/MethodReferenceNullCheckFilter.java
@@ -1,47 +1,41 @@
 package org.pitest.mutationtest.build.intercept.javafeatures;
 
-import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction;
-import static org.pitest.bytecode.analysis.InstructionMatchers.isInstruction;
-import static org.pitest.bytecode.analysis.InstructionMatchers.methodCallTo;
-import static org.pitest.bytecode.analysis.InstructionMatchers.notAnInstruction;
-import static org.pitest.bytecode.analysis.OpcodeMatchers.INVOKEDYNAMIC;
-import static org.pitest.bytecode.analysis.OpcodeMatchers.POP;
-
-import java.util.Collection;
-import java.util.Objects;
-import java.util.function.Predicate;
-
 import org.objectweb.asm.tree.AbstractInsnNode;
-import org.objectweb.asm.tree.MethodInsnNode;
-import org.pitest.bytecode.analysis.ClassTree;
-import org.pitest.bytecode.analysis.MethodMatchers;
 import org.pitest.bytecode.analysis.MethodTree;
 import org.pitest.classinfo.ClassName;
-import org.pitest.functional.FCollection;
-import org.pitest.functional.prelude.Prelude;
-import org.pitest.mutationtest.build.InterceptorType;
-import org.pitest.mutationtest.build.MutationInterceptor;
-import org.pitest.mutationtest.engine.Mutater;
-import org.pitest.mutationtest.engine.MutationDetails;
+import org.pitest.mutationtest.build.intercept.Region;
+import org.pitest.mutationtest.build.intercept.RegionInterceptor;
 import org.pitest.sequence.Context;
+import org.pitest.sequence.Match;
 import org.pitest.sequence.QueryParams;
 import org.pitest.sequence.QueryStart;
 import org.pitest.sequence.SequenceMatcher;
 import org.pitest.sequence.Slot;
+import org.pitest.sequence.SlotWrite;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction;
+import static org.pitest.bytecode.analysis.InstructionMatchers.methodCallTo;
+import static org.pitest.bytecode.analysis.InstructionMatchers.notAnInstruction;
+import static org.pitest.bytecode.analysis.OpcodeMatchers.INVOKEDYNAMIC;
+import static org.pitest.bytecode.analysis.OpcodeMatchers.POP;
+import static org.pitest.sequence.Result.result;
 
 /**
  * Filters out the calls to Objects.requireNotNull the compiler inserts when using method references.
  *
  */
-public class MethodReferenceNullCheckFilter implements MutationInterceptor {
+public class MethodReferenceNullCheckFilter extends RegionInterceptor {
 
   private static final boolean DEBUG = false;
-
   private static final Slot<AbstractInsnNode> MUTATED_INSTRUCTION = Slot.create(AbstractInsnNode.class);
 
   static final SequenceMatcher<AbstractInsnNode> NULL_CHECK = QueryStart
       .any(AbstractInsnNode.class)
-      .then(methodCallTo(ClassName.fromClass(Objects.class), "requireNonNull").and(isInstruction(MUTATED_INSTRUCTION.read())))
+      .then(requireNonNullCall().and(store(MUTATED_INSTRUCTION.write())))
       .then(POP)
       .then(INVOKEDYNAMIC)
       .zeroOrMore(QueryStart.match(anyInstruction()))
@@ -50,47 +44,20 @@ public class MethodReferenceNullCheckFilter implements MutationInterceptor {
           .withDebug(DEBUG)
           );
 
-
-  private ClassTree currentClass;
-
-  @Override
-  public InterceptorType type() {
-    return InterceptorType.FILTER;
+  private static Match<AbstractInsnNode> requireNonNullCall() {
+    return methodCallTo(ClassName.fromClass(Objects.class), "requireNonNull");
   }
 
   @Override
-  public void begin(ClassTree clazz) {
-    this.currentClass = clazz;
+  protected List<Region> computeRegions(MethodTree method) {
+    Context context = Context.start();
+    return NULL_CHECK.contextMatches(method.instructions(), context).stream()
+            .map(c -> new Region(c.retrieve(MUTATED_INSTRUCTION.read()).get(), c.retrieve(MUTATED_INSTRUCTION.read()).get()))
+            .collect(Collectors.toList());
   }
 
-  @Override
-  public Collection<MutationDetails> intercept(
-      Collection<MutationDetails> mutations, Mutater m) {
-    return FCollection.filter(mutations, Prelude.not(isAnImplicitNullCheck()));
-  }
-
-  private Predicate<MutationDetails> isAnImplicitNullCheck() {
-    return a -> {
-      final int instruction = a.getInstructionIndex();
-      final MethodTree method = MethodReferenceNullCheckFilter.this.currentClass.methods().stream()
-          .filter(MethodMatchers.forLocation(a.getId().getLocation()))
-          .findFirst()
-          .get();
-
-      final AbstractInsnNode mutatedInstruction = method.instruction(instruction);
-      // performance hack
-      if (!(mutatedInstruction instanceof MethodInsnNode)) {
-        return false;
-      }
-      Context context = Context.start(DEBUG);
-      context = context.store(MUTATED_INSTRUCTION.write(), mutatedInstruction);
-      return NULL_CHECK.matches(method.instructions(), context);
-    };
-  }
-
-  @Override
-  public void end() {
-    this.currentClass = null;
+  private static Match<AbstractInsnNode> store(SlotWrite<AbstractInsnNode> slot) {
+    return (c, n) -> result(true, c.store(slot, n));
   }
 
 }
diff --git a/...rc/main/java/org/pitest/mutationtest/build/intercept/javafeatures/StringSwitchFilter.java b/...rc/main/java/org/pitest/mutationtest/build/intercept/javafeatures/StringSwitchFilter.java
@@ -80,10 +80,9 @@ private static SequenceQuery<AbstractInsnNode> switchBranchSequence() {
 
     protected List<Region> computeRegions(MethodTree method) {
         Context context = Context.start();
-        List<Region> regions = STRING_SWITCH.contextMatches(method.instructions(), context).stream()
+        return STRING_SWITCH.contextMatches(method.instructions(), context).stream()
                 .map(c -> new Region(c.retrieve(START.read()).get(), c.retrieve(END.read()).get()))
                 .collect(Collectors.toList());
-        return regions;
     }
 
 }
diff --git a/...ain/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilter.java b/...ain/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilter.java
@@ -214,10 +214,9 @@ protected List<Region> computeRegions(MethodTree method) {
 
     Context context = Context.start(DEBUG);
     context = context.store(HANDLERS.write(), handlers);
-    List<Region> regions = TRY_WITH_RESOURCES.contextMatches(method.instructions(), context).stream()
+    return TRY_WITH_RESOURCES.contextMatches(method.instructions(), context).stream()
             .map(c -> new Region(c.retrieve(START.read()).get(), c.retrieve(END.read()).get()))
             .collect(Collectors.toList());
-    return regions;
   }
 
 

diff --git a/...ava/org/pitest/mutationtest/build/intercept/javafeatures/ImplicitNullCheckFilterTest.java b/...ava/org/pitest/mutationtest/build/intercept/javafeatures/ImplicitNullCheckFilterTest.java
@@ -24,7 +24,7 @@ public void shouldFilterMutantsThatAlterGetClassCallsInALambda() {
   }
 
   @Test
-  public void flteraMutantsThatAlterGetClassInImplicitNullCheck() {
+  public void fltersMutantsThatAlterGetClassInImplicitNullCheck() {
     this.verifier.assertFiltersNMutationFromSample(1, "ImplicitNullCheck");
   }
-Original file line number
+Diff line change
@@ Expand Up @@
       }
       @Test
-      public void flteraMutantsThatAlterGetClassInImplicitNullCheck() {
+      public void fltersMutantsThatAlterGetClassInImplicitNullCheck() {
         this.verifier.assertFiltersNMutationFromSample(1, "ImplicitNullCheck");
       }
@@ Expand Down @@