Special handling for yields with multiple arguments

* Inline the yield array into the instruction. * If the unwrap boolean flag is true, there is no need to construct a ruby array only to destroy a short while after in Block.java to pass it to the block body. * In this patch, I haven't updated the JIT to take advantage of the changes here, but should be easy to replicate based on changes to interpretation. The JIT also has the opportunity to go one step further and eliminate the build of the java array as well.
jruby · Feb 6, 2016 · dc0b53f · dc0b53f
1 parent c175d53
commit dc0b53f
Showing 4 changed files with 30 additions and 11 deletions.
diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -378,7 +378,7 @@ private Operand buildOperand(Node node) throws NotCompilableException {
             case ANDNODE: return buildAnd((AndNode) node);
             case ARGSCATNODE: return buildArgsCat((ArgsCatNode) node);
             case ARGSPUSHNODE: return buildArgsPush((ArgsPushNode) node);
-            case ARRAYNODE: return buildArray((ArrayNode) node);
+            case ARRAYNODE: return buildArray((ArrayNode) node, false);
             case ATTRASSIGNNODE: return buildAttrAssign((AttrAssignNode) node);
             case BACKREFNODE: return buildBackref((BackRefNode) node);
             case BEGINNODE: return buildBegin((BeginNode) node);
@@ -813,15 +813,16 @@ public Operand buildAnd(final AndNode andNode) {
         }
     }
 
-    public Operand buildArray(ArrayNode node) {
+    public Operand buildArray(ArrayNode node, boolean operandOnly) {
         Node[] nodes = node.children();
         Operand[] elts = new Operand[nodes.length];
         boolean containsAssignments = node.containsVariableAssignment();
         for (int i = 0; i < nodes.length; i++) {
             elts[i] = buildWithOrder(nodes[i], containsAssignments);
         }
 
-        return copyAndReturnValue(new Array(elts));
+        Operand array = new Array(elts);
+        return operandOnly ? array : copyAndReturnValue(array);
     }
 
     public Operand buildArgsCat(final ArgsCatNode argsCatNode) {
@@ -3464,7 +3465,11 @@ public Operand buildYield(YieldNode node) {
         }
 
         Variable ret = createTemporaryVariable();
-        addInstr(new YieldInstr(ret, scope.getYieldClosureVariable(), build(argNode), unwrap));
+        if (argNode instanceof ArrayNode && unwrap) {
+            addInstr(new YieldInstr(ret, scope.getYieldClosureVariable(), buildArray((ArrayNode)argNode, true), unwrap));
+        } else {
+            addInstr(new YieldInstr(ret, scope.getYieldClosureVariable(), build(argNode), unwrap));
+        }
         return ret;
     }
 

diff --git a/core/src/main/java/org/jruby/ir/instructions/YieldInstr.java b/core/src/main/java/org/jruby/ir/instructions/YieldInstr.java
@@ -3,6 +3,7 @@
 import org.jruby.ir.IRVisitor;
 import org.jruby.ir.Interp;
 import org.jruby.ir.Operation;
+import org.jruby.ir.operands.Array;
 import org.jruby.ir.operands.Operand;
 import org.jruby.ir.operands.UndefinedValue;
 import org.jruby.ir.operands.Variable;
@@ -72,8 +73,15 @@ public Object interpret(ThreadContext context, StaticScope currScope, DynamicSco
         if (getYieldArg() == UndefinedValue.UNDEFINED) {
             return IRRuntimeHelpers.yieldSpecific(context, blk);
         } else {
-            IRubyObject yieldVal = (IRubyObject) getYieldArg().retrieve(context, self, currScope, currDynScope, temp);
-            return IRRuntimeHelpers.yield(context, blk, yieldVal, unwrapArray);
+            Operand yieldOp = getYieldArg();
+            if (unwrapArray && yieldOp instanceof Array && ((Array)yieldOp).size() > 1) {
+                // Special case this path!
+                // Don't build a RubyArray.
+                return blk.yieldArray(context, self, ((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
@@ -107,15 +107,18 @@ public static Array decode(IRReaderDecoder d) {
         return new Array(d.decodeOperandArray());
     }
 
-    @Override
-    public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
+    public IRubyObject[] retrieveArrayElts(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
         IRubyObject[] elements = new IRubyObject[elts.length];
 
         for (int i = 0; i < elements.length; i++) {
             elements[i] = (IRubyObject) elts[i].retrieve(context, self, currScope, currDynScope, temp);
         }
+        return elements;
+    }
 
-        return context.runtime.newArray(elements);
+    @Override
+    public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
+        return context.runtime.newArray(retrieveArrayElts(context, self, currScope, currDynScope, temp));
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/runtime/Block.java b/core/src/main/java/org/jruby/runtime/Block.java
@@ -174,8 +174,11 @@ public IRubyObject yieldArray(ThreadContext context, IRubyObject value, IRubyObj
         // SSS FIXME: Later on, we can move this code into IR insructions or
         // introduce a specialized entry-point when we know that this block has
         // explicit call protocol IR instructions.
-        IRubyObject[] args;
-        args = IRRuntimeHelpers.singleBlockArgToArray(value);
+        IRubyObject[] args = IRRuntimeHelpers.singleBlockArgToArray(value);
+        return body.yield(context, this, args, self);
+    }
+
+    public IRubyObject yieldArray(ThreadContext context, IRubyObject self, IRubyObject[] args) {
         return body.yield(context, this, args, self);
     }