Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: a63f1cfd8c14
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 409c7e654c3b
Choose a head ref
  • 9 commits
  • 14 files changed
  • 3 contributors

Commits on Mar 23, 2016

  1. [Truffle] Backed out concat rope depth limiting specializations.

    These specializations made compiled code much slower and had meager memory savings. The larger idea is sound, but we need to rethink the approach.
    nirvdrum committed Mar 23, 2016
    Copy the full SHA
    079efd4 View commit details
  2. Make parser stop updating coverage data for any eval-related parses.

    This feature is only for load/require'd files.  A second fix will be
    required to actually fix #1196 since we have a global event registered
    but we can at least make sure coverage array is the proper size now.
    enebo committed Mar 23, 2016
    Copy the full SHA
    89f4c06 View commit details
  3. Make RootNode of AST know whether it requires coverage or not.

    This is infrastructure work and not a full fix for #1196.  I
    kept this separate in case we decide to fix 1.7.x as the rest
    of the fixes will be in IR (which 1.7 lacks).
    enebo committed Mar 23, 2016
    Copy the full SHA
    39d35c0 View commit details
  4. Form-fitting a bit. MRI will not wipe out the coverage arrays on a re…

    …set.
    
    What they do instead is if visited then they leave the entry but wipe
    out the array.  If they have nothing marked as visited they leave that
    entry unchanged.
    enebo committed Mar 23, 2016
    Copy the full SHA
    d136537 View commit details
  5. Fixes #1196 Coverage reports wrong number of lines...and then some.

    This also fixes remaining tagged out MRI test_coverage.rb issues.
    Changes:
    
    - Event hook removes lots of extra sanity checks which almost
      entirely was fall out from evals being treated as needing
      coverage.  So, for example, random line number oddities no
      longer exist because load/require always starts at line 0.
    - Removed errant printf from last commit.
    - Instrument COVERAGE Events where LINE events happen so long
      as the AST/IRScope being built requires coverage.  I notice
      MRI does not emit both LINE + COVERAGE when COVERAGE is
      present so perhaps this is a future thing to change.  Adding
      this instrumentation ended up being much ickier than an-
      ticipated since we:
      a: lazily compile methods
      b: transform define_method blocks to IRMethods and re-build
      These two "lazy" builds required adding a new IRScope field
      called CODE_COVERAGE.
    - Fixed inverted logic in ParserConfiguration from last commit.
    - CoverageData was missing a significant piece of behavior. If
      result was called and then start was reenabled any files
      which had already had coverage happen to them are supposed to
      replace their line array with an empty one {"foo.rb" => []}.
      This was key piece to eliminating remaining MRI tests from
      failing.
    
    I also ran specs from simplecov and it is green.  As far as I
    know there are no more missing behavior from coverage.  @donv
    has said he would add specs to ruby/spec for coverage.  At this
    time there are no specs for coverage there at all.
    enebo committed Mar 23, 2016
    Copy the full SHA
    35c1998 View commit details
  6. Update signature after rebase

    enebo committed Mar 23, 2016
    Copy the full SHA
    c02d46d View commit details
  7. Copy the full SHA
    5e3c54b View commit details
  8. Copy the full SHA
    07041c6 View commit details
  9. Copy the full SHA
    409c7e6 View commit details
19 changes: 15 additions & 4 deletions core/src/main/java/org/jruby/ast/RootNode.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
70 changes: 39 additions & 31 deletions core/src/main/java/org/jruby/ext/coverage/CoverageData.java
Original file line number Diff line number Diff line change
@@ -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;
}
};

8 changes: 6 additions & 2 deletions core/src/main/java/org/jruby/ext/coverage/CoverageModule.java
Original file line number Diff line number Diff line change
@@ -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];
20 changes: 16 additions & 4 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
Original file line number Diff line number Diff line change
@@ -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

20 changes: 17 additions & 3 deletions core/src/main/java/org/jruby/ir/IRClosure.java
Original file line number Diff line number Diff line change
@@ -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) {
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/IRFlags.java
Original file line number Diff line number Diff line change
@@ -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)
}
6 changes: 4 additions & 2 deletions core/src/main/java/org/jruby/ir/IRMethod.java
Original file line number Diff line number Diff line change
@@ -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;
}
4 changes: 2 additions & 2 deletions core/src/main/java/org/jruby/ir/persistence/IRReader.java
Original file line number Diff line number Diff line change
@@ -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:
6 changes: 0 additions & 6 deletions core/src/main/java/org/jruby/parser/Parser.java
Original file line number Diff line number Diff line change
@@ -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;
}

Loading