Use a fake hash and kwargs flag in IR to fake out kwarg logic.

jruby · Aug 10, 2017 · 30b7ac9 · 30b7ac9
1 parent ca55782
commit 30b7ac9
Showing 15 changed files with 235 additions and 40 deletions.
diff --git a/core/src/main/java/org/jruby/RubyBasicObject.java b/core/src/main/java/org/jruby/RubyBasicObject.java
@@ -400,23 +400,23 @@ public final IRubyObject callMethod(ThreadContext context, String name, IRubyObj
      * #NIL_F} flag for more information.
      */
     @Override
-    public final boolean isNil() {
-        return (flags & NIL_F) != 0;
+    public boolean isNil() {
+        return false;
     }
 
     /**
      * Is this value a truthy value or not? Based on the {@link #FALSE_F} flag.
      */
     @Override
-    public final boolean isTrue() {
-        return (flags & FALSE_F) == 0;
+    public boolean isTrue() {
+        return true;
     }
 
     /**
      * Is this value a falsey value or not? Based on the {@link #FALSE_F} flag.
      */
-    public final boolean isFalse() {
-        return (flags & FALSE_F) != 0;
+    public boolean isFalse() {
+        return false;
     }
 
     /**

diff --git a/core/src/main/java/org/jruby/RubyBoolean.java b/core/src/main/java/org/jruby/RubyBoolean.java
@@ -157,6 +157,14 @@ public static IRubyObject false_xor(IRubyObject f, IRubyObject oth) {
         public static IRubyObject false_to_s(IRubyObject f) {
             return RubyString.newUSASCIIString(f.getRuntime(), "false");
         }
+
+        public boolean isFalse() {
+            return true;
+        }
+
+        public boolean isTrue() {
+            return false;
+        }
     }
 
     public static class True extends RubyBoolean {

diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java
@@ -517,7 +517,7 @@ private final void internalPut(final IRubyObject key, final IRubyObject value) {
         internalPut(key, value, true);
     }
 
-    private final void internalPutSmall(final IRubyObject key, final IRubyObject value) {
+    protected void internalPutSmall(final IRubyObject key, final IRubyObject value) {
         internalPutSmall(key, value, true);
     }
 
@@ -549,8 +549,11 @@ protected void internalPutSmall(final IRubyObject key, final IRubyObject value,
     }
 
     // get implementation
+    public IRubyObject internalGet(IRubyObject key) {
+        return internalGetEntry(key).value;
+    }
 
-    protected IRubyObject internalGet(IRubyObject key) { // specialized for value
+    public IRubyObject internalGet(ThreadContext context, IRubyObject key) { // specialized for value
         return internalGetEntry(key).value;
     }
 

diff --git a/core/src/main/java/org/jruby/RubyNil.java b/core/src/main/java/org/jruby/RubyNil.java
@@ -270,4 +270,16 @@ public Object toJava(Class target) {
         }
         return null;
     }
+
+    public boolean isTrue() {
+        return false;
+    }
+
+    public boolean isNil() {
+        return true;
+    }
+
+    public boolean isFalse() {
+        return true;
+    }
 }
diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -640,6 +640,18 @@ protected Operand[] buildCallArgs(Node args) {
                 boolean hasAssignments = args.containsVariableAssignment();
 
                 for (int i = 0; i < numberOfArgs; i++) {
+                    if (i + 1 == numberOfArgs) {
+                        if (children[i] instanceof HashNode) {
+                            HashNode hashNode = (HashNode) children[i];
+
+                            if (hashNode.hasOnlySymbolKeys()) {
+                                // build hash with kwargs bit
+                                builtArgs[i] = buildHash(hashNode, true);
+
+                                continue;
+                            }
+                        }
+                    }
                     builtArgs[i] = buildWithOrder(children[i], hasAssignments);
                 }
                 return builtArgs;
@@ -2882,6 +2894,10 @@ public Operand buildGlobalVar(Variable result, GlobalVarNode node) {
     }
 
     public Operand buildHash(HashNode hashNode) {
+        return buildHash(hashNode, false);
+    }
+
+    public Operand buildHash(HashNode hashNode, boolean asKwargs) {
         List<KeyValuePair<Operand, Operand>> args = new ArrayList<>();
         boolean hasAssignments = hashNode.containsVariableAssignment();
         Variable hash = null;
@@ -2909,7 +2925,7 @@ public Operand buildHash(HashNode hashNode) {
         }
 
         if (hash == null) {           // non-**arg ordinary hash
-            hash = copyAndReturnValue(new Hash(args));
+            hash = copyAndReturnValue(new Hash(args, asKwargs));
         } else if (!args.isEmpty()) { // ordinary hash values encountered after a **arg
             addInstr(new RuntimeHelperCall(hash, MERGE_KWARGS, new Operand[] { hash, new Hash(args) }));
         }

diff --git a/core/src/main/java/org/jruby/ir/instructions/EQQInstr.java b/core/src/main/java/org/jruby/ir/instructions/EQQInstr.java
@@ -3,6 +3,7 @@
 import org.jruby.ir.IRVisitor;
 import org.jruby.ir.Operation;
 import org.jruby.ir.operands.Operand;
+import org.jruby.ir.operands.UndefinedValue;
 import org.jruby.ir.operands.Variable;
 import org.jruby.ir.persistence.IRReaderDecoder;
 import org.jruby.ir.persistence.IRWriterEncoder;
@@ -20,7 +21,10 @@ public class EQQInstr extends TwoOperandResultBaseInstr implements FixedArityIns
     private final CallSite callSite;
     // This is a splatted value and eqq should compare each element in the array vs
     // treating the array as a single value.
-    private boolean splattedValue;
+    private final boolean splattedValue;
+
+    /** can we follow the simple EQQ path? **/
+    private final boolean simpleEqq;
 
     public EQQInstr(Variable result, Operand v1, Operand v2, boolean splattedValue) {
         super(Operation.EQQ, result, v1, v2);
@@ -29,6 +33,12 @@ public EQQInstr(Variable result, Operand v1, Operand v2, boolean splattedValue)
 
         this.callSite = new FunctionalCachingCallSite("===");
         this.splattedValue = splattedValue;
+
+        if (splattedValue || v2 instanceof UndefinedValue) {
+            simpleEqq = false;
+        } else {
+            simpleEqq = true;
+        }
     }
 
     @Override
@@ -55,7 +65,7 @@ public boolean isSplattedValue() {
 
     @Override
     public Instr clone(CloneInfo ii) {
-        return new EQQInstr(ii.getRenamedVariable(result), getArg1().cloneForInlining(ii), getArg2().cloneForInlining(ii), isSplattedValue());
+        return new EQQInstr(ii.getRenamedVariable(result), getArg1().cloneForInlining(ii), getArg2().cloneForInlining(ii), splattedValue);
     }
 
     @Override
@@ -74,7 +84,10 @@ public static EQQInstr decode(IRReaderDecoder d) {
     public Object interpret(ThreadContext context, StaticScope currScope, DynamicScope currDynScope, IRubyObject self, Object[] temp) {
         IRubyObject recv = (IRubyObject) getArg1().retrieve(context, self, currScope, currDynScope, temp);
         IRubyObject value = (IRubyObject) getArg2().retrieve(context, self, currScope, currDynScope, temp);
-        return IRRuntimeHelpers.isEQQ(context, recv, value, callSite, isSplattedValue());
+        if (simpleEqq) {
+            return callSite.call(context, recv, recv, value);
+        }
+        return IRRuntimeHelpers.isEQQ(context, recv, value, callSite, splattedValue);
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/ir/operands/Hash.java b/core/src/main/java/org/jruby/ir/operands/Hash.java
@@ -27,7 +27,7 @@ public class Hash extends Operand {
 
     // Is this a hash used to represent a keyword hash to be setup for ZSuper?
     // SSS FIXME: Quick hack for now - this should probably be done with an overloaded operand.
-    final public boolean isKWArgsHash;
+    public boolean isKWArgsHash;
 
     public Hash(List<KeyValuePair<Operand, Operand>> pairs, boolean isKWArgsHash) {
         super();

diff --git a/core/src/main/java/org/jruby/ir/persistence/IRDumper.java b/core/src/main/java/org/jruby/ir/persistence/IRDumper.java
@@ -308,6 +308,10 @@ public void Hash(Hash hash) {
             print("=>");
             visit(pair.getValue());
         }
+        if (comma == true) {
+            print(",isKwargs=");
+            print(hash.isKWArgsHash);
+        }
     }
     public void IRException(IRException irexception) { print(irexception.getType()); }
     public void Label(Label label) { print(label.toString()); }

diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
@@ -502,22 +502,29 @@ public static IRubyObject[] frobnicateKwargsArgument(ThreadContext context, IRub
                 return ArraySupport.newCopy(args, RubyHash.newSmallHash(context.runtime));
             }
 
-            DivvyKeywordsVisitor visitor = new DivvyKeywordsVisitor();
-            // We know toHash makes null, nil, or Hash
-            ((RubyHash) kwargs).visitAll(context, visitor, null);
+            IRubyObject syms;
+            if (kwargs instanceof FakeKwargsHash) {
+                syms = kwargs;
+            } else {
 
-            if (visitor.syms == null) {
-                // no symbols, use empty kwargs hash
-                visitor.syms = RubyHash.newSmallHash(context.runtime);
-            }
+                DivvyKeywordsVisitor visitor = new DivvyKeywordsVisitor();
+                // We know toHash makes null, nil, or Hash
+                ((RubyHash) kwargs).visitAll(context, visitor, null);
+
+                if (visitor.syms == null) {
+                    // no symbols, use empty kwargs hash
+                    visitor.syms = RubyHash.newSmallHash(context.runtime);
+                }
 
-            if (visitor.others != null) { // rest args exists too expand args
-                IRubyObject[] newArgs = new IRubyObject[args.length + 1];
-                System.arraycopy(args, 0, newArgs, 0, args.length);
-                args = newArgs;
-                args[args.length - 2] = visitor.others; // opt args
+                if (visitor.others != null) { // rest args exists too expand args
+                    IRubyObject[] newArgs = new IRubyObject[args.length + 1];
+                    System.arraycopy(args, 0, newArgs, 0, args.length);
+                    args = newArgs;
+                    args[args.length - 2] = visitor.others; // opt args
+                }
+                syms = visitor.syms;
             }
-            args[args.length - 1] = visitor.syms; // kwargs hash
+            args[args.length - 1] = syms; // kwargs hash
         }
 
         return args;
@@ -565,6 +572,37 @@ public static RubyHash extractKwargsHash(Object[] args, int requiredArgsCount, b
         return null;
     }
 
+    public static class FakeKwargsHash extends RubyHash {
+        private final Ruby runtime;
+
+        public FakeKwargsHash(Ruby runtime) {
+            super(runtime);
+
+            this.runtime = runtime;
+        }
+
+        @Override
+        public IRubyObject internalGet(ThreadContext context, IRubyObject key) {
+            if (key instanceof RubySymbol) {
+                return context.getKwarg((RubySymbol) key);
+            }
+            return null;
+        }
+
+        @Override
+        public IRubyObject delete(ThreadContext context, IRubyObject key, Block block) {
+            if (key instanceof RubySymbol) {
+                return context.deleteKwarg(context.findKwarg((RubySymbol) key));
+            }
+            return null;
+        }
+
+        @Override
+        public void internalPutSmall(final IRubyObject key, final IRubyObject value) {
+            runtime.getCurrentContext().setKwarg(key, value);
+        }
+    }
+
     public static void checkForExtraUnwantedKeywordArgs(ThreadContext context, final StaticScope scope, RubyHash keywordArgs) {
         // we do an inexpensive non-gathering scan first to see if there's a bad keyword
         try {
@@ -949,11 +987,13 @@ public static IRubyObject getPreArgSafe(ThreadContext context, IRubyObject[] arg
     public static IRubyObject receiveKeywordArg(ThreadContext context, IRubyObject[] args, int required, String argName, boolean acceptsKeywordArgument) {
         RubyHash keywordArguments = extractKwargsHash(args, required, acceptsKeywordArgument);
 
-        if (keywordArguments == null) return UndefinedValue.UNDEFINED;
+        if (keywordArguments == null) {
+            return UndefinedValue.UNDEFINED;
+        }
 
         RubySymbol keywordName = context.runtime.newSymbol(argName);
 
-        if (keywordArguments.fastARef(keywordName) == null) return UndefinedValue.UNDEFINED;
+        if (keywordArguments.internalGet(context, keywordName) == null) return UndefinedValue.UNDEFINED;
 
         // SSS FIXME: Can we use an internal delete here?
         // Enebo FIXME: Delete seems wrong if we are doing this for duplication purposes.
@@ -1162,6 +1202,25 @@ public static RubyHash constructHashFromArray(Ruby runtime, IRubyObject[] pairs)
         return hash;
     }
 
+    @JIT
+    public static RubyHash constructFakeHashFromArray(Ruby runtime, IRubyObject[] pairs) {
+        int length = pairs.length / 2;
+        boolean useSmallHash = length <= 10;
+
+        runtime.getCurrentContext().clearKwargs();
+
+        RubyHash hash = runtime.getCurrentContext().fakeKwargs;
+        for (int i = 0; i < pairs.length;) {
+            if (useSmallHash) {
+                hash.fastASetSmall(runtime, pairs[i++], pairs[i++], true);
+            } else {
+                hash.fastASet(runtime, pairs[i++], pairs[i++], true);
+            }
+
+        }
+        return hash;
+    }
+
     @JIT
     public static RubyHash dupKwargsHashAndPopulateFromArray(ThreadContext context, RubyHash dupHash, IRubyObject[] pairs) {
         Ruby runtime = context.runtime;

diff --git a/core/src/main/java/org/jruby/ir/targets/Bootstrap.java b/core/src/main/java/org/jruby/ir/targets/Bootstrap.java
@@ -109,10 +109,12 @@ public static CallSite array(Lookup lookup, String name, MethodType type) {
     }
 
     public static CallSite hash(Lookup lookup, String name, MethodType type) {
-        MethodHandle handle = Binder
-                .from(lookup, type)
-                .collect(1, IRubyObject[].class)
-                .invokeStaticQuiet(LOOKUP, Bootstrap.class, "hash");
+        Binder binder = Binder
+                .from(lookup, type);
+        if (type.parameterCount() != 3) {
+            binder = binder.collect(1, IRubyObject[].class);
+        }
+        MethodHandle handle = binder.invokeStaticQuiet(LOOKUP, Bootstrap.class, name);
         CallSite site = new ConstantCallSite(handle);
         return site;
     }
@@ -290,6 +292,31 @@ public static IRubyObject hash(ThreadContext context, IRubyObject[] pairs) {
         return hash;
     }
 
+    public static IRubyObject fakeHash(ThreadContext context, IRubyObject[] pairs) {
+        Ruby runtime = context.runtime;
+        RubyHash hash = context.fakeKwargs;
+        context.clearKwargs();
+        for (int i = 0; i < pairs.length;) {
+            hash.fastASetSmall(runtime, pairs[i++], pairs[i++], false);
+        }
+        return hash;
+    }
+
+    public static IRubyObject hash(ThreadContext context, IRubyObject key, IRubyObject value) {
+        Ruby runtime = context.runtime;
+        RubyHash hash = RubyHash.newHash(runtime);
+        hash.fastASetCheckString(runtime, key, value);
+        return hash;
+    }
+
+    public static IRubyObject fakeHash(ThreadContext context, IRubyObject key, IRubyObject value) {
+        Ruby runtime = context.runtime;
+        RubyHash hash = context.fakeKwargs;
+        context.clearKwargs();
+        hash.fastASetCheckString(runtime, key, value);
+        return hash;
+    }
+
     public static IRubyObject kwargsHash(ThreadContext context, RubyHash hash, IRubyObject[] pairs) {
         return IRRuntimeHelpers.dupKwargsHashAndPopulateFromArray(context, hash, pairs);
     }
@@ -484,7 +511,7 @@ static MethodHandle buildJittedHandle(InvokeSite site, DynamicMethod method, boo
 
             // Temporary fix for missing kwargs dup+splitting logic from frobnicate, called by CompiledIRMethod but
             // skipped by indy's direct binding.
-            if (compiledIRMethod.hasKwargs()) return null;
+//            if (compiledIRMethod.hasKwargs()) return null;
 
             // attempt IR direct binding
             // TODO: this will have to expand when we start specializing arities

diff --git a/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter.java b/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter.java
@@ -592,7 +592,7 @@ public static BlockPassType fromIR(ClosureAcceptingInstr callInstr) {
      *
      * @param length number of element pairs
      */
-    public abstract void hash(int length);
+    public abstract void hash(int length, boolean fake);
 
     /**
      * Construct a Hash based on keyword arguments pasesd to this method, for use in zsuper

diff --git a/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter6.java b/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter6.java
@@ -881,13 +881,13 @@ public void array(int length) {
         adapter.invokestatic(classData.clsName, methodName, incomingSig);
     }
 
-    public void hash(int length) {
+    public void hash(int length, boolean fake) {
         if (length > MAX_ARGUMENTS / 2) throw new NotCompilableException("literal hash has more than " + (MAX_ARGUMENTS / 2) + " pairs");
 
         SkinnyMethodAdapter adapter2;
         String incomingSig = sig(JVM.OBJECT, params(ThreadContext.class, JVM.OBJECT, length * 2));
 
-        final String methodName = "hash:" + length;
+        final String methodName = fake ? "fakehash:" : "hash:" + length;
         final ClassData classData = getClassData();
 
         if (!classData.hashMethodsDefined.contains(length)) {
@@ -903,7 +903,7 @@ public void hash(int length) {
             adapter2.getfield(p(ThreadContext.class), "runtime", ci(Ruby.class));
             buildArrayFromLocals(adapter2, 1, length * 2);
 
-            adapter2.invokestatic(p(IRRuntimeHelpers.class), "constructHashFromArray", sig(RubyHash.class, Ruby.class, IRubyObject[].class));
+            adapter2.invokestatic(p(IRRuntimeHelpers.class), fake ? "constructFakeHashFromArray" : "constructHashFromArray", sig(RubyHash.class, Ruby.class, IRubyObject[].class));
             adapter2.areturn();
             adapter2.end();
 

diff --git a/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter7.java b/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter7.java
@@ -303,10 +303,10 @@ public void array(int length) {
         adapter.invokedynamic("array", sig(JVM.OBJECT, params(ThreadContext.class, JVM.OBJECT, length)), Bootstrap.array());
     }
 
-    public void hash(int length) {
+    public void hash(int length, boolean fake) {
         if (length > MAX_ARGUMENTS / 2) throw new NotCompilableException("literal hash has more than " + (MAX_ARGUMENTS / 2) + " pairs");
 
-        adapter.invokedynamic("hash", sig(JVM.OBJECT, params(ThreadContext.class, JVM.OBJECT, length * 2)), Bootstrap.hash());
+        adapter.invokedynamic(fake ? "fakeHash" : "hash", sig(JVM.OBJECT, params(ThreadContext.class, JVM.OBJECT, length * 2)), Bootstrap.hash());
     }
 
     public void kwargsHash(int length) {

diff --git a/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java b/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
@@ -2406,7 +2406,7 @@ public void Hash(Hash hash) {
         if (kwargs) {
             jvmMethod().kwargsHash(pairs.size() - 1);
         } else {
-            jvmMethod().hash(pairs.size());
+            jvmMethod().hash(pairs.size(), hash.isKWArgsHash);
         }
     }
 

diff --git a/core/src/main/java/org/jruby/runtime/ThreadContext.java b/core/src/main/java/org/jruby/runtime/ThreadContext.java
@@ -44,11 +44,13 @@
 import org.jruby.RubyInstanceConfig;
 import org.jruby.RubyModule;
 import org.jruby.RubyString;
+import org.jruby.RubySymbol;
 import org.jruby.RubyThread;
 import org.jruby.ast.executable.RuntimeCache;
 import org.jruby.exceptions.Unrescuable;
 import org.jruby.ext.fiber.ThreadFiber;
 import org.jruby.internal.runtime.methods.DynamicMethod;
+import org.jruby.ir.runtime.IRRuntimeHelpers;
 import org.jruby.lexer.yacc.ISourcePosition;
 import org.jruby.parser.StaticScope;
 import org.jruby.runtime.backtrace.BacktraceData;
@@ -206,6 +208,8 @@ private ThreadContext(Ruby runtime) {
         this.runtimeCache = runtime.getRuntimeCache();
         this.sites = runtime.sites;
 
+        this.fakeKwargs = new IRRuntimeHelpers.FakeKwargsHash(runtime);
+
         // TOPLEVEL self and a few others want a top-level scope.  We create this one right
         // away and then pass it into top-level parse so it ends up being the top level.
         StaticScope topStaticScope = runtime.getStaticScopeFactory().newLocalScope(null);
@@ -1247,4 +1251,53 @@ public org.jruby.util.RubyDateFormat getRubyDateFormat() {
 
         return dateFormat;
     }
+
+    public final IRubyObject[] keywords = new IRubyObject[100];
+    public final IRubyObject[] values = new IRubyObject[100];
+    public final IRRuntimeHelpers.FakeKwargsHash fakeKwargs;
+
+    public void clearKwargs() {
+        for (int i = 0; i < keywords.length; i++) {
+            if (keywords[i] == null) break;
+            keywords[i] = null;
+        }
+    }
+
+    public void setKwarg(IRubyObject keyword, IRubyObject value) {
+        for (int i = 0; i < keywords.length; i++) {
+            if (keywords[i] == null) {
+                keywords[i] = keyword;
+                values[i] = value;
+                return;
+            }
+        }
+    }
+
+    public int findKwarg(RubySymbol keyword) {
+        int length = keywords.length;
+        for (int i = 0; i < length; i++) {
+            IRubyObject key = keywords[i];
+            if (key == null) break;
+            if (key == NEVER) continue;
+            if (keyword == key) return i;
+        }
+        return -1;
+    }
+
+    public IRubyObject getKwarg(RubySymbol keyword) {
+        int length = keywords.length;
+        for (int i = 0; i < length; i++) {
+            IRubyObject key = keywords[i];
+            if (key == null) break;
+            if (key == NEVER) continue;
+            if (keyword == key) return values[i];
+        }
+        return null;
+    }
+
+    public IRubyObject deleteKwarg(int i) {
+        IRubyObject value = values[i];
+        keywords[i] = NEVER;
+        return value;
+    }
 }