Fix improper yield self and implement new yield path in JIT.

Changes: * Remove self from being passed into yieldValues; let it be derived from Binding like other paths. * Implement new yieldValues path in JIT. Arities 2-3 bind to yieldSpecific and arities >3 construct an array and bind to yieldValues. * Disable indy yield. The handle manipulation or the test+guard appears to be slower than the indirect megamorphic path. Need to revisit this. * Make Array operand Iterable<Operand>.
jruby · Feb 7, 2016 · 7243b99 · 7243b99
1 parent db92b84
commit 7243b99
Showing 11 changed files with 179 additions and 58 deletions.
diff --git a/core/src/main/java/org/jruby/ir/instructions/YieldInstr.java b/core/src/main/java/org/jruby/ir/instructions/YieldInstr.java
@@ -77,7 +77,7 @@ public Object interpret(ThreadContext context, StaticScope currScope, DynamicSco
             if (unwrapArray && yieldOp instanceof Array && ((Array)yieldOp).size() > 1) {
                 // Special case this path!
                 // Don't build a RubyArray.
-                return blk.yieldValues(context, self, ((Array)yieldOp).retrieveArrayElts(context, self, currScope, currDynScope, temp));
+                return blk.yieldValues(context, ((Array)yieldOp).retrieveArrayElts(context, self, currScope, currDynScope, temp));
             } else {
                 IRubyObject yieldVal = (IRubyObject) yieldOp.retrieve(context, self, currScope, currDynScope, temp);
                 return IRRuntimeHelpers.yield(context, blk, yieldVal, unwrapArray);

diff --git a/core/src/main/java/org/jruby/ir/operands/Array.java b/core/src/main/java/org/jruby/ir/operands/Array.java
@@ -9,6 +9,8 @@
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;
 
+import java.util.Arrays;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
@@ -17,7 +19,7 @@
 // NOTE: This operand is only used in the initial stages of optimization.
 // Further down the line, this array operand could get converted to calls
 // that actually build a Ruby object
-public class Array extends Operand {
+public class Array extends Operand implements Iterable<Operand> {
     private final Operand[] elts;
 
     // SSS FIXME: Do we create a special-case for zero-length arrays?
@@ -129,4 +131,9 @@ public void visit(IRVisitor visitor) {
     public Operand[] getElts() {
         return elts;
     }
+
+    @Override
+    public Iterator<Operand> iterator() {
+        return Arrays.asList(elts).iterator();
+    }
 }
diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
@@ -478,6 +478,14 @@ public static IRubyObject yieldSpecific(ThreadContext context, Block b) {
         return b.yieldSpecific(context);
     }
 
+    public static IRubyObject yieldValues(ThreadContext context, Block b, IRubyObject arg0, IRubyObject arg1) {
+        return b.yieldValues(context, new IRubyObject[] {arg0, arg1});
+    }
+
+    public static IRubyObject yieldSpecific(ThreadContext context, Block b, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2) {
+        return b.yieldValues(context, new IRubyObject[]{arg0, arg1, arg2});
+    }
+
     public static IRubyObject[] convertValueIntoArgArray(ThreadContext context, IRubyObject value, int blockArity, boolean argIsArray) {
         // SSS FIXME: This should not really happen -- so, some places in the runtime library are breaking this contract.
         if (argIsArray && !(value instanceof RubyArray)) argIsArray = false;

diff --git a/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter.java b/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter.java
@@ -583,6 +583,13 @@ public org.objectweb.asm.Label newLabel() {
      */
     public abstract void yieldSpecific();
 
+    /**
+     * Yield a number of flat arguments to a block.
+     *
+     * Stack required: context, block
+     */
+    public abstract void yieldValues(int arity);
+
     /**
      * Prepare a block for a subsequent call.
      *

diff --git a/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter6.java b/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter6.java
@@ -827,6 +827,11 @@ public void yieldSpecific() {
         invokeIRHelper("yieldSpecific", sig(IRubyObject.class, ThreadContext.class, Block.class));
     }
 
+    @Override
+    public void yieldValues(int arity) {
+        invokeIRHelper("yieldValues", sig(IRubyObject.class, params(ThreadContext.class, Block.class, IRubyObject.class, arity)));
+    }
+
     @Override
     public void prepareBlock(Handle handle, org.jruby.runtime.Signature signature, String className) {
         // FIXME: too much bytecode

diff --git a/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter7.java b/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter7.java
@@ -313,6 +313,11 @@ public void yieldSpecific() {
         adapter.invokedynamic("yieldSpecific", sig(JVM.OBJECT, params(ThreadContext.class, Block.class)), YieldSite.BOOTSTRAP, 0);
     }
 
+    @Override
+    public void yieldValues(int arity) {
+        adapter.invokedynamic("yieldValues", sig(JVM.OBJECT, params(ThreadContext.class, Block.class, JVM.OBJECT, arity)), YieldSite.BOOTSTRAP, 0);
+    }
+
     @Override
     public void prepareBlock(Handle handle, org.jruby.runtime.Signature signature, String className) {
         Handle scopeHandle = new Handle(Opcodes.H_GETSTATIC, getClassData().clsName, handle.getName() + "_IRScope", ci(IRScope.class));

diff --git a/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java b/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
@@ -2017,8 +2017,17 @@ public void YieldInstr(YieldInstr yieldinstr) {
         if (yieldinstr.getYieldArg() == UndefinedValue.UNDEFINED) {
             jvmMethod().yieldSpecific();
         } else {
-            visit(yieldinstr.getYieldArg());
-            jvmMethod().yield(yieldinstr.isUnwrapArray());
+            Operand yieldOp = yieldinstr.getYieldArg();
+            if (yieldinstr.isUnwrapArray() && yieldOp instanceof Array && ((Array) yieldOp).size() > 1) {
+                Array yieldValues = (Array) yieldOp;
+                for (Operand yieldValue : yieldValues) {
+                    visit(yieldValue);
+                }
+                jvmMethod().yieldValues(yieldValues.size());
+            } else {
+                visit(yieldinstr.getYieldArg());
+                jvmMethod().yield(yieldinstr.isUnwrapArray());
+            }
         }
 
         jvmStoreLocal(yieldinstr.getResult());

diff --git a/core/src/main/java/org/jruby/ir/targets/YieldSite.java b/core/src/main/java/org/jruby/ir/targets/YieldSite.java
@@ -3,6 +3,7 @@
 import com.headius.invokebinder.Binder;
 import org.jruby.ir.runtime.IRRuntimeHelpers;
 import org.jruby.runtime.Block;
+import org.jruby.runtime.BlockBody;
 import org.jruby.runtime.CompiledIRBlockBody;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;
@@ -35,52 +36,98 @@ public YieldSite(MethodType type, boolean unwrap) {
     public static CallSite bootstrap(MethodHandles.Lookup lookup, String name, MethodType type, int unwrap) throws Throwable {
         YieldSite site = new YieldSite(type, unwrap == 1 ? true : false);
 
-        MethodHandle handle = Binder.from(type)
-                .prepend(YieldSite.class, site)
-                .invokeVirtual(lookup, name);
+        MethodHandle handle;
+        switch (name) {
+            case "yield":
+            case "yieldSpecific":
+                handle = Binder.from(type)
+                        .prepend(YieldSite.class, site)
+                        .invokeVirtual(lookup, name);
+                break;
+            case "yieldValues":
+                if (type.parameterCount() > 5) {
+                    handle = Binder.from(type)
+                            .collect(2, IRubyObject[].class)
+                            .prepend(YieldSite.class, site)
+                            .invokeVirtual(lookup, name);
+                } else {
+                    handle = Binder.from(type)
+                            .prepend(YieldSite.class, site)
+                            .invokeVirtual(lookup, name);
+                }
+                break;
+            default:
+                throw new RuntimeException("invalid yield type: " + name);
+        }
 
         site.setTarget(handle);
 
         return site;
     }
 
     public IRubyObject yield(ThreadContext context, Block block, IRubyObject arg) throws Throwable {
-        if (block.getBody() instanceof CompiledIRBlockBody) {
-            CompiledIRBlockBody compiledBody = (CompiledIRBlockBody) block.getBody();
-
-            MethodHandle target = unwrap ? compiledBody.getNormalYieldUnwrapHandle() : compiledBody.getNormalYieldHandle();
-            MethodHandle fallback = getTarget();
-            MethodHandle test = compiledBody.getTestBlockBody();
-
-            MethodHandle guard = MethodHandles.guardWithTest(test, target, fallback);
-
-            setTarget(guard);
-
-            return (IRubyObject)target.invokeExact(context, block, arg);
-        }
-
-        context.setCurrentBlockType(Block.Type.NORMAL);
-
-        return IRRuntimeHelpers.yield(context, block, arg, unwrap);
+//        BlockBody body = block.getBody();
+//        MethodHandle target;
+//
+//        if (block.getBody() instanceof CompiledIRBlockBody) {
+//            CompiledIRBlockBody compiledBody = (CompiledIRBlockBody) block.getBody();
+//
+//            target = unwrap ? compiledBody.getNormalYieldUnwrapHandle() : compiledBody.getNormalYieldHandle();
+//        } else {
+//            target = Binder.from(type())
+//                    .append(unwrap)
+//                    .invokeStaticQuiet(MethodHandles.lookup(), IRRuntimeHelpers.class, "yield");
+//        }
+//
+//        MethodHandle fallback = getTarget();
+//        MethodHandle test = body.getTestBlockBody();
+//
+//        MethodHandle guard = MethodHandles.guardWithTest(test, target, fallback);
+//
+//        setTarget(guard);
+//
+//        return (IRubyObject)target.invokeExact(context, block, arg);
+
+        // Fully MH-based dispatch for these still seems slower than megamorphic path
+        return block.yield(context, arg);
     }
 
     public IRubyObject yieldSpecific(ThreadContext context, Block block) throws Throwable {
-        if (block.getBody() instanceof CompiledIRBlockBody) {
-            CompiledIRBlockBody compiledBody = (CompiledIRBlockBody) block.getBody();
-
-            MethodHandle target = compiledBody.getNormalYieldSpecificHandle();
-            MethodHandle fallback = getTarget();
-            MethodHandle test = compiledBody.getTestBlockBody();
-
-            MethodHandle guard = MethodHandles.guardWithTest(test, target, fallback);
-
-            setTarget(guard);
+//        BlockBody body = block.getBody();
+//        MethodHandle target;
+//
+//        if (block.getBody() instanceof CompiledIRBlockBody) {
+//            CompiledIRBlockBody compiledBody = (CompiledIRBlockBody) block.getBody();
+//
+//            target = compiledBody.getNormalYieldSpecificHandle();
+//        } else {
+//            target = Binder.from(type())
+//                    .permute(0, 1)
+//                    .invokeVirtualQuiet(MethodHandles.lookup(), "yieldSpecific");
+//        }
+//
+//        MethodHandle fallback = getTarget();
+//        MethodHandle test = body.getTestBlockBody();
+//
+//        MethodHandle guard = MethodHandles.guardWithTest(test, target, fallback);
+//
+//        setTarget(guard);
+//
+//        return (IRubyObject)target.invokeExact(context, block);
+
+        // Fully MH-based dispatch for these still seems slower than megamorphic path
+        return block.yieldSpecific(context);
+    }
 
-            return (IRubyObject)target.invokeExact(context, block);
-        }
+    public IRubyObject yieldValues(ThreadContext context, Block block, IRubyObject arg0, IRubyObject arg1) {
+        return block.yieldSpecific(context, arg0, arg1);
+    }
 
-        context.setCurrentBlockType(Block.Type.NORMAL);
+    public IRubyObject yieldValues(ThreadContext context, Block block, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2) {
+        return block.yieldSpecific(context, arg0, arg1, arg2);
+    }
 
-        return IRRuntimeHelpers.yieldSpecific(context, block);
+    public IRubyObject yieldValues(ThreadContext context, Block block, IRubyObject[] args) {
+        return block.yieldValues(context, args);
     }
 }
diff --git a/core/src/main/java/org/jruby/runtime/Block.java b/core/src/main/java/org/jruby/runtime/Block.java
@@ -178,8 +178,8 @@ public IRubyObject yieldArray(ThreadContext context, IRubyObject value, IRubyObj
         return body.yield(context, this, args, self);
     }
 
-    public IRubyObject yieldValues(ThreadContext context, IRubyObject self, IRubyObject[] args) {
-        return body.yield(context, this, args, self);
+    public IRubyObject yieldValues(ThreadContext context, IRubyObject[] args) {
+        return body.yield(context, this, args, null);
     }
 
     public Block cloneBlock() {

diff --git a/core/src/main/java/org/jruby/runtime/BlockBody.java b/core/src/main/java/org/jruby/runtime/BlockBody.java
@@ -33,13 +33,17 @@
 
 package org.jruby.runtime;
 
+import com.headius.invokebinder.Binder;
 import org.jruby.EvalType;
 import org.jruby.RubyArray;
 import org.jruby.RubyProc;
 import org.jruby.ir.runtime.IRRuntimeHelpers;
 import org.jruby.parser.StaticScope;
 import org.jruby.runtime.builtin.IRubyObject;
 
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+
 /**
  * The executable body portion of a closure.
  */
@@ -48,6 +52,7 @@ public abstract class BlockBody {
     public static final String[] EMPTY_PARAMETER_LIST = org.jruby.util.StringSupport.EMPTY_STRING_ARRAY;
 
     protected final Signature signature;
+    protected volatile MethodHandle testBlockBody;
 
     public BlockBody(Signature signature) {
         this.signature = signature;
@@ -64,6 +69,18 @@ public boolean canCallDirect() {
         return false;
     }
 
+    public MethodHandle getTestBlockBody() {
+        if (testBlockBody != null) return testBlockBody;
+
+        return testBlockBody = Binder.from(boolean.class, ThreadContext.class, Block.class).drop(0).append(this).invoke(TEST_BLOCK_BODY);
+    }
+
+    private static final MethodHandle TEST_BLOCK_BODY = Binder.from(boolean.class, Block.class, BlockBody.class).invokeStaticQuiet(MethodHandles.lookup(), BlockBody.class, "testBlockBody");
+
+    public static boolean testBlockBody(Block block, BlockBody body) {
+        return block.getBody() == body;
+    }
+
     protected IRubyObject callDirect(ThreadContext context, Block block, IRubyObject[] args, Block blockArg) {
         throw new RuntimeException("callDirect not implemented in base class. We should never get here.");
     }

diff --git a/core/src/main/java/org/jruby/runtime/CompiledIRBlockBody.java b/core/src/main/java/org/jruby/runtime/CompiledIRBlockBody.java
@@ -3,6 +3,7 @@
 import com.headius.invokebinder.Binder;
 import org.jruby.ir.IRScope;
 import org.jruby.ir.runtime.IRRuntimeHelpers;
+import org.jruby.parser.StaticScope;
 import org.jruby.runtime.builtin.IRubyObject;
 
 import java.lang.invoke.MethodHandle;
@@ -13,7 +14,8 @@ public class CompiledIRBlockBody extends IRBlockBody {
     protected volatile MethodHandle normalYieldSpecificHandle;
     protected volatile MethodHandle normalYieldHandle;
     protected volatile MethodHandle normalYieldUnwrapHandle;
-    protected volatile MethodHandle testBlockBody;
+    protected volatile MethodHandle yieldTwoValuesHandle;
+    protected volatile MethodHandle yieldThreeValuesHandle;
 
     public CompiledIRBlockBody(MethodHandle handle, IRScope closure, long encodedSignature) {
         super(closure, Signature.decode(encodedSignature));
@@ -23,12 +25,6 @@ public CompiledIRBlockBody(MethodHandle handle, IRScope closure, long encodedSig
         closure.getStaticScope().determineModule();
     }
 
-    private static final MethodHandle TEST_BLOCK_BODY = Binder.from(boolean.class, Block.class, IRBlockBody.class).invokeStaticQuiet(MethodHandles.lookup(), CompiledIRBlockBody.class, "testBlockBody");
-
-    public static boolean testBlockBody(Block block, IRBlockBody body) {
-        return block.getBody() == body;
-    }
-
     private static final MethodHandle FOLD_METHOD1 = Binder.from(String.class, ThreadContext.class, Block.class).invokeStaticQuiet(MethodHandles.lookup(), CompiledIRBlockBody.class, "foldMethod");
     private static String foldMethod(ThreadContext context, Block block) {
         return block.getBinding().getMethod();
@@ -78,10 +74,8 @@ public MethodHandle getNormalYieldSpecificHandle() {
                 .foldVoid(SET_NORMAL)
                 .fold(FOLD_METHOD1)
                 .fold(FOLD_TYPE1)
-                .append(getStaticScope())
-                .append(IRubyObject.class, null)
-                .append(IRubyObject[].class, null)
-                .append(Block.class, Block.NULL_BLOCK)
+                .append(new Class[] {StaticScope.class, IRubyObject.class, IRubyObject[].class, Block.class},
+                        getStaticScope(), null, null, Block.NULL_BLOCK)
                 .permute(2, 3, 4, 5, 6, 7, 1, 0)
                 .invoke(handle);
     }
@@ -94,8 +88,7 @@ public MethodHandle getNormalYieldHandle() {
                 .fold(FOLD_METHOD2)
                 .fold(FOLD_TYPE2)
                 .filter(4, WRAP_VALUE)
-                .insert(4, getStaticScope())
-                .insert(5, IRubyObject.class, null)
+                .insert(4, new Class[]{StaticScope.class, IRubyObject.class}, getStaticScope(), null)
                 .append(Block.class, Block.NULL_BLOCK)
                 .permute(2, 3, 4, 5, 6, 7, 1, 0)
                 .invoke(handle);
@@ -109,17 +102,40 @@ public MethodHandle getNormalYieldUnwrapHandle() {
                 .fold(FOLD_METHOD2)
                 .fold(FOLD_TYPE2)
                 .filter(4, VALUE_TO_ARRAY)
-                .insert(4, getStaticScope())
-                .insert(5, IRubyObject.class, null)
+                .insert(4, new Class[] {StaticScope.class, IRubyObject.class}, getStaticScope(), null)
                 .append(Block.class, Block.NULL_BLOCK)
                 .permute(2, 3, 4, 5, 6, 7, 1, 0)
                 .invoke(handle);
     }
 
-    public MethodHandle getTestBlockBody() {
-        if (testBlockBody != null) return testBlockBody;
+    public MethodHandle getYieldTwoValuesHandle() {
+        if (yieldTwoValuesHandle != null) return yieldTwoValuesHandle;
 
-        return testBlockBody = Binder.from(boolean.class, ThreadContext.class, Block.class).drop(0).append(this).invoke(TEST_BLOCK_BODY);
+        return yieldTwoValuesHandle = Binder.from(IRubyObject.class, ThreadContext.class, Block.class, IRubyObject.class, IRubyObject.class)
+                .foldVoid(SET_NORMAL)
+                .fold(FOLD_METHOD1)
+                .fold(FOLD_TYPE1)
+                .collect(5, IRubyObject[].class)
+                .insert(5, new Class[] {StaticScope.class, IRubyObject.class},
+                        getStaticScope(), null)
+                .append(new Class[] {Block.class}, Block.NULL_BLOCK)
+                .permute(2, 3, 4, 5, 6, 7, 1, 0)
+                .invoke(handle);
+    }
+
+    public MethodHandle getYieldThreeValuesHandle() {
+        if (yieldThreeValuesHandle != null) return yieldThreeValuesHandle;
+
+        return yieldThreeValuesHandle = Binder.from(IRubyObject.class, ThreadContext.class, Block.class, IRubyObject.class, IRubyObject.class, IRubyObject.class)
+                .foldVoid(SET_NORMAL)
+                .fold(FOLD_METHOD1)
+                .fold(FOLD_TYPE1)
+                .collect(5, IRubyObject[].class)
+                .insert(5, new Class[] {StaticScope.class, IRubyObject.class},
+                        getStaticScope(), null)
+                .append(new Class[] {Block.class}, Block.NULL_BLOCK)
+                .permute(2, 3, 4, 5, 6, 7, 1, 0)
+                .invoke(handle);
     }
 
     @Override