jruby · Mar 23, 2016 · Mar 23, 2016 · Mar 23, 2016 · Mar 23, 2016 · Mar 23, 2016
diff --git a/core/src/main/java/org/jruby/ast/RootNode.java b/core/src/main/java/org/jruby/ast/RootNode.java
@@ -31,6 +31,7 @@
 import java.util.List;
 
 import org.jruby.ast.visitor.NodeVisitor;
+import org.jruby.ext.coverage.CoverageData;
 import org.jruby.lexer.yacc.ISourcePosition;
 import org.jruby.parser.StaticScope;
 import org.jruby.runtime.DynamicScope;
@@ -49,21 +50,26 @@ public class RootNode extends Node {
     private Node bodyNode;
     private String file;
     private int endPosition;
+    private boolean needsCodeCoverage;
 
     public RootNode(ISourcePosition position, DynamicScope scope, Node bodyNode, String file) {
-        this(position, scope, bodyNode, file, -1);
+        this(position, scope, bodyNode, file, -1, false);
     }
 
-    public RootNode(ISourcePosition position, DynamicScope scope, Node bodyNode, String file, int endPosition) {
+    public RootNode(ISourcePosition position, DynamicScope scope, Node bodyNode, String file, int endPosition, boolean needsCodeCoverage) {
         super(position, bodyNode.containsVariableAssignment());
 
-        assert bodyNode != null : "bodyNode is not null";
-
         this.scope = scope;
         this.staticScope = scope.getStaticScope();
         this.bodyNode = bodyNode;
         this.file = file;
         this.endPosition = endPosition;
+        this.needsCodeCoverage = needsCodeCoverage;
+    }
+
+    @Deprecated
+    public RootNode(ISourcePosition position, DynamicScope scope, Node bodyNode, String file, int endPosition) {
+        this(position, scope, bodyNode, file, endPosition, false);
     }
 
     public NodeType getNodeType() {
@@ -122,4 +128,9 @@ public boolean hasEndPosition() {
     public int getEndPosition() {
         return endPosition;
     }
+
+    // Is coverage enabled and is this a valid source file for coverage to apply?
+    public boolean needsCoverage() {
+        return needsCodeCoverage;
+    }
 }
diff --git a/core/src/main/java/org/jruby/ext/coverage/CoverageData.java b/core/src/main/java/org/jruby/ext/coverage/CoverageData.java
@@ -36,33 +36,59 @@
 import org.jruby.runtime.builtin.IRubyObject;
 
 public class CoverageData {
+    public static final String STARTED = "";        // no load/require ruby file can be "" so we
+    private static final int[] SVALUE = new int[0];  // use it as a holder to know if start occurs
     private volatile Map<String, int[]> coverage;
 
     public boolean isCoverageEnabled() {
-        return coverage != null;
+        return coverage != null && coverage.get(STARTED) != null;
     }
 
     public Map<String, int[]> getCoverage() {
-      return this.coverage;
+      return coverage;
     }
 
     public synchronized void setCoverageEnabled(Ruby runtime, boolean enabled) {
+        Map<String, int[]> coverage = this.coverage;
+
+        if (coverage == null) coverage = new HashMap<String, int[]>();
+
         if (enabled) {
-            coverage = new HashMap<String, int[]>();
+            coverage.put(STARTED, SVALUE);
             runtime.addEventHook(COVERAGE_HOOK);
         } else {
-            coverage = null;
+            coverage.remove(STARTED);
         }
+
+        this.coverage = coverage;
     }
 
     public synchronized Map<String, int[]> resetCoverage(Ruby runtime) {
         Map<String, int[]> coverage = this.coverage;
         runtime.removeEventHook(COVERAGE_HOOK);
-        this.coverage = null;
-
+        coverage.remove(STARTED);
+
+
+        for (Map.Entry<String, int[]> entry : coverage.entrySet()) {
+            String key = entry.getKey();
+            if (key.equals(CoverageData.STARTED)) continue; // ignore our hidden marker
+
+            // on reset we do not reset files where no execution ever happened but we do reset
+            // any files visited to be an empty array.  Why?  I don't know.  Matching MRI.
+            if (hasCodeBeenPartiallyCovered(entry.getValue())) coverage.put(key, SVALUE);
+        }
+
         return coverage;
     }
 
+    private boolean hasCodeBeenPartiallyCovered(int[] lines) {
+        for (int i = 0; i < lines.length; i++) {
+            if (lines[i] > 0) return true;
+        }
+
+        return false;
+    }
+
     public synchronized Map<String, int[]> prepareCoverage(String filename, int[] lines) {
         assert lines != null;
 
@@ -78,36 +104,18 @@ public synchronized Map<String, int[]> prepareCoverage(String filename, int[] li
     private final EventHook COVERAGE_HOOK = new EventHook() {
         @Override
         public synchronized void eventHandler(ThreadContext context, String eventName, String file, int line, String name, IRubyObject type) {
-            if (coverage == null || line <= 0) {
-                return;
-            }
-
-            // make sure we have a lines array of acceptable length for the given file
+            if (coverage == null || line <= 0) return; // Should not be needed but I predict serialization of IR might hit this.
+
             int[] lines = coverage.get(file);
-            if (lines == null) {
-                // loaded before coverage; skip
-                return;
-            } else if (lines.length < line) {
-                // can this happen? shouldn't all coverable lines be here already (from parse time)?
-                int[] newLines = new int[line];
-                Arrays.fill(newLines, lines.length, line, -1); // mark unknown lines as -1
-                System.arraycopy(lines, 0, newLines, 0, lines.length);
-                lines = newLines;
-                coverage.put(file, lines);
-            }
-
-            // increment the line's count or set it to 1
-            int count = lines[line - 1];
-            if (count == -1) {
-                lines[line - 1] = 1;
-            } else {
-                lines[line - 1] = count + 1;
-            }
+            if (lines == null) return;           // no coverage lines for this record.  bail out (should never happen)
+            if (lines.length == 0) return;       // coverage is dead for this record.  result() has been called once
+                                                 // and we marked it as such as an empty list.
+            lines[line - 1] += 1;                // increment usage count by one.
         }
 
         @Override
         public boolean isInterestedInEvent(RubyEvent event) {
-            return event == RubyEvent.LINE;
+            return event == RubyEvent.COVERAGE;
         }
     };
 

diff --git a/core/src/main/java/org/jruby/ext/coverage/CoverageModule.java b/core/src/main/java/org/jruby/ext/coverage/CoverageModule.java
@@ -57,8 +57,10 @@ public static IRubyObject result(ThreadContext context, IRubyObject self) {
         if (!runtime.getCoverageData().isCoverageEnabled()) {
             throw runtime.newRuntimeError("coverage measurement is not enabled");
         }
-
-        return convertCoverageToRuby(context, runtime, runtime.getCoverageData().resetCoverage(runtime));
+
+        IRubyObject result = convertCoverageToRuby(context, runtime, runtime.getCoverageData().getCoverage());
+        runtime.getCoverageData().resetCoverage(runtime);
+        return result;
     }
 
     @JRubyMethod(module = true)
@@ -76,6 +78,8 @@ private static IRubyObject convertCoverageToRuby(ThreadContext context, Ruby run
         // populate a Ruby Hash with coverage data
         RubyHash covHash = RubyHash.newHash(runtime);
         for (Map.Entry<String, int[]> entry : coverage.entrySet()) {
+            if (entry.getKey().equals(CoverageData.STARTED)) continue; // ignore our hidden marker
+
             RubyArray ary = RubyArray.newArray(runtime, entry.getValue().length);
             for (int i = 0; i < entry.getValue().length; i++) {
                 int integer = entry.getValue()[i];

diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -289,6 +289,7 @@ public IRLoop getCurrentLoop() {
     protected IRScope scope;
     protected List<Instr> instructions;
     protected List<String> argumentDescriptions;
+    protected boolean needsCodeCoverage;
 
     public IRBuilder(IRManager manager, IRScope scope, IRBuilder parent) {
         this.manager = manager;
@@ -298,6 +299,10 @@ public IRBuilder(IRManager manager, IRScope scope, IRBuilder parent) {
         this.activeRescuers.push(Label.UNRESCUED_REGION_LABEL);
     }
 
+    private boolean needsCodeCoverage() {
+        return needsCodeCoverage || parent != null && parent.needsCodeCoverage();
+    }
+
     public void addArgumentDescription(ArgumentType type, String name) {
         if (argumentDescriptions == null) argumentDescriptions = new ArrayList<>();
 
@@ -368,6 +373,9 @@ private Operand buildOperand(Node node) throws NotCompilableException {
             if (currLineNum != _lastProcessedLineNum) { // Do not emit multiple line number instrs for the same line
                 if (RubyInstanceConfig.FULL_TRACE_ENABLED) {
                     addInstr(new TraceInstr(RubyEvent.LINE, methodNameFor(), getFileName(), currLineNum));
+                    if (needsCodeCoverage()) {
+                        addInstr(new TraceInstr(RubyEvent.COVERAGE, methodNameFor(), getFileName(), currLineNum));
+                    }
                 }
                 addInstr(manager.newLineNumber(currLineNum));
                 _lastProcessedLineNum = currLineNum;
@@ -516,7 +524,7 @@ private InterpreterContext buildLambdaInner(LambdaNode node) {
     }
 
     public Operand buildLambda(LambdaNode node) {
-        IRClosure closure = new IRClosure(manager, scope, node.getLine(), node.getScope(), Signature.from(node));
+        IRClosure closure = new IRClosure(manager, scope, node.getLine(), node.getScope(), Signature.from(node), needsCodeCoverage);
 
         // Create a new nested builder to ensure this gets its own IR builder state like the ensure block stack
         newIRBuilder(manager, closure).buildLambdaInner(node);
@@ -1764,7 +1772,9 @@ public Operand buildDAsgn(final DAsgnNode dasgnNode) {
 
     // Called by defineMethod but called on a new builder so things like ensure block info recording
     // do not get confused.
-    protected InterpreterContext defineMethodInner(DefNode defNode, IRScope parent) {
+    protected InterpreterContext defineMethodInner(DefNode defNode, IRScope parent, boolean needsCodeCoverage) {
+        this.needsCodeCoverage = needsCodeCoverage;
+
         if (RubyInstanceConfig.FULL_TRACE_ENABLED) {
             addInstr(new TraceInstr(RubyEvent.CALL, getName(), getFileName(), scope.getLineNumber()));
         }
@@ -1815,7 +1825,7 @@ protected InterpreterContext defineMethodInner(DefNode defNode, IRScope parent)
     static final ArgumentDescriptor[] NO_ARG_DESCS = new ArgumentDescriptor[0];
 
     private IRMethod defineNewMethod(MethodDefNode defNode, boolean isInstanceMethod) {
-        return new IRMethod(manager, scope, defNode, defNode.getName(), isInstanceMethod, defNode.getLine(), defNode.getScope());
+        return new IRMethod(manager, scope, defNode, defNode.getName(), isInstanceMethod, defNode.getLine(), defNode.getScope(), needsCodeCoverage());
 
         //return newIRBuilder(manager).defineMethodInner(defNode, method, parent);
     }
@@ -2744,7 +2754,7 @@ private InterpreterContext buildIterInner(IterNode iterNode) {
         return scope.allocateInterpreterContext(instructions);
     }
     public Operand buildIter(final IterNode iterNode) {
-        IRClosure closure = new IRClosure(manager, scope, iterNode.getLine(), iterNode.getScope(), Signature.from(iterNode));
+        IRClosure closure = new IRClosure(manager, scope, iterNode.getLine(), iterNode.getScope(), Signature.from(iterNode), needsCodeCoverage);
 
         // Create a new nested builder to ensure this gets its own IR builder state like the ensure block stack
         newIRBuilder(manager, closure).buildIterInner(iterNode);
@@ -3419,6 +3429,7 @@ public Operand buildReturn(ReturnNode returnNode) {
     }
 
     public InterpreterContext buildEvalRoot(RootNode rootNode) {
+        needsCodeCoverage = false;  // Assuming there is no path into build eval root without actually being an eval.
         addInstr(manager.newLineNumber(scope.getLineNumber()));
 
         prepareImplicitState();                                    // recv_self, add frame block, etc)
@@ -3442,6 +3453,7 @@ private void addCurrentScopeAndModule() {
     }
 
     private InterpreterContext buildRootInner(RootNode rootNode) {
+        needsCodeCoverage = rootNode.needsCoverage();
         prepareImplicitState();                                    // recv_self, add frame block, etc)
         addCurrentScopeAndModule();                                // %current_scope/%current_module
 

diff --git a/core/src/main/java/org/jruby/ir/IRClosure.java b/core/src/main/java/org/jruby/ir/IRClosure.java
@@ -77,16 +77,27 @@ protected IRClosure(IRClosure c, IRScope lexicalParent, int closureId, String fu
         this.signature = c.signature;
     }
 
-    // Used by iter + lambda by IRBuilder
+    // Used by persistence.  Knowledge of coverage not needed here since it is already instrumented into the instrs
     public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, StaticScope staticScope, Signature signature) {
-        this(manager, lexicalParent, lineNumber, staticScope, signature, "_CLOSURE_");
+        this(manager, lexicalParent, lineNumber, staticScope, signature, "_CLOSURE_", false);
     }
 
+    // Used by iter + lambda by IRBuilder
+    public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, StaticScope staticScope, Signature signature, boolean needsCoverage) {
+        this(manager, lexicalParent, lineNumber, staticScope, signature, "_CLOSURE_", false, needsCoverage);
+    }
+
+
     public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, StaticScope staticScope, Signature signature, String prefix) {
         this(manager, lexicalParent, lineNumber, staticScope, signature, prefix, false);
     }
 
     public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, StaticScope staticScope, Signature signature, String prefix, boolean isBeginEndBlock) {
+        this(manager, lexicalParent, lineNumber, staticScope, signature, prefix, isBeginEndBlock, false);
+    }
+
+    public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, StaticScope staticScope,
+                     Signature signature, String prefix, boolean isBeginEndBlock, boolean needsCoverage) {
         this(manager, lexicalParent, lineNumber, staticScope, prefix);
         this.signature = signature;
         lexicalParent.addClosure(this);
@@ -101,8 +112,11 @@ public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, Stati
                 staticScope.setScopeType(this.getScopeType());
             }
         }
+
+        if (needsCoverage) getFlags().add(IRFlags.CODE_COVERAGE);
     }
 
+
     @Override
     public InterpreterContext allocateInterpreterContext(List<Instr> instructions) {
         interpreterContext = new ClosureInterpreterContext(this, instructions);
@@ -205,7 +219,7 @@ public IRMethod convertToMethod(String name) {
         DefNode def = source;
         source = null;
 
-        return new IRMethod(getManager(), getLexicalParent(), def, name, true,  getLineNumber(), getStaticScope());
+        return new IRMethod(getManager(), getLexicalParent(), def, name, true,  getLineNumber(), getStaticScope(), getFlags().contains(IRFlags.CODE_COVERAGE));
     }
 
     public void setSource(IterNode iter) {

diff --git a/core/src/main/java/org/jruby/ir/IRFlags.java b/core/src/main/java/org/jruby/ir/IRFlags.java
@@ -58,4 +58,5 @@ public enum IRFlags {
     DYNSCOPE_ELIMINATED,          // local var load/stores have been converted to tmp var accesses
     REUSE_PARENT_DYNSCOPE,        // for closures -- reuse parent's dynscope
     SIMPLE_METHOD,                // probably temporary flag.  Can this method scope fit into a simple method interpreter
+    CODE_COVERAGE,                // Was marked as needing code coverage (only used by lazy methods and converting closures->methods)
 }
diff --git a/core/src/main/java/org/jruby/ir/IRMethod.java b/core/src/main/java/org/jruby/ir/IRMethod.java
@@ -16,12 +16,14 @@ public class IRMethod extends IRScope {
     private DefNode defn;
 
     public IRMethod(IRManager manager, IRScope lexicalParent, DefNode defn, String name,
-            boolean isInstanceMethod, int lineNumber, StaticScope staticScope) {
+            boolean isInstanceMethod, int lineNumber, StaticScope staticScope, boolean needsCodeCoverage) {
         super(manager, lexicalParent, name, lineNumber, staticScope);
 
         this.defn = defn;
         this.isInstanceMethod = isInstanceMethod;
 
+        if (needsCodeCoverage) getFlags().add(IRFlags.CODE_COVERAGE);
+
         if (!getManager().isDryRun() && staticScope != null) {
             staticScope.setIRScope(this);
         }
@@ -34,7 +36,7 @@ public boolean hasBeenBuilt() {
 
     public synchronized InterpreterContext lazilyAcquireInterpreterContext() {
         if (!hasBeenBuilt()) {
-            IRBuilder.topIRBuilder(getManager(), this).defineMethodInner(defn, getLexicalParent());
+            IRBuilder.topIRBuilder(getManager(), this).defineMethodInner(defn, getLexicalParent(), getFlags().contains(IRFlags.CODE_COVERAGE));
 
             defn = null;
         }

diff --git a/core/src/main/java/org/jruby/ir/persistence/IRReader.java b/core/src/main/java/org/jruby/ir/persistence/IRReader.java
@@ -150,9 +150,9 @@ public static IRScope createScope(IRManager manager, IRScopeType type, String na
         case METACLASS_BODY:
             return new IRMetaClassBody(manager, lexicalParent, manager.getMetaClassName(), line, staticScope);
         case INSTANCE_METHOD:
-            return new IRMethod(manager, lexicalParent, null, name, true, line, staticScope);
+            return new IRMethod(manager, lexicalParent, null, name, true, line, staticScope, false);
         case CLASS_METHOD:
-            return new IRMethod(manager, lexicalParent, null, name, false, line, staticScope);
+            return new IRMethod(manager, lexicalParent, null, name, false, line, staticScope, false);
         case MODULE_BODY:
             return new IRModuleBody(manager, lexicalParent, name, line, staticScope);
         case SCRIPT_BODY:

diff --git a/core/src/main/java/org/jruby/parser/Parser.java b/core/src/main/java/org/jruby/parser/Parser.java
@@ -158,12 +158,6 @@ public Node parse(String file, LexerSource lexerSource, DynamicScope blockScope,
         totalTime += System.nanoTime() - startTime;
         totalBytes += lexerSource.getOffset();
 
-        // set coverage baseline into coverage data
-        if (runtime.getCoverageData().isCoverageEnabled()) {
-            configuration.growCoverageLines(parser.lexer.lineno());
-            runtime.getCoverageData().prepareCoverage(file, configuration.getCoverage());
-        }
-
         return ast;
     }