Skip to content

Commit

Permalink
Use a fake hash and kwargs flag in IR to fake out kwarg logic.
Browse files Browse the repository at this point in the history
headius committed Aug 10, 2017
1 parent ca55782 commit 30b7ac9
Showing 15 changed files with 235 additions and 40 deletions.
12 changes: 6 additions & 6 deletions core/src/main/java/org/jruby/RubyBasicObject.java
Original file line number Diff line number Diff line change
@@ -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;
}

/**
8 changes: 8 additions & 0 deletions core/src/main/java/org/jruby/RubyBoolean.java
Original file line number Diff line number Diff line change
@@ -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 {
7 changes: 5 additions & 2 deletions core/src/main/java/org/jruby/RubyHash.java
Original file line number Diff line number Diff line change
@@ -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;
}

12 changes: 12 additions & 0 deletions core/src/main/java/org/jruby/RubyNil.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
18 changes: 17 additions & 1 deletion core/src/main/java/org/jruby/ir/IRBuilder.java
Original file line number Diff line number Diff line change
@@ -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) }));
}
19 changes: 16 additions & 3 deletions core/src/main/java/org/jruby/ir/instructions/EQQInstr.java
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/operands/Hash.java
Original file line number Diff line number Diff line change
@@ -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();
4 changes: 4 additions & 0 deletions core/src/main/java/org/jruby/ir/persistence/IRDumper.java
Original file line number Diff line number Diff line change
@@ -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()); }
89 changes: 74 additions & 15 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
Original file line number Diff line number Diff line change
@@ -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;
37 changes: 32 additions & 5 deletions core/src/main/java/org/jruby/ir/targets/Bootstrap.java
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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();

Original file line number Diff line number Diff line change
@@ -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) {
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -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);
}
}

53 changes: 53 additions & 0 deletions core/src/main/java/org/jruby/runtime/ThreadContext.java
Original file line number Diff line number Diff line change
@@ -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;
}
}

0 comments on commit 30b7ac9

Please sign in to comment.