Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: a761edbe926e
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 131e748e2ca1
Choose a head ref
  • 3 commits
  • 5 files changed
  • 1 contributor

Commits on Dec 15, 2016

  1. Copy the full SHA
    33dba52 View commit details
  2. Emit only one body for jitted methods.

    Previously, in order to support a specific-arity path for Ruby
    method bodies, we emitted both a variable-arity version (with
    arity checks) and a specific-arity version. This doubled the
    amount of bytecode emitted for a given method body, and also
    doubled the code generated for any contained blocks or metaclass
    bodies. This commit modifies method jit to make the variable-arity
    path just call the specific-arity path when present, reducing
    generated bytecode by up to 50%.
    headius committed Dec 15, 2016
    Copy the full SHA
    db15830 View commit details
  3. Merge pull request #4390 from headius/specific-arity-jit-cleanup

    Specific arity jit cleanup
    headius authored Dec 15, 2016
    Copy the full SHA
    131e748 View commit details
Original file line number Diff line number Diff line change
@@ -121,7 +121,7 @@ public DynamicMethod getAnnotatedMethod(final RubyModule implementationClass, fi
implementationClass,
desc1.anno.visibility(),
(min == max) ?
org.jruby.runtime.Signature.from(min, 0, 0, 0, 0, org.jruby.runtime.Signature.Rest.NONE, false).encode() :
org.jruby.runtime.Signature.from(min, 0, 0, 0, 0, org.jruby.runtime.Signature.Rest.NONE, -1).encode() :
org.jruby.runtime.Signature.OPTIONAL.encode(),
true,
notImplemented,
5 changes: 2 additions & 3 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
Original file line number Diff line number Diff line change
@@ -2144,7 +2144,6 @@ public void receiveRequiredArg(Node node, int argIndex, Signature signature) {

protected void receiveNonBlockArgs(final ArgsNode argsNode) {
Signature signature = scope.getStaticScope().getSignature();
KeywordRestArgNode keyRest = argsNode.getKeyRest();

// For closures, we don't need the check arity call
if (scope instanceof IRMethod) {
@@ -2155,12 +2154,12 @@ protected void receiveNonBlockArgs(final ArgsNode argsNode) {
// But later, perhaps can make this implicit in the method setup preamble?

addInstr(new CheckArityInstr(signature.required(), signature.opt(), signature.hasRest(), argsNode.hasKwargs(),
keyRest == null ? -1 : keyRest.getIndex()));
signature.keyRest()));
} else if (scope instanceof IRClosure && argsNode.hasKwargs()) {
// FIXME: This is added to check for kwargs correctness but bypass regular correctness.
// Any other arity checking currently happens within Java code somewhere (RubyProc.call?)
addInstr(new CheckArityInstr(signature.required(), signature.opt(), signature.hasRest(), argsNode.hasKwargs(),
keyRest == null ? -1 : keyRest.getIndex()));
signature.keyRest()));
}

// Other args begin at index 0
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/persistence/IRWriter.java
Original file line number Diff line number Diff line change
@@ -88,7 +88,7 @@ private static void persistScopeHeader(IRWriterEncoder file, IRScope scope) {
if (scope instanceof IRClosure) {
IRClosure closure = (IRClosure) scope;

file.encode(closure.getSignature().encode());
file.encode(closure.getSignature());
}

persistStaticScope(file, scope.getStaticScope());
83 changes: 63 additions & 20 deletions core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -66,6 +66,7 @@ public class JVMVisitor extends IRVisitor {
private static final Signature METHOD_SIGNATURE_BASE = Signature
.returning(IRubyObject.class)
.appendArgs(new String[]{"context", "scope", "self", BLOCK_ARG_NAME, "class", "callName"}, ThreadContext.class, StaticScope.class, IRubyObject.class, Block.class, RubyModule.class, String.class);
private static final Signature METHOD_SIGNATURE_VARARGS = METHOD_SIGNATURE_BASE.insertArg(BLOCK_ARG_NAME, "args", IRubyObject[].class);

public static final Signature CLOSURE_SIGNATURE = Signature
.returning(IRubyObject.class)
@@ -238,6 +239,45 @@ protected void emitScope(IRScope scope, String name, Signature signature, boolea
jvm.popmethod();
}

protected void emitVarargsMethodWrapper(IRScope scope, String name, Signature variableSignature, Signature specificSignature) {

Map<BasicBlock, Label> exceptionTable = scope.buildJVMExceptionTable();

jvm.pushmethod(name, scope, variableSignature, false);

IRBytecodeAdapter m = jvmMethod();

// check arity
org.jruby.runtime.Signature scopeSig = scope.getStaticScope().getSignature();
checkArity(scopeSig.required(), scopeSig.opt(), scopeSig.hasRest(), scopeSig.hasKwargs(), scopeSig.keyRest());

// push leading args
m.loadContext();
m.loadStaticScope();
m.loadSelf();

// unwrap specific args
if (scopeSig.required() > 0) {
for (int i = 0; i < scopeSig.required(); i++) {
m.loadArgs();
jvmAdapter().pushInt(i);
jvmAdapter().aaload();
}
}

// push trailing args
m.loadBlock();
m.loadFrameClass();
m.loadFrameName();

// invoke specific-arity version and return
Method specificMethod = new Method(name, Type.getType(specificSignature.type().returnType()), IRRuntimeHelpers.typesFromSignature(specificSignature));
jvmAdapter().invokestatic(m.getClassData().clsName, name, specificMethod.getDescriptor());
jvmAdapter().areturn();

jvm.popmethod();
}

protected static final Signature signatureFor(IRScope method, boolean aritySplit) {
if (aritySplit) {
StaticScope argScope = method.getStaticScope();
@@ -306,14 +346,16 @@ protected void emitBlockJIT(IRClosure closure, JVMVisitorMethodContext context)
private void emitWithSignatures(IRMethod method, JVMVisitorMethodContext context, String name) {
context.setJittedName(name);

Signature signature = signatureFor(method, false);
emitScope(method, name, signature, false, true);
context.addNativeSignature(-1, signature.type());

Signature specificSig = signatureFor(method, true);
if (specificSig != null) {
if (specificSig == null) {
Signature signature = signatureFor(method, false);
emitScope(method, name, signature, false, true);
context.addNativeSignature(-1, signature.type());
} else {
emitScope(method, name, specificSig, true, false);
context.addNativeSignature(method.getStaticScope().getSignature().required(), specificSig.type());
emitVarargsMethodWrapper(method, name, METHOD_SIGNATURE_VARARGS, specificSig);
context.addNativeSignature(-1, METHOD_SIGNATURE_VARARGS.type());
}
}

@@ -1057,20 +1099,24 @@ public void CheckArityInstr(CheckArityInstr checkarityinstr) {
if (jvm.methodData().specificArity >= 0) {
// no arity check in specific arity path
} else {
jvmMethod().loadContext();
jvmMethod().loadStaticScope();
jvmMethod().loadArgs();
// TODO: pack these, e.g. in a constant pool String
jvmAdapter().ldc(checkarityinstr.required);
jvmAdapter().ldc(checkarityinstr.opt);
jvmAdapter().ldc(checkarityinstr.rest);
jvmAdapter().ldc(checkarityinstr.receivesKeywords);
jvmAdapter().ldc(checkarityinstr.restKey);
jvmMethod().loadBlockType();
jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "checkArity", sig(void.class, ThreadContext.class, StaticScope.class, Object[].class, int.class, int.class, boolean.class, boolean.class, int.class, Block.Type.class));
checkArity(checkarityinstr.required, checkarityinstr.opt, checkarityinstr.rest, checkarityinstr.receivesKeywords, checkarityinstr.restKey);
}
}

private void checkArity(int required, int opt, boolean rest, boolean receivesKeywords, int restKey) {
jvmMethod().loadContext();
jvmMethod().loadStaticScope();
jvmMethod().loadArgs();
// TODO: pack these, e.g. in a constant pool String
jvmAdapter().ldc(required);
jvmAdapter().ldc(opt);
jvmAdapter().ldc(rest);
jvmAdapter().ldc(receivesKeywords);
jvmAdapter().ldc(restKey);
jvmMethod().loadBlockType();
jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "checkArity", sig(void.class, ThreadContext.class, StaticScope.class, Object[].class, int.class, int.class, boolean.class, boolean.class, int.class, Block.Type.class));
}

@Override
public void CheckForLJEInstr(CheckForLJEInstr checkForljeinstr) {
jvmMethod().loadContext();
@@ -1151,13 +1197,10 @@ public void DefineClassMethodInstr(DefineClassMethodInstr defineclassmethodinstr
JVMVisitorMethodContext context = new JVMVisitorMethodContext();
emitMethod(method, context);

MethodType variable = context.getNativeSignature(-1); // always a variable arity handle
assert(variable != null);

String defSignature = pushHandlesForDef(
context.getJittedName(),
context.getNativeSignaturesExceptVariable(),
variable,
METHOD_SIGNATURE_VARARGS.type(),
sig(void.class, ThreadContext.class, java.lang.invoke.MethodHandle.class, IRScope.class, IRubyObject.class),
sig(void.class, ThreadContext.class, java.lang.invoke.MethodHandle.class, java.lang.invoke.MethodHandle.class, int.class, IRScope.class, IRubyObject.class));

57 changes: 29 additions & 28 deletions core/src/main/java/org/jruby/runtime/Signature.java
Original file line number Diff line number Diff line change
@@ -31,14 +31,14 @@ public static Rest fromOrdinal(int ordinal) {
}
}

public static final Signature NO_ARGUMENTS = new Signature(0, 0, 0, Rest.NONE, 0, 0, false);
public static final Signature ONE_ARGUMENT = new Signature(1, 0, 0, Rest.NONE, 0, 0, false);
public static final Signature TWO_ARGUMENTS = new Signature(2, 0, 0, Rest.NONE, 0, 0, false);
public static final Signature THREE_ARGUMENTS = new Signature(3, 0, 0, Rest.NONE, 0, 0, false);
public static final Signature OPTIONAL = new Signature(0, 0, 0, Rest.NORM, 0, 0, false);
public static final Signature ONE_REQUIRED = new Signature(1, 0, 0, Rest.NORM, 0, 0, false);
public static final Signature TWO_REQUIRED = new Signature(2, 0, 0, Rest.NORM, 0, 0, false);
public static final Signature THREE_REQUIRED = new Signature(3, 0, 0, Rest.NORM, 0, 0, false);
public static final Signature NO_ARGUMENTS = new Signature(0, 0, 0, Rest.NONE, 0, 0, -1);
public static final Signature ONE_ARGUMENT = new Signature(1, 0, 0, Rest.NONE, 0, 0, -1);
public static final Signature TWO_ARGUMENTS = new Signature(2, 0, 0, Rest.NONE, 0, 0, -1);
public static final Signature THREE_ARGUMENTS = new Signature(3, 0, 0, Rest.NONE, 0, 0, -1);
public static final Signature OPTIONAL = new Signature(0, 0, 0, Rest.NORM, 0, 0, -1);
public static final Signature ONE_REQUIRED = new Signature(1, 0, 0, Rest.NORM, 0, 0, -1);
public static final Signature TWO_REQUIRED = new Signature(2, 0, 0, Rest.NORM, 0, 0, -1);
public static final Signature THREE_REQUIRED = new Signature(3, 0, 0, Rest.NORM, 0, 0, -1);

private final short pre;
private final short opt;
@@ -47,16 +47,16 @@ public static Rest fromOrdinal(int ordinal) {
private final short kwargs;
private final short requiredKwargs;
private final Arity arity;
private final boolean restKwargs;
private final int keyRest;

public Signature(int pre, int opt, int post, Rest rest, int kwargs, int requiredKwargs, boolean restKwargs) {
public Signature(int pre, int opt, int post, Rest rest, int kwargs, int requiredKwargs, int keyRest) {
this.pre = (short) pre;
this.opt = (short) opt;
this.post = (short) post;
this.rest = rest;
this.kwargs = (short) kwargs;
this.requiredKwargs = (short) requiredKwargs;
this.restKwargs = restKwargs;
this.keyRest = keyRest;

// NOTE: Some logic to *assign* variables still uses Arity, which treats Rest.ANON (the
// |a,| form) as a rest arg for destructuring purposes. However ANON does *not*
@@ -74,15 +74,16 @@ public int getRequiredKeywordForArityCount() {
}

public boolean restKwargs() {
return restKwargs;
return keyRest != -1;
}

public int pre() { return pre; }
public int opt() { return opt; }
public Rest rest() { return rest; }
public int post() { return post; }
public boolean hasKwargs() { return kwargs > 0 || restKwargs; }
public boolean hasKwargs() { return kwargs > 0 || restKwargs(); }
public boolean hasRest() { return rest != Rest.NONE; }
public int keyRest() { return keyRest; }

/**
* Are there an exact (fixed) number of parameters to this signature?
@@ -128,8 +129,8 @@ public static Signature from(Arity arity) {
throw new UnsupportedOperationException("We do not know enough about the arity to convert it to a signature");
}

public static Signature from(int pre, int opt, int post, int kwargs, int requiredKwargs, Rest rest, boolean restKwargs) {
if (opt == 0 && post == 0 && kwargs == 0 && !restKwargs) {
public static Signature from(int pre, int opt, int post, int kwargs, int requiredKwargs, Rest rest, int keyRest) {
if (opt == 0 && post == 0 && kwargs == 0 && keyRest == -1) {
switch (pre) {
case 0:
switch (rest) {
@@ -165,15 +166,15 @@ public static Signature from(int pre, int opt, int post, int kwargs, int require
break;
}
}
return new Signature(pre, opt, post, rest, kwargs, requiredKwargs, restKwargs);
return new Signature(pre, opt, post, rest, kwargs, requiredKwargs, keyRest);
}

public static Signature from(ArgsNode args) {
ArgumentNode restArg = args.getRestArgNode();
Rest rest = restArg != null ? restFromArg(restArg) : Rest.NONE;

return Signature.from(args.getPreCount(), args.getOptionalArgsCount(), args.getPostCount(),
args.getKeywordCount(), args.getRequiredKeywordCount(),rest,args.hasKeyRest());
args.getKeywordCount(), args.getRequiredKeywordCount(),rest,args.hasKeyRest() ? args.getKeyRest().getIndex() : -1);
}

public static Signature from(IterNode iter) {
@@ -213,7 +214,7 @@ public static Signature from(ForNode iter) {
Node restArg = masgn.getRest();
rest = restFromArg(restArg);
}
return Signature.from(masgn.getPreCount(), 0, masgn.getPostCount(), 0, 0, rest, false);
return Signature.from(masgn.getPreCount(), 0, masgn.getPostCount(), 0, 0, rest, -1);
}
return Signature.ONE_ARGUMENT;
}
@@ -229,7 +230,7 @@ public static Signature from(PostExeNode iter) {
private static final int MAX_ENCODED_ARGS_EXPONENT = 8;
private static final int MAX_ENCODED_ARGS_MASK = 0xFF;
private static final int ENCODE_RESTKWARGS_SHIFT = 0;
private static final int ENCODE_REST_SHIFT = ENCODE_RESTKWARGS_SHIFT + 1;
private static final int ENCODE_REST_SHIFT = ENCODE_RESTKWARGS_SHIFT + MAX_ENCODED_ARGS_EXPONENT;
private static final int ENCODE_REQKWARGS_SHIFT = ENCODE_REST_SHIFT + MAX_ENCODED_ARGS_EXPONENT;
private static final int ENCODE_KWARGS_SHIFT = ENCODE_REQKWARGS_SHIFT + MAX_ENCODED_ARGS_EXPONENT;
private static final int ENCODE_POST_SHIFT = ENCODE_KWARGS_SHIFT + MAX_ENCODED_ARGS_EXPONENT;
@@ -244,24 +245,24 @@ public long encode() {
((long)kwargs << ENCODE_KWARGS_SHIFT) |
((long)requiredKwargs << ENCODE_REQKWARGS_SHIFT) |
(rest.ordinal() << ENCODE_REST_SHIFT) |
((restKwargs?1:0) << ENCODE_RESTKWARGS_SHIFT);
(keyRest & 0xFF) << ENCODE_RESTKWARGS_SHIFT;
}

public static Signature decode(long l) {
return Signature.from(
(int)(l >> ENCODE_PRE_SHIFT) & MAX_ENCODED_ARGS_MASK,
(int)(l >> ENCODE_OPT_SHIFT) & MAX_ENCODED_ARGS_MASK,
(int)(l >> ENCODE_POST_SHIFT) & MAX_ENCODED_ARGS_MASK,
(int)(l >> ENCODE_KWARGS_SHIFT) & MAX_ENCODED_ARGS_MASK,
(int)(l >> ENCODE_REQKWARGS_SHIFT) & MAX_ENCODED_ARGS_MASK,
Rest.fromOrdinal((int)((l >> ENCODE_REST_SHIFT) & MAX_ENCODED_ARGS_MASK)),
((int)(l >> ENCODE_RESTKWARGS_SHIFT) & 0x1)==1 ? true : false
(int)(l >>> ENCODE_PRE_SHIFT) & MAX_ENCODED_ARGS_MASK,
(int)(l >>> ENCODE_OPT_SHIFT) & MAX_ENCODED_ARGS_MASK,
(int)(l >>> ENCODE_POST_SHIFT) & MAX_ENCODED_ARGS_MASK,
(int)(l >>> ENCODE_KWARGS_SHIFT) & MAX_ENCODED_ARGS_MASK,
(int)(l >>> ENCODE_REQKWARGS_SHIFT) & MAX_ENCODED_ARGS_MASK,
Rest.fromOrdinal((int)((l >>> ENCODE_REST_SHIFT) & MAX_ENCODED_ARGS_MASK)),
(byte)(l >>> ENCODE_RESTKWARGS_SHIFT) & MAX_ENCODED_ARGS_MASK

);
}

public String toString() {
return "signature(pre=" + pre + ",opt=" + opt + ",post=" + post + ",rest=" + rest + ",kwargs=" + kwargs + ",kwreq=" + requiredKwargs + ",kwrest=" + restKwargs + ")";
return "signature(pre=" + pre + ",opt=" + opt + ",post=" + post + ",rest=" + rest + ",kwargs=" + kwargs + ",kwreq=" + requiredKwargs + ",kwrest=" + keyRest + ")";
}

public void checkArity(Ruby runtime, IRubyObject[] args) {