New {Closure,}InterpreterContext with all state interpreter needs to …

…execute. This should eliminate nearly all contention between JIT and intepreter. There is still one remaining bug involving label operands cloning when we don't want them to so instr cloning (last bit) is disabled atm.
jruby · Oct 14, 2014 · 13cd181 · 13cd181
1 parent 4a5d818
commit 13cd181
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 30 deletions.
diff --git a/core/src/main/java/org/jruby/ir/IRClosure.java b/core/src/main/java/org/jruby/ir/IRClosure.java
@@ -109,6 +109,21 @@ public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, Stati
         this.nestingDepth++;
     }
 
+    public InterpreterContext getInterpreterContext(Operand self) {
+        initScope(false);
+
+        checkRelinearization();
+
+        if (interpreterContext != null) return interpreterContext; // Already prepared
+
+        Instr[] linearizedInstrArray = prepareInstructions();
+        interpreterContext = new ClosureInterpreterContext(getTemporaryVariablesCount(), getBooleanVariablesCount(),
+                getFixnumVariablesCount(), getFloatVariablesCount(),getFlags().clone(), linearizedInstrArray,
+                self, getStaticScope(), getBlockBody());
+
+        return interpreterContext;
+    }
+
     public void setBeginEndBlock() {
         this.isBeginEndBlock = true;
     }

diff --git a/core/src/main/java/org/jruby/ir/IRScope.java b/core/src/main/java/org/jruby/ir/IRScope.java
@@ -109,7 +109,7 @@ public abstract class IRScope implements ParseResult {
     private List<CompilerPass> executedPasses;
 
     /** What the interpreter depends on to interpret this IRScope */
-    private InterpreterContext interpreterContext;
+    protected InterpreterContext interpreterContext;
     private List<BasicBlock> linearizedBBList;
     protected int temporaryVariableIndex;
     protected int floatVariableIndex;
@@ -467,50 +467,50 @@ public CFG getCFG() {
         return cfg;
     }
 
-    private synchronized InterpreterContext prepareInstructions() {
+    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();
 
         SimpleCloneInfo cloneInfo = new SimpleCloneInfo(this, false);
-        // Clear old set
-        initNestedClosures();
 
         // FIXME: If CFG (or linearizedBBList) knew number of instrs we could end up allocing better
 
         // Pass 1. Set up IPCs for labels and instructions and build linear instr list
-        List<Instr> newInstrs = new ArrayList<Instr>();
+        List<Instr> newInstrs = new ArrayList<>();
         int ipc = 0;
         for (BasicBlock b: linearizedBBList) {
             // All same-named labels must be same Java instance for this to work or we would need
             // to examine all Label operands and update this as well which would be expensive.
-            Label l = b.getLabel();
-            Label newL = l;//cloneInfo.getRenamedLabel(l);
-            l.setTargetPC(ipc);
-            newL.setTargetPC(ipc);
+            b.getLabel().setTargetPC(ipc);
 
             List<Instr> bbInstrs = b.getInstrs();
             int bbInstrsLength = bbInstrs.size();
             for (int i = 0; i < bbInstrsLength; i++) {
                 Instr instr = bbInstrs.get(i);
                 if (!(instr instanceof ReceiveSelfInstr)) {
-                    Instr newInstr = instr;//.clone(cloneInfo);
-                    // if (newInstr == instr) {
-                    //     System.out.println("Instruction " + instr.getOperation() + " returns itself on clone. Probably fragile!");
-                    // }
-
-                    if (newInstr instanceof Specializeable) {
-                        newInstr = ((Specializeable) newInstr).specializeForInterpretation();
-                        // Make sure debug CFG is identical to interpreted instr output
-                        if (IRRuntimeHelpers.isDebug()) {
-                            bbInstrs.set(i, ((Specializeable) instr).specializeForInterpretation());
-                        }
+                    if (instr instanceof Specializeable) {
+                        instr = ((Specializeable) instr).specializeForInterpretation();
                     }
 
-                    newInstrs.add(newInstr);
+                    Instr newInstr = instr;//.clone(cloneInfo);
                     newInstr.setIPC(ipc);
+
+                    // We add back to original CFG so that debug output will match up with what
+                    // we saved to interpreter.  This knowledge may get changed if we are interp'ing
+                    // with debug output while JIT is running (FIXME?)
+                    bbInstrs.set(i, newInstr);
+                    newInstrs.add(newInstr);
                     ipc++;
                 }
             }
@@ -527,7 +527,7 @@ private synchronized InterpreterContext prepareInstructions() {
         ipc = 0;
         for (BasicBlock b : linearizedBBList) {
             BasicBlock rescuerBB = cfg().getRescuerBBFor(b);
-            int rescuerPC = (rescuerBB == null) ? -1 : rescuerBB.getLabel().getTargetPC();
+            int rescuerPC = rescuerBB == null ? -1 : rescuerBB.getLabel().getTargetPC();
             for (Instr instr : b.getInstrs()) {
                 // FIXME: If we did not omit instrs from previous pass we could end up just doing a
                 // a size and for loop this n times instead of walking an examining each instr
@@ -538,10 +538,7 @@ private synchronized InterpreterContext prepareInstructions() {
             }
         }
 
-        interpreterContext = new InterpreterContext(getTemporaryVariablesCount(), getBooleanVariablesCount(),
-                getFixnumVariablesCount(), getFloatVariablesCount(),getFlags().clone(), linearizedInstrArray);
-
-        return interpreterContext;
+        return linearizedInstrArray;
     }
 
     private boolean isUnsafeScope() {
@@ -609,7 +606,7 @@ private void optimizeSimpleScopes() {
         }
     }
 
-    private void initScope(boolean jitMode) {
+    protected void initScope(boolean jitMode) {
         // Reset linearization, if any exists
         resetLinearizationData();
 
@@ -639,7 +636,7 @@ public synchronized InterpreterContext prepareForInterpretation() {
         // System.out.println("-- passes run for: " + this + " = " + java.util.Arrays.toString(executedPasses.toArray()));
 
         // Linearize CFG, etc.
-        return prepareInstructions();
+        return prepareInterpreterContext();
     }
 
     /* SSS FIXME: Do we need to synchronize on this?  Cache this info in a scope field? */

diff --git a/core/src/main/java/org/jruby/ir/operands/ClosureInterpreterContext.java b/core/src/main/java/org/jruby/ir/operands/ClosureInterpreterContext.java
@@ -0,0 +1,47 @@
+package org.jruby.ir.operands;
+
+import java.util.EnumSet;
+import org.jruby.ir.IRFlags;
+import org.jruby.ir.instructions.Instr;
+import org.jruby.ir.transformations.inlining.CloneInfo;
+import org.jruby.parser.StaticScope;
+import org.jruby.runtime.Binding;
+import org.jruby.runtime.Block;
+import org.jruby.runtime.BlockBody;
+import org.jruby.runtime.DynamicScope;
+import org.jruby.runtime.ThreadContext;
+import org.jruby.runtime.builtin.IRubyObject;
+
+/**
+ * Created by enebo on 10/14/14.
+ */
+public class ClosureInterpreterContext extends InterpreterContext {
+    private Operand self;
+    private StaticScope staticScope;
+    private BlockBody body;
+
+    public ClosureInterpreterContext(int temporaryVariablecount, int temporaryBooleanVariablecount,
+                                     int temporaryFixnumVariablecount, int temporaryFloatVariablecount,
+                                     EnumSet<IRFlags> flags, Instr[] instructions,
+                                     Operand self, StaticScope staticScope, BlockBody body) {
+        super(temporaryVariablecount, temporaryBooleanVariablecount, temporaryFixnumVariablecount,
+                temporaryFloatVariablecount, flags, instructions);
+
+        this.self = self;
+        this.staticScope = staticScope;
+        this.body = body;
+    }
+
+    @Override
+    public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
+        staticScope.determineModule();
+
+        // In non-inlining scenarios, this.self will always be %self.
+        // However, in inlined scenarios, this.self will be the self in the original scope where the closure
+        // was present before inlining.
+        IRubyObject selfVal = (this.self instanceof Self) ? self : (IRubyObject) this.self.retrieve(context, self, currScope, currDynScope, temp);
+        Binding binding = context.currentBinding(selfVal, currDynScope);
+
+        return new Block(body, binding);
+    }
+}
diff --git a/core/src/main/java/org/jruby/ir/operands/InterpreterContext.java b/core/src/main/java/org/jruby/ir/operands/InterpreterContext.java
@@ -5,6 +5,10 @@
 import org.jruby.ir.IRFlags;
 import org.jruby.ir.instructions.Instr;
 import org.jruby.ir.transformations.inlining.CloneInfo;
+import org.jruby.parser.StaticScope;
+import org.jruby.runtime.DynamicScope;
+import org.jruby.runtime.ThreadContext;
+import org.jruby.runtime.builtin.IRubyObject;
 
 /**
  *
@@ -64,4 +68,9 @@ public EnumSet<IRFlags> getFlags() {
     public Instr[] getInstructions() {
         return instructions;
     }
+
+    @Override
+    public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
+        return super.retrieve(context, self, currScope, currDynScope, temp);
+    }
 }
diff --git a/core/src/main/java/org/jruby/ir/operands/WrappedIRClosure.java b/core/src/main/java/org/jruby/ir/operands/WrappedIRClosure.java
@@ -3,6 +3,7 @@
 import org.jruby.ir.IRClosure;
 import org.jruby.ir.IRVisitor;
 import org.jruby.ir.transformations.inlining.CloneInfo;
+import org.jruby.ir.transformations.inlining.SimpleCloneInfo;
 import org.jruby.parser.StaticScope;
 import org.jruby.runtime.*;
 import org.jruby.runtime.builtin.IRubyObject;
@@ -51,8 +52,21 @@ public Operand getSimplifiedOperand(Map<Operand, Operand> valueMap, boolean forc
     }
 
     @Override
-    public Operand cloneForInlining(CloneInfo ii) {
-        return new WrappedIRClosure(ii.getRenamedVariable(self), closure.cloneForInlining(ii));
+    public Operand cloneForInlining(CloneInfo info) {
+        if (info instanceof SimpleCloneInfo) {
+            SimpleCloneInfo simpleCloneInfo = (SimpleCloneInfo) info;
+
+            // For IRBuilding we are pre-interpretation and do want a traditional simple clone
+            if (simpleCloneInfo.isEnsureBlockCloneMode()) {
+                return new WrappedIRClosure(info.getRenamedVariable(self), closure.cloneForInlining(info));
+            }
+
+            // We are saving instructions so that if JIT hits IRClosure it will not concurrently
+            // modify the same object.
+            return closure.getInterpreterContext(self);
+        }
+
+        return new WrappedIRClosure(info.getRenamedVariable(self), closure.cloneForInlining(info));
     }
 
     @Override