select tests by block

The in memory coverage data store held a map entry for each instruction
within the code under test. This was almost unbelievably stupid and
result in very high memory consumption.

Coverage is tracked by block, which maps 1:many with instructions. The
reason coverage was being held against instruction seems to be because
the block wasn't being tracked when mutants were combined due code
inlining, so only the instruction could be used for targetting.

This change updates MutationDetails to track mutants affecting multiple
blocks, and changes the interface and internals of the coverage store
to be block driven.

For the assertj project this reduces memory consumption from 3G to 1G.
Although a large improvement, this is still too high.

pitest-aggregator/src/main/java/org/pitest/aggregate/ReportAggregator.java

@@ -5,7 +5,6 @@
 import org.pitest.coverage.BlockLocation;
 import org.pitest.coverage.CoverageData;
 import org.pitest.coverage.ReportCoverage;
-import org.pitest.coverage.TestInfo;
 import org.pitest.coverage.analysis.LineMapper;
 import org.pitest.functional.FCollection;
 import org.pitest.mutationtest.ClassMutationResults;
@@ -23,11 +22,8 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.logging.Logger;
@@ -101,29 +97,6 @@ private ReportCoverage calculateCoverage(final CodeSource codeSource) throws Rep
     }
   }
 
-  private Map<TestInfo, Collection<BlockLocation>>  blocksToMap(
-      final Collection<BlockCoverage> coverageData) {
-
-    Map<TestInfo, Collection<BlockLocation>> blockCoverageMap = new HashMap<>();
-
-    for (final BlockCoverage blockData : coverageData) {
-      List<TestInfo> tests = blockData.getTests().stream()
-              .map(toTestInfo(blockData))
-              .collect(Collectors.toList());
-
-      for (TestInfo each : tests) {
-        Collection<BlockLocation> collection = blockCoverageMap.computeIfAbsent(each, k -> new ArrayList<>());
-        collection.add(blockData.getBlock());
-      }
-
-    }
-    return blockCoverageMap;
-  }
-
-  private Function<String, TestInfo> toTestInfo(final BlockCoverage blockData) {
-    return a -> new TestInfo(null, a, 0, Optional.ofNullable(blockData.getBlock().getLocation().getClassName()), blockData.getBlock().getBlock());
-  }
-
   public static Builder builder() {
     return new Builder();
   }

pitest-aggregator/src/test/java/org/pitest/aggregate/DataLoaderTest.java

@@ -36,7 +36,7 @@ public void testLoadMutationSnippet() throws Exception {
         "com.mycompany.SmallScaleOrderedWeightedValueSamplerTest.shouldSucceedWithVariousGapTimestamps(com.mycompany.SmallScaleOrderedWeightedValueSamplerTest)",
         result.getKillingTest().orElse(null));
     assertEquals("Replaced long multiplication with division", result.getDetails().getDescription());
-    assertEquals(27, result.getDetails().getBlock());
+    assertEquals(27, result.getDetails().getFirstBlock());
   }
 
   @Test

pitest-aggregator/src/test/java/org/pitest/aggregate/MutationResultDataLoaderTest.java

@@ -36,13 +36,13 @@ public void testLoadData() throws Exception {
 
     for (final MutationResult result : results) {
       if (result.getDetails().getFirstIndex() == 5) {
-        assertEquals(38, result.getDetails().getBlock());
+        assertEquals(38, result.getDetails().getFirstBlock());
         assertEquals("com.mycompany.OrderedWeightedValueSampler", result.getDetails().getId().getClassName().asJavaName());
         assertEquals(202, result.getDetails().getLineNumber());
         assertFalse(result.getStatus().isDetected());
         assertEquals(DetectionStatus.NO_COVERAGE, result.getStatus());
       } else {
-        assertEquals(27, result.getDetails().getBlock());
+        assertEquals(27, result.getDetails().getFirstBlock());
         assertEquals("com.mycompany.OrderedWeightedValueSampler", result.getDetails().getId().getClassName().asJavaName());
         assertEquals(77, result.getDetails().getLineNumber());
         assertTrue(result.getStatus().isDetected());

pitest-entry/src/main/java/org/pitest/coverage/CoverageData.java

@@ -77,8 +77,8 @@ public void loadBlockDataOnly(Collection<BlockLocation> coverageData) {
 
 
   @Override
-  public Collection<TestInfo> getTestsForInstructionLocation(InstructionLocation location) {
-    return this.blockCoverage.getOrDefault(location.getBlockLocation(), Collections.emptySet());
+  public Collection<TestInfo> getTestsForBlockLocation(BlockLocation location) {
+    return this.blockCoverage.getOrDefault(location, Collections.emptySet());
   }
 
   @Override

pitest-entry/src/main/java/org/pitest/coverage/CoverageDatabase.java

@@ -9,7 +9,7 @@ public interface CoverageDatabase extends ReportCoverage {
 
   Collection<TestInfo> getTestsForClass(ClassName clazz);
 
-  Collection<TestInfo> getTestsForInstructionLocation(InstructionLocation location);
+  Collection<TestInfo> getTestsForBlockLocation(BlockLocation location);
 
   BigInteger getCoverageIdForClass(ClassName clazz);
 

pitest-entry/src/main/java/org/pitest/coverage/NoCoverage.java

@@ -24,7 +24,7 @@ public Collection<TestInfo> getTestsForClass(ClassName clazz) {
     }
 
     @Override
-    public Collection<TestInfo> getTestsForInstructionLocation(InstructionLocation location) {
+    public Collection<TestInfo> getTestsForBlockLocation(BlockLocation location) {
         return Collections.emptyList();
     }
 

pitest-entry/src/main/java/org/pitest/mutationtest/build/DefaultTestPrioritiser.java

@@ -4,15 +4,13 @@
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
-import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 import org.pitest.classinfo.ClassName;
 import org.pitest.coverage.BlockLocation;
 import org.pitest.coverage.CoverageDatabase;
-import org.pitest.coverage.InstructionLocation;
 import org.pitest.coverage.TestInfo;
 import org.pitest.mutationtest.engine.MutationDetails;
-import org.pitest.util.Log;
 
 /**
  * Assigns tests based on line coverage and order them by execution speed with a
@@ -24,9 +22,6 @@
  */
 public class DefaultTestPrioritiser implements TestPrioritiser {
 
-  private static final Logger    LOG                                  = Log
-                                                                          .getLogger();
-
   private static final int       TIME_WEIGHTING_FOR_DIRECT_UNIT_TESTS = 1000;
 
   private final CoverageDatabase coverage;
@@ -41,23 +36,10 @@ public List<TestInfo> assignTests(MutationDetails mutation) {
   }
 
   private Collection<TestInfo> pickTests(MutationDetails mutation) {
-      if (mutation.getId().getIndexes().size() > 1) {
-        HashSet<TestInfo> ret = new HashSet<>();
-        for (int each : mutation.getId().getIndexes()) {
-          Collection<TestInfo> r = this.coverage.getTestsForInstructionLocation(
-              new InstructionLocation(
-                  new BlockLocation(mutation.getId().getLocation(),
-                      mutation.getBlock(), -1, -1), each - 1));
-          if (r != null) {
-            ret.addAll(r);
-          }
-        }
-        return ret;
-      } else {
-        return this.coverage
-            .getTestsForInstructionLocation(new InstructionLocation(new BlockLocation(mutation.getId().getLocation(), mutation.getBlock(), -1, -1),
-                mutation.getId().getFirstIndex() - 1));
-      }
+    return mutation.getBlocks().stream()
+            .map(block -> new BlockLocation(mutation.getId().getLocation(), block, -1, -1))
+            .flatMap(loc -> this.coverage.getTestsForBlockLocation(loc).stream())
+            .collect(Collectors.toCollection(() -> new HashSet<>()));
   }
 
   private List<TestInfo> prioritizeTests(ClassName clazz,

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/InlinedFinallyBlockFilter.java

@@ -42,7 +42,6 @@
 import static org.pitest.bytecode.analysis.OpcodeMatchers.LRETURN;
 import static org.pitest.bytecode.analysis.OpcodeMatchers.RETURN;
 import static org.pitest.functional.FCollection.bucket;
-import static org.pitest.functional.FCollection.map;
 import static org.pitest.functional.FCollection.mapTo;
 import static org.pitest.functional.prelude.Prelude.not;
 import static org.pitest.sequence.Result.result;
@@ -155,19 +154,21 @@ private void checkForInlinedCode(Collection<MutationDetails> mutantsToReturn,
     }
 
     final MutationDetails baseMutation = mutationsInHandlerBlock.get(0);
-    final int firstBlock = baseMutation.getBlock();
+    final int firstBlock = baseMutation.getBlocks().get(0);
 
     // check that we have at least on mutation in a different block
     // to the base one (is this not implied by there being only 1 mutation in
     // the handler ????)
-    final List<Integer> ids = map(similarMutantsOnSameLine, MutationDetails::getBlock);
+    final List<Integer> ids = blocksForMutants(similarMutantsOnSameLine);
     if (ids.stream().anyMatch(not(isEqual(firstBlock)))) {
       mutantsToReturn.add(makeCombinedMutant(similarMutantsOnSameLine));
     } else {
       mutantsToReturn.addAll(similarMutantsOnSameLine);
     }
   }
 
+
+
   private boolean isInFinallyBlock(MutationDetails m) {
     MethodTree method = currentClass.method(m.getId().getLocation()).get();
     List<LabelNode> handlers = method.rawNode().tryCatchBlocks.stream()
@@ -205,12 +206,18 @@ private static MutationDetails makeCombinedMutant(Collection<MutationDetails> va
         .getLocation(), indexes, first.getId().getMutator());
 
     return new MutationDetails(id, first.getFilename(), first.getDescription(),
-        first.getLineNumber(), first.getBlock());
+        first.getLineNumber(), blocksForMutants(value));
   }
 
   private static Function<MutationDetails, LineMutatorPair> toLineMutatorPair() {
     // bucket by combination of mutator and description
     return a -> new LineMutatorPair(a.getLineNumber(), a.getMutator() + a.getDescription());
   }
 
+  private static List<Integer> blocksForMutants(Collection<MutationDetails> mutants) {
+    return mutants.stream()
+            .flatMap(m -> m.getBlocks().stream())
+            .collect(Collectors.toList());
+  }
+
 }

pitest-entry/src/main/java/org/pitest/mutationtest/report/xml/XMLReportListener.java

@@ -88,7 +88,7 @@ private String makeMutationNode(final MutationResult mutation) {
         + makeNode("" + details.getLineNumber(), lineNumber)
         + makeNode(clean(details.getMutator()), mutator)
         + makeNode("" + details.getFirstIndex(), index)
-        + makeNode("" + details.getBlock(), block)
+        + makeNode("" + details.getFirstBlock(), block)
         + makeNodeWhenConditionSatisfied(!fullMutationMatrix,
             createKillingTestDesc(mutation.getKillingTest()), killingTest)
         + makeNodeWhenConditionSatisfied(fullMutationMatrix,

pitest-entry/src/test/java/org/pitest/mutationtest/build/DefaultTestPrioritiserTest.java

@@ -1,5 +1,6 @@
 package org.pitest.mutationtest.build;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.when;
@@ -16,8 +17,8 @@
 import org.mockito.MockitoAnnotations;
 import org.pitest.classinfo.ClassByteArraySource;
 import org.pitest.classinfo.ClassName;
+import org.pitest.coverage.BlockLocation;
 import org.pitest.coverage.CoverageDatabase;
-import org.pitest.coverage.InstructionLocation;
 import org.pitest.coverage.TestInfo;
 import org.pitest.functional.FCollection;
 import java.util.Optional;
@@ -45,7 +46,7 @@ public void setUp() {
   @Test
   public void shouldAssignTestsForRelevantLineToGeneratedMutations() {
     final List<TestInfo> expected = makeTestInfos(0);
-    when(this.coverage.getTestsForInstructionLocation(any(InstructionLocation.class))).thenReturn(
+    when(this.coverage.getTestsForBlockLocation(any(BlockLocation.class))).thenReturn(
         expected);
     final List<TestInfo> actual = this.testee.assignTests(makeMutation("foo"));
     assertEquals(expected, actual);
@@ -54,10 +55,12 @@ public void shouldAssignTestsForRelevantLineToGeneratedMutations() {
   @Test
   public void shouldPrioritiseTestsByExecutionTime() {
     final List<TestInfo> unorderedTests = makeTestInfos(10000, 100, 1000, 1);
-    when(this.coverage.getTestsForInstructionLocation(any(InstructionLocation.class))).thenReturn(
+    when(this.coverage.getTestsForBlockLocation(any(BlockLocation.class))).thenReturn(
         unorderedTests);
     final List<TestInfo> actual = this.testee.assignTests(makeMutation("foo"));
 
+    assertThat(actual.stream().map(toTime())).contains(1, 100, 1000, 10000);
+
     assertEquals(Arrays.asList(1, 100, 1000, 10000),
         FCollection.map(actual, toTime()));
   }
@@ -72,7 +75,7 @@ private List<TestInfo> makeTestInfos(final Integer... times) {
   }
 
   private Function<Integer, TestInfo> timeToTestInfo() {
-    return a -> new TestInfo("foo", "bar", a, Optional.<ClassName> empty(), 0);
+    return a -> new TestInfo("foo", "bar" + a, a, Optional.<ClassName> empty(), 0);
   }
 
   private MutationDetails makeMutation(final String method) {

pitest/src/main/java/org/pitest/coverage/BlockLocation.java

@@ -16,6 +16,7 @@ public BlockLocation(final Location location, final int block,
       final int firstInsnInBlock, final int lastInsnInBlock) {
     this.location = location;
     this.block = block;
+    // what are these for?
     this.firstInsnInBlock = firstInsnInBlock;
     this.lastInsnInBlock = lastInsnInBlock;
   }

pitest/src/main/java/org/pitest/mutationtest/engine/MutationDetails.java

@@ -25,6 +25,8 @@
 import org.pitest.coverage.TestInfo;
 import org.pitest.util.StringUtil;
 
+import static java.util.Collections.singletonList;
+
 /**
  * Captures all data relating to a mutant.
  */
@@ -34,29 +36,34 @@ public final class MutationDetails implements Serializable {
 
   private final MutationIdentifier  id;
   private final String              filename;
-  private final int                 block;
+  private final List<Integer>       blocks;
   private final int                 lineNumber;
   private final String              description;
   private final ArrayList<TestInfo> testsInOrder = new ArrayList<>();
 
   public MutationDetails(final MutationIdentifier id, final String filename,
       final String description, final int lineNumber, final int block) {
+    this(id, filename, description, lineNumber, singletonList(block));
+  }
+
+  public MutationDetails(final MutationIdentifier id, final String filename,
+                         final String description, final int lineNumber, List<Integer> blocks) {
     this.id = id;
     this.description = Objects.requireNonNull(description);
     this.filename = defaultFilenameIfNotSupplied(filename);
     this.lineNumber = lineNumber;
-    this.block = block;
+    this.blocks = blocks;
   }
 
   @Override
   public String toString() {
     return "MutationDetails [id=" + this.id + ", filename=" + this.filename + ", block="
-        + this.block + ", lineNumber=" + this.lineNumber + ", description=" + this.description
+        + this.blocks + ", lineNumber=" + this.lineNumber + ", description=" + this.description
         + ", testsInOrder=" + this.testsInOrder + "]";
   }
 
   public MutationDetails withDescription(String desc) {
-    return new MutationDetails(this.id, this.filename, desc, this.lineNumber, this.block);
+    return new MutationDetails(this.id, this.filename, desc, this.lineNumber, this.blocks);
   }
 
   /**
@@ -168,14 +175,26 @@ public void addTestsInOrder(final Collection<TestInfo> testNames) {
   }
 
   /**
-   * Returns the basic block in which this mutation occurs. See
+   * Returns the basic blocks in which this mutation occurs. See
    * https://github.com/hcoles/pitest/issues/131 for discussion on block
    * coverage
    *
+   * Usually this will be only a single block, but where mutants have
+   * been combined as code has been inlined, the mutant may occupy
+   * more than one block.
+   *
    * @return the block within the method that this mutation is located in
    */
-  public int getBlock() {
-    return this.block;
+  public List<Integer> getBlocks() {
+    return this.blocks;
+  }
+
+  /**
+   * Legacy method to retain interface for report code expecting a single block
+   */
+  @Deprecated
+  public int getFirstBlock() {
+    return this.blocks.get(0);
   }
 
   /**