Remove IRMethod.callArgs+keywordArgs and IRClosure.blockArgs+keywordArgs

This reduces memory usage of Rails console about 3.2M. It also probably reduces build time a little bit because it is not examining every instr to see if it is a call argument. With that said, these fields are used for converting zsuper -> super. In this case we will walk instrs a second time to find recv instrs. As a result builds which contain zsuper will end up slower. Generally most Ruby methods are so small that walking a small list a second time will not really be a burden combined with how uncommon zsuper is I don't see a need to address this second scan.
jruby · Jan 26, 2015 · 6eae595 · 6eae595
1 parent dafa4cc
commit 6eae595
Showing 5 changed files with 66 additions and 113 deletions.
diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -3399,7 +3399,6 @@ public Operand buildZArray() {
     }
 
     private Operand buildZSuperIfNest(final Operand block) {
-        final IRScope s = scope;
         // If we are in a block, we cannot make any assumptions about what args
         // the super instr is going to get -- if there were no 'define_method'
         // for defining methods, we could guarantee that the super is going to
@@ -3419,9 +3418,9 @@ public Operand run() {
 
                 Label allDoneLabel = getNewLabel();
 
-                IRScope superScope = s;
                 int depthFromSuper = 0;
                 Label next = null;
+                IRScope superScope = scope;
 
                 // Loop and generate a block for each possible value of depthFromSuper
                 Variable zsuperResult = createTemporaryVariable();
@@ -3430,7 +3429,7 @@ public Operand run() {
                     if (next != null) addInstr(new LabelInstr(next));
                     next = getNewLabel();
                     addInstr(BNEInstr.create(new Fixnum(depthFromSuper), scopeDepth, next));
-                    Operand[] args = adjustVariableDepth(((IRClosure)superScope).getBlockArgs(), depthFromSuper);
+                    Operand[] args = adjustVariableDepth(superScope.getCallArgs(), depthFromSuper);
                     addInstr(new ZSuperInstr(zsuperResult, buildSelf(), args,  block));
                     addInstr(new JumpInstr(allDoneLabel));
 
@@ -3443,7 +3442,7 @@ public Operand run() {
 
                 // If we hit a method, this is known to always succeed
                 if (superScope instanceof IRMethod) {
-                    Operand[] args = adjustVariableDepth(((IRMethod)superScope).getCallArgs(), depthFromSuper);
+                    Operand[] args = adjustVariableDepth(superScope.getCallArgs(), depthFromSuper);
                     addInstr(new ZSuperInstr(zsuperResult, buildSelf(), args, block));
                 } //else {
                 // FIXME: Do or don't ... there is no try
@@ -3467,7 +3466,7 @@ public Operand buildZSuper(ZSuperNode zsuperNode) {
 
         // Enebo:ZSuper in for (or nested for) can be statically resolved like method but it needs to fixup depth.
         if (scope instanceof IRMethod) {
-            return buildSuperInstr(block, ((IRMethod) scope).getCallArgs());
+            return buildSuperInstr(block, scope.getCallArgs());
         } else {
             return buildZSuperIfNest(block);
         }

diff --git a/core/src/main/java/org/jruby/ir/IRClosure.java b/core/src/main/java/org/jruby/ir/IRClosure.java
@@ -1,6 +1,5 @@
 package org.jruby.ir;
 
-import org.jcodings.specific.USASCIIEncoding;
 import org.jruby.ir.instructions.*;
 import org.jruby.ir.interpreter.ClosureInterpreterContext;
 import org.jruby.ir.interpreter.InterpreterContext;
@@ -14,12 +13,8 @@
 import org.jruby.runtime.IRBlockBody;
 import org.jruby.runtime.InterpretedIRBlockBody;
 import org.jruby.runtime.Signature;
-import org.jruby.util.KeyValuePair;
 import org.objectweb.asm.Handle;
 
-import java.util.ArrayList;
-import java.util.List;
-
 // Closures are contexts/scopes for the purpose of IR building.  They are self-contained and accumulate instructions
 // that don't merge into the flow of the containing scope.  They are manipulated as an unit.
 // Their parents are always execution scopes.
@@ -33,10 +28,6 @@ public class IRClosure extends IRScope {
 
     private boolean isBeginEndBlock;
 
-    // Block parameters
-    private List<Operand> blockArgs;
-    private List<KeyValuePair<Operand, Operand>> keywordArgs;
-
     /** The parameter names, for Proc#parameters */
     private String[] parameterList;
 
@@ -83,8 +74,7 @@ protected IRClosure(IRClosure c, IRScope lexicalParent, int closureId, String fu
         } else {
             this.body = new InterpretedIRBlockBody(this, c.body.getSignature());
         }
-        this.blockArgs = new ArrayList<>();
-        this.keywordArgs = new ArrayList<>();
+
         this.signature = c.signature;
     }
 
@@ -98,8 +88,6 @@ public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, Stati
 
     public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, StaticScope staticScope, Signature signature, int argumentType, String prefix, boolean isBeginEndBlock) {
         this(manager, lexicalParent, lexicalParent.getFileName(), lineNumber, staticScope, prefix);
-        this.blockArgs = new ArrayList<>();
-        this.keywordArgs = new ArrayList<>();
         this.argumentType = argumentType;
         this.signature = signature;
         lexicalParent.addClosure(this);
@@ -132,9 +120,8 @@ public boolean isBeginEndBlock() {
 
     public void setParameterList(String[] parameterList) {
         this.parameterList = parameterList;
-        if (!getManager().isDryRun()) {
-            ((InterpretedIRBlockBody)this.body).setParameterList(parameterList);
-        }
+
+        if (!getManager().isDryRun()) this.body.setParameterList(parameterList);
     }
 
     public String[] getParameterList() {
@@ -186,39 +173,6 @@ public boolean isFlipScope() {
         return false;
     }
 
-    @Override
-    public void addInstr(Instr i) {
-        // Accumulate block arguments
-        if (i instanceof ReceiveKeywordRestArgInstr) {
-            // Always add the keyword rest arg to the beginning
-            keywordArgs.add(0, new KeyValuePair<Operand, Operand>(Symbol.KW_REST_ARG_DUMMY, ((ReceiveArgBase) i).getResult()));
-        } else if (i instanceof ReceiveKeywordArgInstr) {
-            ReceiveKeywordArgInstr rkai = (ReceiveKeywordArgInstr)i;
-            // FIXME: This lost encoding information when name was converted to string earlier in IRBuilder
-            keywordArgs.add(new KeyValuePair<Operand, Operand>(new Symbol(rkai.argName, USASCIIEncoding.INSTANCE), rkai.getResult()));
-        } else if (i instanceof ReceiveRestArgInstr) {
-            blockArgs.add(new Splat(((ReceiveRestArgInstr)i).getResult()));
-        } else if (i instanceof ReceiveArgBase) {
-            blockArgs.add(((ReceiveArgBase) i).getResult());
-        }
-
-        super.addInstr(i);
-    }
-
-    public Operand[] getBlockArgs() {
-        if (receivesKeywordArgs()) {
-            int i = 0;
-            Operand[] args = new Operand[blockArgs.size() + 1];
-            for (Operand arg: blockArgs) {
-                args[i++] = arg;
-            }
-            args[i] = new Hash(keywordArgs, true);
-            return args;
-        } else {
-            return blockArgs.toArray(new Operand[blockArgs.size()]);
-        }
-    }
-
     public String toStringBody() {
         StringBuilder buf = new StringBuilder();
         buf.append(getName()).append(" = { \n");

diff --git a/core/src/main/java/org/jruby/ir/IREvalScript.java b/core/src/main/java/org/jruby/ir/IREvalScript.java
@@ -8,7 +8,6 @@
 import org.jruby.ir.interpreter.InterpreterContext;
 import org.jruby.ir.operands.Label;
 import org.jruby.ir.operands.LocalVariable;
-import org.jruby.ir.operands.Operand;
 import org.jruby.parser.StaticScope;
 
 public class IREvalScript extends IRClosure {
@@ -50,11 +49,6 @@ public IRScopeType getScopeType() {
         return IRScopeType.EVAL_SCRIPT;
     }
 
-    @Override
-    public Operand[] getBlockArgs() {
-        return new Operand[0];
-    }
-
     public boolean isModuleOrInstanceEval() {
         return evalType == EvalType.MODULE_EVAL || evalType == EvalType.INSTANCE_EVAL;
     }

diff --git a/core/src/main/java/org/jruby/ir/IRMethod.java b/core/src/main/java/org/jruby/ir/IRMethod.java
@@ -1,21 +1,10 @@
 package org.jruby.ir;
 
-import org.jcodings.specific.USASCIIEncoding;
 import org.jruby.ast.MethodDefNode;
 import org.jruby.internal.runtime.methods.IRMethodArgs;
-import org.jruby.ir.instructions.Instr;
-import org.jruby.ir.instructions.ReceiveArgBase;
-import org.jruby.ir.instructions.ReceiveKeywordArgInstr;
-import org.jruby.ir.instructions.ReceiveKeywordRestArgInstr;
-import org.jruby.ir.instructions.ReceiveRestArgInstr;
 import org.jruby.ir.interpreter.InterpreterContext;
 import org.jruby.ir.operands.LocalVariable;
-import org.jruby.ir.operands.Operand;
-import org.jruby.ir.operands.Symbol;
-import org.jruby.ir.operands.Hash;
-import org.jruby.ir.operands.Splat;
 import org.jruby.ir.representations.BasicBlock;
-import org.jruby.util.KeyValuePair;
 import org.jruby.parser.StaticScope;
 
 import java.lang.invoke.MethodType;
@@ -28,13 +17,6 @@
 public class IRMethod extends IRScope {
     public final boolean isInstanceMethod;
 
-    // Note that if operands from the method are modified,
-    // callArgs would have to be updated as well
-    //
-    // Call parameters
-    private List<Operand> callArgs;
-    private List<KeyValuePair<Operand, Operand>> keywordArgs;
-
     // Argument description of the form [:req, "a"], [:opt, "b"] ..
     private List<String[]> argDesc;
 
@@ -52,8 +34,6 @@ public IRMethod(IRManager manager, IRScope lexicalParent, MethodDefNode defn, St
 
         this.defn = defn;
         this.isInstanceMethod = isInstanceMethod;
-        this.callArgs = new ArrayList<>();
-        this.keywordArgs = new ArrayList<>();
         this.argDesc = new ArrayList<>();
         this.signatures = new HashMap<>();
 
@@ -85,25 +65,6 @@ public IRScopeType getScopeType() {
         return isInstanceMethod ? IRScopeType.INSTANCE_METHOD : IRScopeType.CLASS_METHOD;
     }
 
-    @Override
-    public void addInstr(Instr i) {
-        // Accumulate call arguments
-        if (i instanceof ReceiveKeywordRestArgInstr) {
-            // Always add the keyword rest arg to the beginning
-            keywordArgs.add(0, new KeyValuePair<Operand, Operand>(Symbol.KW_REST_ARG_DUMMY, ((ReceiveArgBase) i).getResult()));
-        } else if (i instanceof ReceiveKeywordArgInstr) {
-            ReceiveKeywordArgInstr rkai = (ReceiveKeywordArgInstr)i;
-            // FIXME: This lost encoding information when name was converted to string earlier in IRBuilder
-            keywordArgs.add(new KeyValuePair<Operand, Operand>(new Symbol(rkai.argName, USASCIIEncoding.INSTANCE), rkai.getResult()));
-        } else if (i instanceof ReceiveRestArgInstr) {
-            callArgs.add(new Splat(((ReceiveRestArgInstr)i).getResult()));
-        } else if (i instanceof ReceiveArgBase) {
-            callArgs.add(((ReceiveArgBase) i).getResult());
-        }
-
-        super.addInstr(i);
-    }
-
     public void addArgDesc(IRMethodArgs.ArgType type, String argName) {
         argDesc.add(new String[]{type.name(), argName});
     }
@@ -112,20 +73,6 @@ public List<String[]> getArgDesc() {
         return argDesc;
     }
 
-    public Operand[] getCallArgs() {
-        if (receivesKeywordArgs()) {
-            int i = 0;
-            Operand[] args = new Operand[callArgs.size() + 1];
-            for (Operand arg: callArgs) {
-                args[i++] = arg;
-            }
-            args[i] = new Hash(keywordArgs, true);
-            return args;
-        } else {
-            return callArgs.toArray(new Operand[callArgs.size()]);
-        }
-    }
-
     @Override
     protected LocalVariable findExistingLocalVariable(String name, int scopeDepth) {
         assert scopeDepth == 0: "Local variable depth in IRMethod should always be zero (" + name + " had depth of " + scopeDepth + ")";

diff --git a/core/src/main/java/org/jruby/ir/IRScope.java b/core/src/main/java/org/jruby/ir/IRScope.java
@@ -1,5 +1,6 @@
 package org.jruby.ir;
 
+import org.jcodings.specific.USASCIIEncoding;
 import org.jruby.ParseResult;
 import org.jruby.RubyInstanceConfig;
 import org.jruby.RubyModule;
@@ -17,6 +18,7 @@
 import org.jruby.ir.transformations.inlining.CFGInliner;
 import org.jruby.ir.transformations.inlining.SimpleCloneInfo;
 import org.jruby.parser.StaticScope;
+import org.jruby.util.KeyValuePair;
 import org.jruby.util.log.Logger;
 import org.jruby.util.log.LoggerFactory;
 
@@ -1011,6 +1013,63 @@ public boolean definesLocalVariable(Variable v) {
         return false;
     }
 
+    /**
+     * Extract all call arguments from the specified scope (only useful for Closures and Methods) so that
+     * we can convert zsupers to supers with explicit arguments.
+     *
+     * Note: This is fairly expensive because we walk entire scope when we could potentially stop earlier
+     * if we knew when recv_* were done.
+     */
+    public Operand[] getCallArgs() {
+        List<Operand> callArgs = new ArrayList<>(5);
+        List<KeyValuePair<Operand, Operand>> keywordArgs = new ArrayList<>(3);
+
+        // We have two paths.  eval and non-eval.
+        if (instrList == null) {  // CFG already made.  eval has zsuper and we walk back to some executing method/script
+            for (BasicBlock bb: getCFG().getBasicBlocks()) {
+                for (Instr instr: bb.getInstrs()) {
+                    extractCallOperands(callArgs, keywordArgs, instr);
+                }
+            }
+        } else {                  // common zsuper case.  non-eval and at build time entirely.
+            for (Instr instr : getInstrs()) {
+                extractCallOperands(callArgs, keywordArgs, instr);
+            }
+        }
+
+        return getCallOperands(callArgs, keywordArgs);
+    }
+
+
+    private void extractCallOperands(List<Operand> callArgs, List<KeyValuePair<Operand, Operand>> keywordArgs, Instr instr) {
+        if (instr instanceof ReceiveKeywordRestArgInstr) {
+            // Always add the keyword rest arg to the beginning
+            keywordArgs.add(0, new KeyValuePair<Operand, Operand>(Symbol.KW_REST_ARG_DUMMY, ((ReceiveArgBase) instr).getResult()));
+        } else if (instr instanceof ReceiveKeywordArgInstr) {
+            ReceiveKeywordArgInstr rkai = (ReceiveKeywordArgInstr) instr;
+            // FIXME: This lost encoding information when name was converted to string earlier in IRBuilder
+            keywordArgs.add(new KeyValuePair<Operand, Operand>(new Symbol(rkai.argName, USASCIIEncoding.INSTANCE), rkai.getResult()));
+        } else if (instr instanceof ReceiveRestArgInstr) {
+            callArgs.add(new Splat(((ReceiveRestArgInstr) instr).getResult()));
+        } else if (instr instanceof ReceiveArgBase) {
+            callArgs.add(((ReceiveArgBase) instr).getResult());
+        }
+    }
+
+    private Operand[] getCallOperands(List<Operand> callArgs, List<KeyValuePair<Operand, Operand>> keywordArgs) {
+        if (receivesKeywordArgs()) {
+            int i = 0;
+            Operand[] args = new Operand[callArgs.size() + 1];
+            for (Operand arg: callArgs) {
+                args[i++] = arg;
+            }
+            args[i] = new Hash(keywordArgs, true);
+            return args;
+        }
+
+        return callArgs.toArray(new Operand[callArgs.size()]);
+    }
+
     public void setDataFlowSolution(String name, DataFlowProblem p) {
         dfProbs.put(name, p);
     }