Skip to content

Commit

Permalink
Prepare instrs. for interp once per scope
Browse files Browse the repository at this point in the history 
* If interp context is available, don't rerun prepareInterpreterContext
  repeatedly on scopes. But, in order to make sure we get the right
  flags for pushing/popping scopes, move the cached info for them
  into InterpreterContext. But, now that I write this, perhaps the
  biggest perf. regression was from running prepareInterpreterContext
  repeatedly rather than fetching the flags repeatedly.
  • Loading branch information
@subbuss
subbuss committed Oct 15, 2014
1 parent 695b389 commit c664f89
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 47 deletions.
19 changes: 12 additions & 7 deletions core/src/main/java/org/jruby/internal/runtime/methods/InterpretedIRMetaClassBody.java
View file
Expand Up @@ -5,6 +5,7 @@

import org.jruby.RubyModule;
import org.jruby.ir.*;
import org.jruby.ir.operands.InterpreterContext;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Block;
import org.jruby.runtime.DynamicScope;
Expand All @@ -22,20 +23,24 @@ public List<String[]> getParameterList() {
}

@Override
protected void post(ThreadContext context) {
protected void post(InterpreterContext ic, ThreadContext context) {
// update call stacks (pop: ..)
context.popFrame();
context.popScope();
if (ic.popDynScope()) {
context.popScope();
}
}

@Override
protected void pre(ThreadContext context, IRubyObject self, String name, Block block) {
protected void pre(InterpreterContext ic, ThreadContext context, IRubyObject self, String name, Block block) {
// update call stacks (push: frame, class, scope, etc.)
context.preMethodFrameOnly(getImplementationClass(), name, self, block);
// Add a parent-link to current dynscope to support non-local returns cheaply
// This doesn't affect variable scoping since local variables will all have
// the right scope depth.
context.pushScope(DynamicScope.newDynamicScope(method.getStaticScope(), context.getCurrentScope()));
if (ic.pushNewDynScope()) {
// Add a parent-link to current dynscope to support non-local returns cheaply
// This doesn't affect variable scoping since local variables will all have
// the right scope depth.
context.pushScope(DynamicScope.newDynamicScope(method.getStaticScope(), context.getCurrentScope()));
}
context.setCurrentVisibility(getVisibility());
}

Expand Down
32 changes: 16 additions & 16 deletions core/src/main/java/org/jruby/internal/runtime/methods/InterpretedIRMethod.java
View file
Expand Up @@ -29,7 +29,6 @@ public class InterpretedIRMethod extends DynamicMethod implements IRMethodArgs,

private Arity arity;
private boolean displayedCFG = false; // FIXME: Remove when we find nicer way of logging CFG
private boolean pushScope;

protected final IRScope method;

Expand All @@ -45,7 +44,6 @@ public InterpretedIRMethod(IRScope method, Visibility visibility, RubyModule imp
this.method = method;
this.method.getStaticScope().determineModule();
this.arity = calculateArity();
this.pushScope = true;
if (!implementationClass.getRuntime().getInstanceConfig().getCompileMode().shouldJIT()) {
this.box.callCount = -1;
}
Expand Down Expand Up @@ -90,18 +88,18 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz
DynamicMethod actualMethod = box.actualMethod;
if (actualMethod != null) return actualMethod.call(context, self, clazz, name, args, block);

ensureInstrsReady();
InterpreterContext ic = ensureInstrsReady();

if (IRRuntimeHelpers.isDebug()) doDebug();

if (method.hasExplicitCallProtocol()) return Interpreter.INTERPRET_METHOD(context, this, self, name, args, block);

pre(context, self, name, block);
pre(ic, context, self, name, block);

try {
return Interpreter.INTERPRET_METHOD(context, this, self, name, args, block);
} finally {
post(context);
post(ic, context);
}
}

Expand All @@ -110,35 +108,37 @@ protected void doDebug() {
String realName = name == null || "".equals(name) ? method.getName() : name;
LOG.info("Executing '" + realName + "'");
if (displayedCFG == false) {
// The base IR may not have been processed yet
CFG cfg = method.getCFG();
LOG.info("Graph:\n" + cfg.toStringGraph());
LOG.info("CFG:\n" + cfg.toStringInstrs());
displayedCFG = true;
}
}

protected void post(ThreadContext context) {
protected void post(InterpreterContext ic, ThreadContext context) {
// update call stacks (pop: ..)
context.popFrame();
if (this.pushScope) {
if (ic.popDynScope()) {
context.popScope();
}
}

protected void pre(ThreadContext context, IRubyObject self, String name, Block block) {
protected void pre(InterpreterContext ic, ThreadContext context, IRubyObject self, String name, Block block) {
// update call stacks (push: frame, class, scope, etc.)
StaticScope ss = method.getStaticScope();
context.preMethodFrameOnly(getImplementationClass(), name, self, block);
if (this.pushScope) context.pushScope(DynamicScope.newDynamicScope(method.getStaticScope()));
if (ic.pushNewDynScope()) {
context.pushScope(DynamicScope.newDynamicScope(method.getStaticScope()));
}
context.setCurrentVisibility(getVisibility());
}

public void ensureInstrsReady() {
// SSS FIXME: Move this out of here to some other place?
// Prepare method if not yet done so we know if the method has an explicit/implicit call protocol
// FIXME: This is resetting this flag per call and I now may want to push interpretercontext through to interpreter
InterpreterContext context = method.prepareForInterpretation();
this.pushScope = !context.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);
public InterpreterContext ensureInstrsReady() {
InterpreterContext context = method.getInstrsForInterpretation();
if (context == null) {
context = method.prepareForInterpretation();
}
return context;
}

public DynamicMethod getMethodForCaching() {
Expand Down
35 changes: 26 additions & 9 deletions core/src/main/java/org/jruby/ir/operands/InterpreterContext.java
View file
Expand Up @@ -10,18 +10,20 @@
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;

/**
*
*/
public class InterpreterContext extends Operand {
private int temporaryVariablecount;
private int temporaryBooleanVariablecount;
private int temporaryFixnumVariablecount;
private int temporaryFloatVariablecount;
private final int temporaryVariablecount;
private final int temporaryBooleanVariablecount;
private final int temporaryFixnumVariablecount;
private final int temporaryFloatVariablecount;

private EnumSet<IRFlags> flags;
private final EnumSet<IRFlags> flags;

private Instr[] instructions;
private final Instr[] instructions;

// Cached computed fields
private final boolean pushNewDynScope;
private final boolean reuseParentDynScope;
private final boolean popDynScope;

public InterpreterContext(int temporaryVariablecount, int temporaryBooleanVariablecount,
int temporaryFixnumVariablecount, int temporaryFloatVariablecount,
Expand All @@ -34,6 +36,9 @@ public InterpreterContext(int temporaryVariablecount, int temporaryBooleanVariab
this.temporaryFloatVariablecount = temporaryFloatVariablecount;
this.flags = flags;
this.instructions = instructions;
this.reuseParentDynScope = flags.contains(IRFlags.REUSE_PARENT_DYNSCOPE);
this.pushNewDynScope = !flags.contains(IRFlags.DYNSCOPE_ELIMINATED) && !this.reuseParentDynScope;
this.popDynScope = this.pushNewDynScope || this.reuseParentDynScope;
}

@Override
Expand Down Expand Up @@ -69,6 +74,18 @@ public Instr[] getInstructions() {
return instructions;
}

public boolean pushNewDynScope() {
return pushNewDynScope;
}

public boolean reuseParentDynScope() {
return reuseParentDynScope;
}

public boolean popDynScope() {
return popDynScope;
}

@Override
public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
return super.retrieve(context, self, currScope, currDynScope, temp);
Expand Down
29 changes: 14 additions & 15 deletions core/src/main/java/org/jruby/runtime/InterpretedIRBlockBody.java
View file
Expand Up @@ -17,6 +17,7 @@ public class InterpretedIRBlockBody extends IRBlockBody {
protected final IRClosure closure;
protected boolean pushScope;
protected boolean reuseParentScope;
private boolean displayedCFG = false; // FIXME: Remove when we find nicer way of logging CFG

public InterpretedIRBlockBody(IRClosure closure, Arity arity, int argumentType) {
super(closure.getStaticScope(), closure.getParameterList(), closure.getFileName(), closure.getLineNumber(), arity);
Expand All @@ -25,24 +26,24 @@ public InterpretedIRBlockBody(IRClosure closure, Arity arity, int argumentType)
this.reuseParentScope = false;
}

public void ensureInstrsReady(Block.Type blockType) {
// FIXME: probably save interpcontext as a field and use that as a check so we are not retrieving per call
// Prepare closure if not yet done so we know if the method requires a dynscope or not
InterpreterContext context = closure.prepareForInterpretation();
pushScope = !context.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);
reuseParentScope = context.getFlags().contains(IRFlags.REUSE_PARENT_DYNSCOPE);
public InterpreterContext ensureInstrsReady() {
InterpreterContext context = closure.getInstrsForInterpretation();
if (context == null) {
context = closure.prepareForInterpretation();
}

if (IRRuntimeHelpers.isDebug()) {
if (IRRuntimeHelpers.isDebug() && !displayedCFG) {
LOG.info("Executing '" + closure + "' (pushScope=" + pushScope + ", reuseParentScope=" + reuseParentScope);
// The base IR may not have been processed yet
CFG cfg = closure.getCFG();
LOG.info("Graph:\n" + cfg.toStringGraph());
LOG.info("CFG:\n" + cfg.toStringInstrs());
displayedCFG = true;
}
return context;
}

protected IRubyObject commonYieldPath(ThreadContext context, IRubyObject[] args, IRubyObject self, Binding binding, Type type, Block block) {
ensureInstrsReady(type);
InterpreterContext ic = ensureInstrsReady();

// SSS: Important! Use getStaticScope() to use a copy of the static-scope stored in the block-body.
// Do not use 'closure.getStaticScope()' -- that returns the original copy of the static scope.
Expand All @@ -59,18 +60,16 @@ protected IRubyObject commonYieldPath(ThreadContext context, IRubyObject[] args,
self = useBindingSelf(binding);
}


// SSS FIXME: Maybe, we should allocate a NoVarsScope/DummyScope for for-loop bodies because the static-scope here
// probably points to the parent scope? To be verified and fixed if necessary. There is no harm as it is now. It
// is just wasteful allocation since the scope is not used at all.

// Pass on eval state info to the dynamic scope and clear it on the block-body
DynamicScope prevScope = binding.getDynamicScope();
if (this.pushScope) {
if (ic.pushNewDynScope()) {
context.pushScope(DynamicScope.newDynamicScope(getStaticScope(), prevScope, this.evalType.get()));
} else if (this.reuseParentScope) {
// Reuse!
// We can avoid the push only if surrounding vars aren't referenced!
} else if (ic.reuseParentDynScope()) {
// Reuse! We can avoid the push only if surrounding vars aren't referenced!
context.pushScope(prevScope);
}
this.evalType.set(EvalType.NONE);
Expand All @@ -83,7 +82,7 @@ protected IRubyObject commonYieldPath(ThreadContext context, IRubyObject[] args,
// Ex: eval("...", foo.instance_eval { binding })
// The dyn-scope used for binding needs to have its eval-type set to INSTANCE_EVAL
binding.getFrame().setVisibility(oldVis);
if (this.pushScope || this.reuseParentScope) {
if (ic.popDynScope()) {
context.postYield(binding, prevFrame);
} else {
context.postYieldNoScope(prevFrame);
Expand Down

0 comments on commit c664f89

Please sign in to comment.