Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Further streamlining of interpreter context
* Deleted old code in IRScope that is no longer relevant.

* For closures, short-circuited prepareForInterpretation from
  base class to ensure that callers use the 1-arg version where
  it needs to be built.

* Subsequent crashers exposed an unresolved FIXME in BuildLambdaInstr
  that had a WrappedIRClosure that needed cloning and build of an
  interpreter context as well.
  • Loading branch information
subbuss committed Oct 15, 2014
1 parent c664f89 commit 1a1f927
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 57 deletions.
Expand Up @@ -134,7 +134,7 @@ protected void pre(InterpreterContext ic, ThreadContext context, IRubyObject sel
}

public InterpreterContext ensureInstrsReady() {
InterpreterContext context = method.getInstrsForInterpretation();
InterpreterContext context = method.getInterpreterContext();
if (context == null) {
context = method.prepareForInterpretation();
}
Expand Down
16 changes: 11 additions & 5 deletions core/src/main/java/org/jruby/ir/IRClosure.java
Expand Up @@ -109,21 +109,27 @@ public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, Stati
this.nestingDepth++;
}

public InterpreterContext getInterpreterContext(Operand self) {
initScope(false);

checkRelinearization();

public InterpreterContext prepareInterpreterContext(Operand self) {
if (interpreterContext != null) return interpreterContext; // Already prepared

initScope(false);

Instr[] linearizedInstrArray = prepareInstructions();

interpreterContext = new ClosureInterpreterContext(getTemporaryVariablesCount(), getBooleanVariablesCount(),
getFixnumVariablesCount(), getFloatVariablesCount(),getFlags().clone(), linearizedInstrArray,
self, getStaticScope(), getBlockBody());

return interpreterContext;
}

@Override
public synchronized InterpreterContext prepareForInterpretation() {
// This should have already been prepared during preparation of parent scopes.
// If this is null, it would be a bug and let users throw a NPE.
return interpreterContext;
}

public void setBeginEndBlock() {
this.isBeginEndBlock = true;
}
Expand Down
41 changes: 13 additions & 28 deletions core/src/main/java/org/jruby/ir/IRScope.java
Expand Up @@ -466,18 +466,6 @@ public CFG getCFG() {
return cfg;
}

private synchronized InterpreterContext prepareInterpreterContext() {
checkRelinearization();

if (interpreterContext != null) return interpreterContext; // Already prepared

Instr[] linearizedInstrArray = prepareInstructions();
interpreterContext = new InterpreterContext(getTemporaryVariablesCount(), getBooleanVariablesCount(),
getFixnumVariablesCount(), getFloatVariablesCount(),getFlags().clone(), linearizedInstrArray);

return interpreterContext;
}

protected Instr[] prepareInstructions() {
setupLinearization();

Expand Down Expand Up @@ -609,9 +597,6 @@ private void optimizeSimpleScopes() {
}

protected void initScope(boolean jitMode) {
// Reset linearization, if any exists
resetLinearizationData();

// Build CFG and run compiler passes, if necessary
if (getCFG() == null) {
buildCFG();
Expand All @@ -631,25 +616,31 @@ protected void initScope(boolean jitMode) {

/** Run any necessary passes to get the IR ready for interpretation */
public synchronized InterpreterContext prepareForInterpretation() {
initScope(false);
if (interpreterContext != null) return interpreterContext; // Already prepared

checkRelinearization();
initScope(false);

// System.out.println("-- passes run for: " + this + " = " + java.util.Arrays.toString(executedPasses.toArray()));

// Linearize CFG, etc.
return prepareInterpreterContext();
Instr[] linearizedInstrArray = prepareInstructions();

interpreterContext = new InterpreterContext(getTemporaryVariablesCount(), getBooleanVariablesCount(),
getFixnumVariablesCount(), getFloatVariablesCount(),getFlags().clone(), linearizedInstrArray);

return interpreterContext;
}

/* SSS FIXME: Do we need to synchronize on this? Cache this info in a scope field? */
/** Run any necessary passes to get the IR ready for compilation */
public synchronized List<BasicBlock> prepareForCompilation() {
// Reset linearization, if any exists
resetLinearizationData();

initScope(true);

runCompilerPasses(getManager().getJITPasses(this));

checkRelinearization();

prepareInstructions();

return buildLinearization();
Expand Down Expand Up @@ -1047,7 +1038,7 @@ public List<Instr> getInstrs() {
return instrList;
}

public InterpreterContext getInstrsForInterpretation() {
public InterpreterContext getInterpreterContext() {
return interpreterContext;
}

Expand All @@ -1056,13 +1047,7 @@ public void resetLinearizationData() {
relinearizeCFG = false;
}

public void checkRelinearization() {
if (relinearizeCFG) resetLinearizationData();
}

public List<BasicBlock> buildLinearization() {
checkRelinearization();

if (linearizedBBList != null) return linearizedBBList; // Already linearized

linearizedBBList = CFGLinearizer.linearize(cfg);
Expand Down Expand Up @@ -1090,8 +1075,8 @@ public CFG cfg() {
}

public void resetState() {
relinearizeCFG = true;
interpreterContext = null;
resetLinearizationData();
cfg.resetState();

// reset flags
Expand Down
Expand Up @@ -21,9 +21,9 @@ public class BuildLambdaInstr extends Instr implements ResultInstr, FixedArityIn
/** The position for the block */
private final ISourcePosition position;
private Variable result;
private WrappedIRClosure lambdaBody;
private Operand lambdaBody;

public BuildLambdaInstr(Variable lambda, WrappedIRClosure lambdaBody, ISourcePosition position) {
public BuildLambdaInstr(Variable lambda, Operand lambdaBody, ISourcePosition position) {
super(Operation.LAMBDA);

this.result = lambda;
Expand All @@ -32,8 +32,10 @@ public BuildLambdaInstr(Variable lambda, WrappedIRClosure lambdaBody, ISourcePos
}

public String getLambdaBodyName() {
return getLambdaBody().getClosure().getName();
// SSS FIXME: this requires a fix
return ""; // getLambdaBody().getClosure().getName();
}

@Override
public Operand[] getOperands() {
return new Operand[] { lambdaBody, new StringLiteral(position.getFile()), new Fixnum(position.getLine()) };
Expand Down Expand Up @@ -61,16 +63,15 @@ public boolean computeScopeFlags(IRScope scope) {

@Override
public Instr clone(CloneInfo ii) {
// SSS FIXME: This is buggy. The lambda body might have to be cloned depending on cloning context.
return new BuildLambdaInstr(ii.getRenamedVariable(getResult()), getLambdaBody(), position);
return new BuildLambdaInstr(ii.getRenamedVariable(getResult()), getLambdaBody().cloneForInlining(ii), position);
}

@Override
public void simplifyOperands(Map<Operand, Operand> valueMap, boolean force) {
lambdaBody = (WrappedIRClosure) lambdaBody.getSimplifiedOperand(valueMap, force);
lambdaBody = lambdaBody.getSimplifiedOperand(valueMap, force);
}

public WrappedIRClosure getLambdaBody() {
public Operand getLambdaBody() {
return lambdaBody;
}

Expand All @@ -83,7 +84,7 @@ public Object interpret(ThreadContext context, StaticScope currScope, DynamicSco
// SSS FIXME: Copied this from ast/LambdaNode ... Is this required here as well?
//
// JRUBY-5686: do this before executing so first time sets cref module
getLambdaBody().getClosure().getStaticScope().determineModule();
((ClosureInterpreterContext)getLambdaBody()).getStaticScope().determineModule();

// CON: This must not be happening, because nil would never cast to Block
// IRClosure body = getLambdaBody().getClosure();
Expand Down
9 changes: 6 additions & 3 deletions core/src/main/java/org/jruby/ir/interpreter/Interpreter.java
Expand Up @@ -78,7 +78,10 @@ public static IRubyObject interpretCommonEval(Ruby runtime, String file, int lin
StaticScope ss = rootNode.getStaticScope();
IRScope containingIRScope = getEvalContainerScope(ss, evalType);
IREvalScript evalScript = IRBuilder.createIRBuilder(runtime, runtime.getIRManager()).buildEvalRoot(ss, containingIRScope, file, lineNumber, rootNode, evalType);
evalScript.prepareForInterpretation();
// ClosureInterpreterContext never retrieved as an operand in this context.
// So, self operand is not required here.
// Passing null to force early crasher if ever used differently.
evalScript.prepareInterpreterContext(null);
ThreadContext context = runtime.getCurrentContext();

IRubyObject rv = null;
Expand Down Expand Up @@ -121,7 +124,7 @@ public static void runBeginEndBlocks(List<IRClosure> beBlocks, ThreadContext con

for (IRClosure b: beBlocks) {
// SSS FIXME: Should I piggyback on WrappedIRClosure.retrieve or just copy that code here?
b.prepareForInterpretation();
b.prepareInterpreterContext(b.getSelf());
Block blk = (Block)(new WrappedIRClosure(b.getSelf(), b)).retrieve(context, self, currScope, context.getCurrentScope(), temp);
blk.yield(context, null);
}
Expand Down Expand Up @@ -525,7 +528,7 @@ private static DynamicScope getNewDynScope(ThreadContext context, IRScope scope,

private static IRubyObject interpret(ThreadContext context, IRubyObject self,
IRScope scope, Visibility visibility, RubyModule implClass, String name, IRubyObject[] args, Block block, Block.Type blockType) {
InterpreterContext interpreterContext = scope.getInstrsForInterpretation();
InterpreterContext interpreterContext = scope.getInterpreterContext();
Instr[] instrs = interpreterContext.getInstructions();
int numTempVars = interpreterContext.getTemporaryVariablecount();
Object[] temp = numTempVars > 0 ? new Object[numTempVars] : null;
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/jruby/ir/interpreter/Profiler.java
Expand Up @@ -162,7 +162,7 @@ public int compare(IRCallSite a, IRCallSite b) {

IRScope tgtMethod = ircs.tgtM.getIRMethod();

Instr[] instrs = tgtMethod.getInstrsForInterpretation().getInstructions();
Instr[] instrs = tgtMethod.getInterpreterContext().getInstructions();
// Dont inline large methods -- 500 is arbitrary
// Can be null if a previously inlined method hasn't been rebuilt
if ((instrs == null) || instrs.length > 500) {
Expand Down Expand Up @@ -228,8 +228,8 @@ public int compare(IRScope a, IRScope b) {
if (bden == 0) bden = 1;

// Use estimated instr count to order scopes -- rather than raw thread-poll count
float aCount = scopeThreadPollCounts.get(a).count * (1.0f * a.getInstrsForInterpretation().getInstructions().length/aden);
float bCount = scopeThreadPollCounts.get(b).count * (1.0f * b.getInstrsForInterpretation().getInstructions().length/bden);
float aCount = scopeThreadPollCounts.get(a).count * (1.0f * a.getInterpreterContext().getInstructions().length/aden);
float bCount = scopeThreadPollCounts.get(b).count * (1.0f * b.getInterpreterContext().getInstructions().length/bden);
if (aCount == bCount) return 0;
return (aCount < bCount) ? 1 : -1;
}
Expand Down
Expand Up @@ -32,6 +32,8 @@ public ClosureInterpreterContext(int temporaryVariablecount, int temporaryBoolea
this.body = body;
}

public StaticScope getStaticScope() { return staticScope; }

@Override
public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
staticScope.determineModule();
Expand Down
Expand Up @@ -61,9 +61,9 @@ public Operand cloneForInlining(CloneInfo info) {
return new WrappedIRClosure(info.getRenamedVariable(self), closure.cloneForInlining(info));
}

// We are saving instructions so that if JIT hits IRClosure it will not concurrently
// We are saving instructions so that if JIT hits IRClosure, it will not concurrently
// modify the same object.
return closure.getInterpreterContext(self);
return closure.prepareInterpreterContext(self);
}

return new WrappedIRClosure(info.getRenamedVariable(self), closure.cloneForInlining(info));
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Expand Up @@ -1931,7 +1931,7 @@ public void BuildLambdaInstr(BuildLambdaInstr buildlambdainstr) {

jvmMethod().loadRuntime();

IRClosure body = buildlambdainstr.getLambdaBody().getClosure();
IRClosure body = ((WrappedIRClosure)buildlambdainstr.getLambdaBody()).getClosure();
if (body == null) {
jvmMethod().pushNil();
} else {
Expand Down
Expand Up @@ -27,19 +27,15 @@ public InterpretedIRBlockBody(IRClosure closure, Arity arity, int argumentType)
}

public InterpreterContext ensureInstrsReady() {
InterpreterContext context = closure.getInstrsForInterpretation();
if (context == null) {
context = closure.prepareForInterpretation();
}

if (IRRuntimeHelpers.isDebug() && !displayedCFG) {
LOG.info("Executing '" + closure + "' (pushScope=" + pushScope + ", reuseParentScope=" + reuseParentScope);
CFG cfg = closure.getCFG();
LOG.info("Graph:\n" + cfg.toStringGraph());
LOG.info("CFG:\n" + cfg.toStringInstrs());
displayedCFG = true;
}
return context;
// Always prepared in the context of parent scope -- so a null value here is a bug.
return closure.getInterpreterContext();
}

protected IRubyObject commonYieldPath(ThreadContext context, IRubyObject[] args, IRubyObject self, Binding binding, Type type, Block block) {
Expand Down

0 comments on commit 1a1f927

Please sign in to comment.