TL;DR define_method defined methods will execute ~2x faster in common…

… cases. This optimization will convert define_method from being a block and fully promoting it to a method (only for JIT now). Once we unify InterpretedIRMethod and MixedModeIRMethod we can enable this for full builds in -X-C as well. This ended up have more issues than anticipated. block_given? in particular refers to the block outside the define_method block and not to any block passed to that newly defined method. This required the creation of a new Method type DefineMethodMethod. The biggest limitation to this optimization is that if the block for define_method references any variables in a parent scope then we will not perform the optimization. This is mildly undesirable and for read-only lvar access I think we can hoist the value from the parent scope and pass it into our converted method. This seemed like a ton of extra work so it is a blue sky future. There will be a future set of commit(s) to make this optimization trigger more often. Right now if the new method has contained closures which acceess lvars outside of their current scope then we do not perform this opt. The next round of optimization will involve calculating whether nested closures can escape the define_method only. Before: Calculating ------------------------------------- define_method w/ capture 56.284k i/100ms def 104.909k i/100ms define_method 63.224k i/100ms ------------------------------------------------- define_method w/ capture 1.115M (± 5.8%) i/s - 5.572M def 3.409M (± 9.3%) i/s - 16.890M define_method 1.086M (± 4.8%) i/s - 5.437M After: system ~/work/jruby master * 1412% jruby ../snippets/define_method3.rb Calculating ------------------------------------- define_method w/ capture 58.167k i/100ms def 108.644k i/100ms define_method 92.163k i/100ms ------------------------------------------------- define_method w/ capture 1.171M (± 6.4%) i/s - 5.875M def 3.241M (± 8.1%) i/s - 16.079M define_method 2.795M (± 8.7%) i/s - 13.917M MRI: system ~/work/jruby master * 1408% mri22 ../snippets/define_method3.rb Calculating ------------------------------------- define_method w/ capture 46.709k i/100ms def 63.690k i/100ms define_method 46.901k i/100ms ------------------------------------------------- define_method w/ capture 832.886k (± 6.4%) i/s - 4.157M def 1.732M (± 7.8%) i/s - 8.662M define_method 856.637k (± 6.6%) i/s - 4.268M
jruby · Oct 6, 2015 · 3c3055d · 3c3055d
1 parent dffe5aa
commit 3c3055d
Showing 11 changed files with 178 additions and 13 deletions.
diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -53,14 +53,17 @@
 import org.jruby.internal.runtime.methods.AttrWriterMethod;
 import org.jruby.internal.runtime.methods.CacheableMethod;
 import org.jruby.internal.runtime.methods.CallConfiguration;
+import org.jruby.internal.runtime.methods.DefineMethodMethod;
 import org.jruby.internal.runtime.methods.DynamicMethod;
 import org.jruby.internal.runtime.methods.Framing;
 import org.jruby.internal.runtime.methods.JavaMethod;
+import org.jruby.internal.runtime.methods.MixedModeIRMethod;
 import org.jruby.internal.runtime.methods.ProcMethod;
 import org.jruby.internal.runtime.methods.Scoping;
 import org.jruby.internal.runtime.methods.SynchronizedDynamicMethod;
 import org.jruby.internal.runtime.methods.UndefinedMethod;
 import org.jruby.internal.runtime.methods.WrapperMethod;
+import org.jruby.ir.IRClosure;
 import org.jruby.ir.IRMethod;
 import org.jruby.parser.StaticScope;
 import org.jruby.runtime.Arity;
@@ -1788,6 +1791,22 @@ public IRubyObject define_method(ThreadContext context, IRubyObject arg0, Block
             throw getRuntime().newArgumentError("tried to create Proc object without a block");
         }
 
+        // If we know it comes from IR we can convert this directly to a method and
+        // avoid overhead of invoking it as a block
+        if (block.getBody() instanceof IRBlockBody &&
+                runtime.getInstanceConfig().getCompileMode().shouldJIT()) { // FIXME: Once Interp and Mixed Methods are one class we can fix this to work in interp mode too.
+            IRBlockBody body = (IRBlockBody) block.getBody();
+            IRClosure closure = body.getScope();
+
+            // Ask closure to give us a method equivalent.
+            IRMethod method = closure.convertToMethod(name);
+            if (method != null) {
+                newMethod = new DefineMethodMethod(method, visibility, this, context.getFrameBlock());
+                Helpers.addInstanceMethod(this, name, newMethod, visibility, context, runtime);
+                return nameSym;
+            }
+        }
+
         block = block.cloneBlockAndFrame();
         RubyProc proc = runtime.newProc(Block.Type.LAMBDA, block);
 

diff --git a/core/src/main/java/org/jruby/ast/DefNode.java b/core/src/main/java/org/jruby/ast/DefNode.java
@@ -0,0 +1,28 @@
+package org.jruby.ast;
+
+import org.jruby.parser.StaticScope;
+
+/**
+ * Methods and blocks both implement these.
+ */
+public interface DefNode {
+    /**
+     * Gets the argsNode.
+     * @return Returns a Node
+     */
+    ArgsNode getArgsNode();
+
+    /**
+     * Get the static scoping information.
+     *
+     * @return the scoping info
+     */
+    StaticScope getScope();
+
+    /**
+     * Gets the body of this class.
+     *
+     * @return the contents
+     */
+    Node getBodyNode();
+}
diff --git a/core/src/main/java/org/jruby/ast/ForNode.java b/core/src/main/java/org/jruby/ast/ForNode.java
@@ -59,6 +59,10 @@ public ForNode(ISourcePosition position, Node varNode, Node bodyNode, Node iterN
         this.iterNode = iterNode;
     }
 
+    public ArgsNode getArgsNode() {
+        throw new IllegalArgumentException("For nodes are not technically def nodes so they do not have args");
+    }
+
     @Override
     public NodeType getNodeType() {
         return NodeType.FORNODE;

diff --git a/core/src/main/java/org/jruby/ast/IterNode.java b/core/src/main/java/org/jruby/ast/IterNode.java
@@ -36,12 +36,11 @@
 import org.jruby.ast.visitor.NodeVisitor;
 import org.jruby.lexer.yacc.ISourcePosition;
 import org.jruby.parser.StaticScope;
-import org.jruby.runtime.BlockBody;
 
 /**
  * Represents a block.  
  */
-public class IterNode extends Node {
+public class IterNode extends Node implements DefNode {
     private final Node varNode;
     private final Node bodyNode;
 
@@ -83,6 +82,11 @@ public <T> T accept(NodeVisitor<T> iVisitor) {
         return iVisitor.visitIterNode(this);
     }
 
+    @Override
+    public ArgsNode getArgsNode() {
+        return (ArgsNode) varNode;
+    }
+
     public StaticScope getScope() {
         return scope;
     }

diff --git a/core/src/main/java/org/jruby/ast/MethodDefNode.java b/core/src/main/java/org/jruby/ast/MethodDefNode.java
@@ -35,7 +35,7 @@
 import org.jruby.lexer.yacc.ISourcePosition;
 import org.jruby.parser.StaticScope;
 
-public abstract class MethodDefNode extends Node implements INameNode {
+public abstract class MethodDefNode extends Node implements INameNode, DefNode {
 	protected final String name;
 	protected final ArgsNode argsNode;
 	protected final StaticScope scope;
@@ -64,7 +64,7 @@ public ArgsNode getArgsNode() {
 
 	/**
 	 * Get the static scoping information.
-	 * 
+	 *
 	 * @return the scoping info
 	 */
 	public StaticScope getScope() {
@@ -73,7 +73,7 @@ public StaticScope getScope() {
 
 	/**
 	 * Gets the body of this class.
-	 * 
+	 *
 	 * @return the contents
 	 */
 	public Node getBodyNode() {

diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/DefineMethodMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/DefineMethodMethod.java
@@ -0,0 +1,49 @@
+package org.jruby.internal.runtime.methods;
+
+import org.jruby.RubyModule;
+import org.jruby.ir.IRScope;
+import org.jruby.runtime.Block;
+import org.jruby.runtime.ThreadContext;
+import org.jruby.runtime.Visibility;
+import org.jruby.runtime.builtin.IRubyObject;
+
+/**
+ * Supports optimization for define_method.  block that defined_method sees happens to be the block
+ * passed to the scope where the define_method originally was called...not the block potentially passed to
+ * the newly defined method.
+ */
+public class DefineMethodMethod extends MixedModeIRMethod {
+
+    private Block capturedBlock;
+
+    public DefineMethodMethod(IRScope method, Visibility visibility, RubyModule implementationClass, Block capturedBlock) {
+        super(method, visibility, implementationClass);
+
+        this.capturedBlock = capturedBlock;
+    }
+
+    @Override
+    public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
+        return super.call(context, self, clazz, name, args, capturedBlock);
+    }
+
+    @Override
+    public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, Block block) {
+        return super.call(context, self, clazz, name, capturedBlock);
+    }
+
+    @Override
+    public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject arg0, Block block) {
+        return super.call(context, self, clazz, name, arg0, capturedBlock);
+    }
+
+    @Override
+    public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject arg0, IRubyObject arg1, Block block) {
+        return super.call(context, self, clazz, name, arg0, arg1, capturedBlock);
+    }
+
+    @Override
+    public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2, Block block) {
+        return super.call(context, self, clazz, name, arg0, arg1, arg2, capturedBlock);
+    }
+}
diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -1683,7 +1683,7 @@ public Operand buildDAsgn(final DAsgnNode dasgnNode) {
 
     // Called by defineMethod but called on a new builder so things like ensure block info recording
     // do not get confused.
-    protected InterpreterContext defineMethodInner(MethodDefNode defNode, IRScope parent) {
+    protected InterpreterContext defineMethodInner(DefNode defNode, IRScope parent) {
         if (RubyInstanceConfig.FULL_TRACE_ENABLED) {
             addInstr(new TraceInstr(RubyEvent.CALL, getName(), getFileName(), scope.getLineNumber()));
         }
@@ -2309,6 +2309,18 @@ public Operand buildFCall(FCallNode fcallNode) {
 
         determineIfMaybeUsingMethod(fcallNode.getName(), args);
 
+        // We will stuff away the iters AST source into the closure in the hope we can convert
+        // this closure to a method.
+        if (fcallNode.getName().equals("define_method") && block instanceof WrappedIRClosure) {
+            IRClosure closure = ((WrappedIRClosure) block).getClosure();
+
+            // To convert to a method we need its variable scoping to appear like a normal method.
+            if (!closure.getFlags().contains(IRFlags.ACCESS_PARENTS_LOCAL_VARIABLES) &&
+                    fcallNode.getIterNode() instanceof IterNode) {
+                closure.setSource((IterNode) fcallNode.getIterNode());
+            }
+        }
+
         CallInstr callInstr    = CallInstr.create(scope, CallType.FUNCTIONAL, callResult, fcallNode.getName(), buildSelf(), args, block);
         receiveBreakException(block, callInstr);
         return callResult;

diff --git a/core/src/main/java/org/jruby/ir/IRClosure.java b/core/src/main/java/org/jruby/ir/IRClosure.java
@@ -3,19 +3,18 @@
 import java.util.ArrayList;
 import java.util.List;
 
-import org.jruby.RubyInstanceConfig;
+import org.jruby.ast.DefNode;
+import org.jruby.ast.IterNode;
 import org.jruby.ir.instructions.*;
 import org.jruby.ir.interpreter.ClosureInterpreterContext;
 import org.jruby.ir.interpreter.InterpreterContext;
 import org.jruby.ir.operands.*;
-import org.jruby.ir.representations.BasicBlock;
 import org.jruby.ir.transformations.inlining.CloneInfo;
 import org.jruby.ir.transformations.inlining.SimpleCloneInfo;
 import org.jruby.parser.StaticScope;
 import org.jruby.runtime.ArgumentDescriptor;
 import org.jruby.runtime.BlockBody;
 import org.jruby.runtime.IRBlockBody;
-import org.jruby.runtime.InterpretedIRBlockBody;
 import org.jruby.runtime.MixedModeIRBlockBody;
 import org.jruby.runtime.Signature;
 import org.objectweb.asm.Handle;
@@ -33,6 +32,10 @@ public class IRClosure extends IRScope {
 
     private Signature signature;
 
+    // We allow closures who happen to be assigned to calls named 'defined_method' to save the original
+    // AST so we can attempt to convert those blocks to full methods.
+    private IterNode source;
+
     // Argument description
     protected ArgumentDescriptor[] argDesc = ArgumentDescriptor.EMPTY_ARRAY;
 
@@ -70,6 +73,7 @@ protected IRClosure(IRClosure c, IRScope lexicalParent, int closureId, String fu
         this.signature = c.signature;
     }
 
+    // Used by iter + lambda by IRBuilder
     public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, StaticScope staticScope, Signature signature) {
         this(manager, lexicalParent, lineNumber, staticScope, signature, "_CLOSURE_");
     }
@@ -162,14 +166,50 @@ public BlockBody getBlockBody() {
         return body;
     }
 
+    // FIXME: This is too strict.  We can use any closure which does not dip below the define_method closure.  This
+    // will deopt any nested block which dips out of itself.
+    public boolean isNestedClosuresSafeForMethodConversion() {
+        for (IRClosure closure: getClosures()) {
+            if (!closure.isNestedClosuresSafeForMethodConversion()) return false;
+        }
+
+        return !getFlags().contains(IRFlags.ACCESS_PARENTS_LOCAL_VARIABLES);
+    }
+
+    public IRMethod convertToMethod(String name) {
+        // We want variable scoping to be the same as a method and not see outside itself.
+        if (source == null ||
+            getFlags().contains(IRFlags.ACCESS_PARENTS_LOCAL_VARIABLES) ||  // Built methods cannot search down past method scope
+            getFlags().contains(IRFlags.RECEIVES_CLOSURE_ARG) ||            // we pass in captured block at define_method as block so explicits ones not supported
+            !isNestedClosuresSafeForMethodConversion()) {
+            source = null;
+            return null;
+        }
+
+        DefNode def = source;
+        source = null;
+
+        return new IRMethod(getManager(), getLexicalParent(), def, name, true,  getLineNumber(), getStaticScope());
+    }
+
+    public void setSource(IterNode iter) {
+        source = iter;
+    }
+
     @Override
     protected LocalVariable findExistingLocalVariable(String name, int scopeDepth) {
         LocalVariable lvar = lookupExistingLVar(name);
         if (lvar != null) return lvar;
 
         int newDepth = scopeDepth - 1;
 
-        return newDepth >= 0 ? getLexicalParent().findExistingLocalVariable(name, newDepth) : null;
+        if (newDepth >= 0) {
+            lvar = getLexicalParent().findExistingLocalVariable(name, newDepth);
+
+            if (lvar != null) flags.add(IRFlags.ACCESS_PARENTS_LOCAL_VARIABLES);
+        }
+
+        return lvar;
     }
 
     public LocalVariable getNewLocalVariable(String name, int depth) {
@@ -178,6 +218,8 @@ public LocalVariable getNewLocalVariable(String name, int depth) {
             localVars.put(name, lvar);
             return lvar;
         } else {
+            // IRFor does not have it's own state
+            if (!(this instanceof IRFor)) flags.add(IRFlags.ACCESS_PARENTS_LOCAL_VARIABLES);
             IRScope s = this;
             int     d = depth;
             do {
@@ -211,6 +253,8 @@ public LocalVariable getLocalVariable(String name, int depth) {
         LocalVariable lvar;
         IRScope s = this;
         int d = depth;
+        if (depth > 0 && !(this instanceof IRFor)) flags.add(IRFlags.ACCESS_PARENTS_LOCAL_VARIABLES);
+
         do {
             // account for for-loops
             while (s instanceof IRFor) {

diff --git a/core/src/main/java/org/jruby/ir/IRFlags.java b/core/src/main/java/org/jruby/ir/IRFlags.java
@@ -36,6 +36,7 @@ public enum IRFlags {
     //
     // This logic was extracted from an email thread on the JRuby mailing list -- Yehuda Katz & Charles Nutter
     // contributed this analysis above.
+    ACCESS_PARENTS_LOCAL_VARIABLES, // Closure is capturing parent(s) variable state
     CAN_CAPTURE_CALLERS_BINDING,
     CAN_RECEIVE_BREAKS,           // may receive a break during execution
     CAN_RECEIVE_NONLOCAL_RETURNS, // may receive a non-local return during execution

diff --git a/core/src/main/java/org/jruby/ir/IRMethod.java b/core/src/main/java/org/jruby/ir/IRMethod.java
@@ -1,6 +1,6 @@
 package org.jruby.ir;
 
-import org.jruby.ast.MethodDefNode;
+import org.jruby.ast.DefNode;
 import org.jruby.ir.interpreter.InterpreterContext;
 import org.jruby.ir.operands.LocalVariable;
 import org.jruby.ir.representations.BasicBlock;
@@ -13,9 +13,9 @@ public class IRMethod extends IRScope {
     // Argument description
     protected ArgumentDescriptor[] argDesc = ArgumentDescriptor.EMPTY_ARRAY;
 
-    private MethodDefNode defn;
+    private DefNode defn;
 
-    public IRMethod(IRManager manager, IRScope lexicalParent, MethodDefNode defn, String name,
+    public IRMethod(IRManager manager, IRScope lexicalParent, DefNode defn, String name,
             boolean isInstanceMethod, int lineNumber, StaticScope staticScope) {
         super(manager, lexicalParent, name, lineNumber, staticScope);
 

diff --git a/core/src/main/java/org/jruby/runtime/IRBlockBody.java b/core/src/main/java/org/jruby/runtime/IRBlockBody.java
@@ -156,6 +156,10 @@ protected IRubyObject useBindingSelf(Binding binding) {
 
     protected abstract IRubyObject commonYieldPath(ThreadContext context, IRubyObject[] args, IRubyObject self, Binding binding, Type type, Block block);
 
+    public IRClosure getScope() {
+        return closure;
+    }
+
     @Override
     public String getFile() {
         return fileName;