Merge pull request #545 from gmu-swe/coverage-optimization

Enhance coverage collection performance by caching probe arrays local…

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

@@ -0,0 +1,7 @@
+package org.pitest.coverage;
+
+public class AlreadyInstrumentedException extends IllegalArgumentException {
+  public AlreadyInstrumentedException(String msg) {
+    super(msg);
+  }
+}

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

@@ -18,11 +18,12 @@
 package org.pitest.coverage;
 
 import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
 import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
 import org.pitest.classinfo.BridgeMethodFilter;
 import org.pitest.classinfo.MethodFilteringAdapter;
 import org.pitest.coverage.analysis.CoverageAnalyser;
-
 import sun.pitest.CodeCoverageStore;
 
 /**
@@ -31,7 +32,14 @@
 public class CoverageClassVisitor extends MethodFilteringAdapter {
   private final int classId;
 
-  private int       probeCount = 0;
+  /**
+   * Probe count starts at 1, because probe "0" indicates that the class was hit
+   * by this test.
+   */
+  private int       probeCount = 1;
+
+  private String    className;
+  private boolean   foundClinit;
 
   public CoverageClassVisitor(final int classId, final ClassWriter writer) {
     super(writer, BridgeMethodFilter.INSTANCE);
@@ -42,19 +50,82 @@ public void registerProbes(final int number) {
     this.probeCount = this.probeCount + number;
   }
 
+  @Override
+  public void visit(int version, int access, String name, String signature,
+      String superName, String[] interfaces) {
+    super.visit(version, access, name, signature, superName, interfaces);
+    this.className = name;
+  }
+
+
   @Override
   public MethodVisitor visitMethodIfRequired(final int access,
       final String name, final String desc, final String signature,
       final String[] exceptions, final MethodVisitor methodVisitor) {
 
+    if (name.equals("<clinit>")) {
+      foundClinit = true;
+    }
+
     return new CoverageAnalyser(this, this.classId, this.probeCount,
         methodVisitor, access, name, desc, signature, exceptions);
 
   }
 
+  @Override
+  public FieldVisitor visitField(int access, String name, String descriptor,
+      String signature, Object value) {
+
+    /*
+    If this class was already instrumented, then do not do it again!
+     */
+    if (name.equals(CodeCoverageStore.PROBE_FIELD_NAME)) {
+      throw new AlreadyInstrumentedException("Class " + getClassName()
+          + " already has coverage instrumentation, but asked to do it again!");
+    }
+    return super.visitField(access, name, descriptor, signature, value);
+  }
+
   @Override
   public void visitEnd() {
-    CodeCoverageStore.registerClassProbes(this.classId, this.probeCount);
+    addCoverageProbeField();
   }
 
+  private void addCoverageProbeField() {
+
+    super.visitField(Opcodes.ACC_STATIC | Opcodes.ACC_FINAL | Opcodes.ACC_PUBLIC
+            | Opcodes.ACC_SYNTHETIC, CodeCoverageStore.PROBE_FIELD_NAME, "[Z", null,
+        null);
+
+    super.visitField(Opcodes.ACC_STATIC | Opcodes.ACC_FINAL | Opcodes.ACC_PUBLIC
+            | Opcodes.ACC_SYNTHETIC, CodeCoverageStore.PROBE_LENGTH_FIELD_NAME, "I",
+        null, this.probeCount + 1);
+
+    //If there is no <clinit>, then generate one that sets the probe field directly
+    if (!foundClinit) {
+      MethodVisitor clinitMv = this.cv
+          .visitMethod(Opcodes.ACC_STATIC, "<clinit>", "()V", null, null);
+      clinitMv.visitCode();
+
+      clinitMv.visitIntInsn(
+          (this.classId <= Byte.MAX_VALUE ? Opcodes.BIPUSH : Opcodes.SIPUSH),
+          this.classId);
+      clinitMv.visitIntInsn(
+          (1 + this.probeCount <= Byte.MAX_VALUE ? Opcodes.BIPUSH : Opcodes.SIPUSH),
+          1 + this.probeCount);
+      clinitMv
+          .visitMethodInsn(Opcodes.INVOKESTATIC, CodeCoverageStore.CLASS_NAME,
+              "getOrRegisterClassProbes", "(II)[Z", false);
+
+      clinitMv.visitFieldInsn(Opcodes.PUTSTATIC, className,
+          CodeCoverageStore.PROBE_FIELD_NAME, "[Z");
+      clinitMv.visitInsn(Opcodes.RETURN);
+      clinitMv.visitMaxs(0, 0);
+      clinitMv.visitEnd();
+    }
+  }
+
+  public String getClassName() {
+    return className;
+  }
 }

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

@@ -46,14 +46,25 @@ public byte[] transform(final ClassLoader loader, final String className,
   private byte[] transformBytes(final ClassLoader loader,
       final String className, final byte[] classfileBuffer) {
     final ClassReader reader = new ClassReader(classfileBuffer);
+
+    /*
+    Make sure that this class has not already been instrumented for coverage
+    generation. It is possible that some other bytecode instrumentation tool
+    would try to redefine a class (that we already added coverage tracking to),
+    in which case we will just allow that previous coverage tracking to stand.
+     */
     final ClassWriter writer = new ComputeClassWriter(
         new ClassloaderByteArraySource(loader), this.computeCache,
         FrameOptions.pickFlags(classfileBuffer));
 
     final int id = CodeCoverageStore.registerClass(className);
-    reader.accept(new CoverageClassVisitor(id, writer),
-        ClassReader.EXPAND_FRAMES);
-    return writer.toByteArray();
+    try {
+      reader.accept(new CoverageClassVisitor(id, writer),
+          ClassReader.EXPAND_FRAMES);
+      return writer.toByteArray();
+    } catch (AlreadyInstrumentedException ex) {
+      return null;
+    }
   }
 
   private boolean shouldInclude(final String className) {

pitest/src/main/java/org/pitest/coverage/analysis/AbstractCoverageStrategy.java

@@ -12,30 +12,23 @@
 
 abstract class AbstractCoverageStrategy extends AdviceAdapter {
 
+  protected final String           className;
   protected final MethodVisitor    methodVisitor;
   protected final int              classId;
   protected final int              probeOffset;
   protected final List<Block>      blocks;
 
   private final InstructionCounter counter;
 
-  /**
-   * label to mark start of try finally block that is added to each method
-   */
-  private final Label              before     = new Label();
-
-  /**
-   * label to mark handler block of try finally
-   */
-  private final Label              handler    = new Label();
-
   protected int                    probeCount = 0;
 
   AbstractCoverageStrategy(List<Block> blocks, InstructionCounter counter,
       final int classId, final MethodVisitor writer, final int access,
-      final String name, final String desc, final int probeOffset) {
+      final String className, final String name, final String desc, final int probeOffset) {
     super(ASMVersion.ASM_VERSION, writer, access, name, desc);
 
+
+    this.className = className;
     this.methodVisitor = writer;
     this.classId = classId;
     this.counter = counter;
@@ -54,22 +47,6 @@ public void visitCode() {
     super.visitCode();
 
     prepare();
-
-    this.mv.visitLabel(this.before);
-  }
-
-  @Override
-  public void visitMaxs(final int maxStack, final int maxLocals) {
-
-    this.mv.visitTryCatchBlock(this.before, this.handler, this.handler, null);
-    this.mv.visitLabel(this.handler);
-
-    generateProbeReportCode();
-
-    this.mv.visitInsn(ATHROW);
-
-    // values actually unimportant as we're using compute max
-    this.mv.visitMaxs(maxStack, this.nextLocal);
   }
 
   @Override

pitest/src/main/java/org/pitest/coverage/analysis/ArrayProbeCoverageMethodVisitor.java

@@ -23,65 +23,100 @@
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.Type;
 import org.pitest.mutationtest.engine.gregor.analysis.InstructionCounter;
-
 import sun.pitest.CodeCoverageStore;
 
 /**
- * Instruments a method adding probes at each line. The strategy requires the
- * compiler to be configured to add line number debug information.
  *
- * Probes are implemented by adding an array to each method. Lines hits are
- * registered by a write to this local array. Each method exit point is then
- * augmented with a call that passes this array to the coverage store class that
- * handles communication of this data back to the parent process on the
- * completion of each test.
+ * Instruments a method adding probes at each block.
+ *
+ * Probes are implemented by adding an array to each class. Block hits are
+ * registered by a write to this local array. The array is registered upon class
+ * initialization with the CodeCoverageStore, and all methods in the same class
+ * share the same array. The coverage store class reads this array at the end of
+ * the test and handles communication of this data back to the parent process.
+ *
+ * The old approach was to allocate an array in *each* invocation of each method,
+ * and merge this in to a global array, which could get flushed between test runs.
+ * The approach implemented here requires far fewer allocations and is faster, plus
+ * it's better from a concurrency perspective (no locking needed except when first
+ * initializing the coverage probe array).
+ *
+ *
+ * Here's a source-level example of the instrumentation result:
  *
- * All methods are wrapped in a try finally block to ensure that coverage data
- * is sent in the event of a runtime exception.
+ * public class Foo {
+ *   public static final int $$pitCoverageProbeSize = 10; //however many blocks there are + 1
+ *   public static final byte[] $$pitCoverageProbes = CodeCoverageStore.getOrRegisterClassProbes(thisClassID,$$pitCoverageProbeSize);
+ *
+ *   private void bar(){
+ *     byte[] localRefToProbes = $$pitCoverageProbes;
+ *     //line of code
+ *     localRefToProbes[1] = 1; //assuming above line was probe 1
+ *   }
+ *
+ * }
+ *
+ * CodeCoverageStore maintains a reference to all of these $$pitCoverageProbes arrays
+ * and empties them out between each test.
  *
- * Creating a new array on each method entry is not cheap - other coverage
- * systems add a static field used across all methods. We must clear down all
- * coverage history for each test however. Resetting static fields in all loaded
- * classes would be messy to implement - it may or may not be faster than the
- * current approach.
  */
 public class ArrayProbeCoverageMethodVisitor extends AbstractCoverageStrategy {
 
-  private int probeHitArrayLocal;
+  private int           probeHitArrayLocal;
 
   public ArrayProbeCoverageMethodVisitor(List<Block> blocks,
       InstructionCounter counter, final int classId,
-      final MethodVisitor writer, final int access, final String name,
+      final MethodVisitor writer, final int access, final String className, final String name,
       final String desc, final int probeOffset) {
-    super(blocks, counter, classId, writer, access, name, desc, probeOffset);
+    super(blocks, counter, classId, writer, access, className, name, desc, probeOffset);
+  }
+
+  @Override
+  public void visitMethodInsn(int opcode, String owner, String name,
+      String desc, boolean itf) {
+
+    super.visitMethodInsn(opcode, owner, name, desc, itf);
+  }
+
+  @Override
+  public void visitFieldInsn(int opcode, String owner, String name,
+      String desc) {
+    super.visitFieldInsn(opcode, owner, name, desc);
   }
 
   @Override
   void prepare() {
+    if (getName().equals("<clinit>")) {
+        this.mv.visitIntInsn(Opcodes.SIPUSH, this.classId);
+        this.mv.visitFieldInsn(Opcodes.GETSTATIC, this.className, CodeCoverageStore.PROBE_LENGTH_FIELD_NAME,"I");
+        this.mv
+            .visitMethodInsn(Opcodes.INVOKESTATIC, CodeCoverageStore.CLASS_NAME,
+                "getOrRegisterClassProbes", "(II)[Z", false);
+        this.mv.visitFieldInsn(Opcodes.PUTSTATIC, className,
+            CodeCoverageStore.PROBE_FIELD_NAME, "[Z");
+    }
     this.probeHitArrayLocal = newLocal(Type.getType("[Z"));
 
-    pushConstant(this.blocks.size());
-    this.mv.visitIntInsn(NEWARRAY, T_BOOLEAN);
+    this.mv.visitFieldInsn(Opcodes.GETSTATIC, className,
+        CodeCoverageStore.PROBE_FIELD_NAME, "[Z");
+
+    //Make sure that we recorded that the class was hit
+    this.mv.visitInsn(DUP);
+    this.mv.visitInsn(ICONST_0);
+    this.mv.visitInsn(ICONST_1);
+    this.mv.visitInsn(BASTORE);
     this.mv.visitVarInsn(ASTORE, this.probeHitArrayLocal);
   }
 
   @Override
   void generateProbeReportCode() {
-
-    pushConstant(this.classId);
-    pushConstant(this.probeOffset);
-    this.mv.visitVarInsn(ALOAD, this.probeHitArrayLocal);
-
-    this.methodVisitor.visitMethodInsn(Opcodes.INVOKESTATIC,
-        CodeCoverageStore.CLASS_NAME, CodeCoverageStore.PROBE_METHOD_NAME,
-        "(II[Z)V", false);
   }
 
   @Override
   void insertProbe() {
     this.mv.visitVarInsn(ALOAD, this.probeHitArrayLocal);
-    pushConstant(this.probeCount);
-    pushConstant(1);
+    pushConstant(this.probeOffset + this.probeCount);
+    this.mv.visitInsn(ICONST_1);
     this.mv.visitInsn(BASTORE);
   }
 

pitest/src/main/java/org/pitest/coverage/analysis/CoverageAnalyser.java

@@ -8,7 +8,6 @@
 import org.pitest.coverage.CoverageClassVisitor;
 import org.pitest.mutationtest.engine.gregor.analysis.DefaultInstructionCounter;
 import org.pitest.mutationtest.engine.gregor.analysis.InstructionTrackingMethodVisitor;
-
 import sun.pitest.CodeCoverageStore;
 
 /**
@@ -45,34 +44,11 @@ public void visitEnd() {
     CodeCoverageStore.registerMethod(this.classId, this.name, this.desc,
         this.probeOffset, (this.probeOffset + blocks.size()) - 1);
 
-    // according to the jvm spec
-    // "There must never be an uninitialized class instance in a local variable in code protected by an exception handler"
-    // the code to add finally blocks used by the local variable and array based
-    // probe approaches is not currently
-    // able to meet this guarantee for constructors. Although they appear to
-    // work, they are rejected by the
-    // java 7 verifier - hence fall back to a simple but slow approach.
     final DefaultInstructionCounter counter = new DefaultInstructionCounter();
-
-    if ((blockCount == 1) || this.name.equals("<init>")) {
-      accept(new InstructionTrackingMethodVisitor(
-          new SimpleBlockCoverageVisitor(blocks, counter, this.classId,
-              this.mv, this.access, this.name, this.desc, this.probeOffset),
-              counter));
-    } else if ((blockCount <= MAX_SUPPORTED_LOCAL_PROBES) && (blockCount >= 1)) {
-      accept(new InstructionTrackingMethodVisitor(
-          new LocalVariableCoverageMethodVisitor(blocks, counter, this.classId,
-              this.mv, this.access, this.name, this.desc, this.probeOffset),
-              counter));
-    } else {
-      // for now fall back to the naive implementation - could instead use array
-      // passing version
-      accept(new InstructionTrackingMethodVisitor(
-          new ArrayProbeCoverageMethodVisitor(blocks, counter, this.classId,
-              this.mv, this.access, this.name, this.desc, this.probeOffset),
-              counter));
-    }
-
+    accept(new InstructionTrackingMethodVisitor(
+        new ArrayProbeCoverageMethodVisitor(blocks, counter, this.classId,
+            this.mv, this.access, parent.getClassName(), this.name, this.desc,
+            this.probeOffset), counter));
   }
 
   private List<Block> findRequriedProbeLocations() {