Skip to content

Commit

Permalink
Unify and clean up dispatch paths for Java methods and ctors.
Browse files Browse the repository at this point in the history
This work was prompted by running test:jruby:jit with indy turned
on, which caused many calls to follow the IRubyObject[] path where
they would have followed the specific-arity paths. Failures showed
up because the IRubyObject[] paths did not handle error messages
the same way as the specific paths.

So I started by modifying findCallable to switch on arity, to use
the same logic for low-arity calls whether they come through
the appropriate path or not. That caused more bugs because there
were expectations in the tests that varargs processing would be
done for all of the IRubyObject[] path and the specific-arity
findCallable versions did not do varargs.

So I added varargs logic to all paths except for arity = 0,
thinking it did not need it. That broke a test that expected the
no-arg constructor to be called rather than the vararg constructor
when called with no arguments, so I added varargs logic to that
as well.

All tests appear to pass. I have some minor concern about the
additional overhead of checking for varargs matches, but not doing
it before was inconsistent, and we will only look for varargs
matches when nothing else matches and there's actually varargs
methods to look at.

Please review my changes, @kares.
headius committed Jul 9, 2016
1 parent d15e35a commit 7718d51
Showing 2 changed files with 162 additions and 53 deletions.
10 changes: 10 additions & 0 deletions core/src/main/java/org/jruby/java/dispatch/CallableSelector.java
Original file line number Diff line number Diff line change
@@ -98,6 +98,16 @@ public static <T extends JavaCallable> T matchingCallableArityN(Ruby runtime, Ru
return method;
}

public static <T extends JavaCallable> T matchingCallableArityZero(Ruby runtime, RubyToJavaInvoker<T> invoker, T[] methods) {
final int signatureCode = 0;
T method = invoker.getSignature(signatureCode);
if (method == null) {
method = findMatchingCallableForArgs(runtime, methods);
if (method != null) invoker.putSignature(signatureCode, method);
}
return method;
}

public static <T extends JavaCallable> T matchingCallableArityOne(Ruby runtime, RubyToJavaInvoker<T> invoker, T[] methods, IRubyObject arg0) {
final int signatureCode = argsHashCode(arg0);
T method = invoker.getSignature(signatureCode);
205 changes: 152 additions & 53 deletions core/src/main/java/org/jruby/java/invokers/RubyToJavaInvoker.java
Original file line number Diff line number Diff line change
@@ -295,52 +295,40 @@ static <T extends AccessibleObject> T[] setAccessible(T[] accessibles) {
}

protected T findCallable(IRubyObject self, String name, IRubyObject[] args, final int arity) {
switch (arity) {
case 0:
return findCallableArityZero(self, name);
case 1:
return findCallableArityOne(self, name, args[0]);
case 2:
return findCallableArityTwo(self, name, args[0], args[1]);
case 3:
return findCallableArityThree(self, name, args[0], args[1], args[2]);
case 4:
return findCallableArityFour(self, name, args[0], args[1], args[2], args[3]);
}
return findCallableArityN(self, name, args, arity);
}

protected final T findCallableArityZero(IRubyObject self, String name) {
T callable = this.javaCallable;
if ( callable == null ) {
final T[] callablesForArity;
if ( arity >= javaCallables.length || (callablesForArity = javaCallables[arity]) == null ) {
if ( ( callable = matchVarArgsCallableArityN(self, args) ) == null ) {
throw runtime.newArgumentError(args.length, javaCallables.length - 1);
if ( javaCallables.length == 0 || (callablesForArity = javaCallables[0]) == null ) {
if ( ( callable = matchVarArgsCallableArityZero(self) ) == null ) {
throw newErrorDueNoMatchingCallable(self, name);
}
return callable;
}
callable = CallableSelector.matchingCallableArityN(runtime, this, callablesForArity, args);
callable = CallableSelector.matchingCallableArityZero(runtime, this, callablesForArity);
if ( callable == null ) {
if ( ( callable = matchVarArgsCallableArityN(self, args) ) == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, args);
if ((callable = matchVarArgsCallableArityZero(self)) == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity);
}
}
}
else {
if ( ! callable.isVarArgs() ) checkCallableArity(callable, args.length);
}
return callable;
}

private T matchVarArgsCallableArityN(IRubyObject self, IRubyObject[] args) {
final T[] varArgsCallables = this.javaVarargsCallables;
if ( varArgsCallables != null ) {
T callable = CallableSelector.matchingCallableArityN(runtime, this, varArgsCallables, args);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, varArgsCallables, args);
}
return callable;
}
return null;
}

protected final T findCallableArityZero(IRubyObject self, String name) {
T callable = this.javaCallable;
if ( callable == null ) {
// TODO: varargs?
final T[] callablesForArity;
if ( javaCallables.length == 0 || (callablesForArity = javaCallables[0]) == null ) {
throw newErrorDueNoMatchingCallable(self, name);
}
callable = callablesForArity[0];
}
else {
checkCallableArity(callable, 0);
if (!callable.isVarArgs()) checkCallableArity(callable, 0);
}
return callable;
}
@@ -351,15 +339,19 @@ protected final T findCallableArityOne(IRubyObject self, String name, IRubyObjec
// TODO: varargs?
final T[] callablesForArity;
if ( javaCallables.length <= 1 || (callablesForArity = javaCallables[1]) == null ) {
throw runtime.newArgumentError(1, javaCallables.length - 1);
if ((callable = matchVarArgsCallableArityOne(self, arg0)) == null) {
throw runtime.newArgumentError(1, javaCallables.length - 1);
}
return callable;
}
callable = CallableSelector.matchingCallableArityOne(runtime, this, callablesForArity, arg0);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, arg0);
if ((callable = matchVarArgsCallableArityOne(self, arg0)) == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, arg0);
}
}
}
else {
checkCallableArity(callable, 1);
} else {
if (!callable.isVarArgs()) checkCallableArity(callable, 1);
}
return callable;
}
@@ -370,15 +362,19 @@ protected final T findCallableArityTwo(IRubyObject self, String name, IRubyObjec
// TODO: varargs?
final T[] callablesForArity;
if ( javaCallables.length <= 2 || (callablesForArity = javaCallables[2]) == null ) {
throw runtime.newArgumentError(2, javaCallables.length - 1);
if ((callable = matchVarArgsCallableArityTwo(self, arg0, arg1)) == null ) {
throw runtime.newArgumentError(2, javaCallables.length - 1);
}
return callable;
}
callable = CallableSelector.matchingCallableArityTwo(runtime, this, callablesForArity, arg0, arg1);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, arg0, arg1);
if ((callable = matchVarArgsCallableArityTwo(self, arg0, arg1)) == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, arg0, arg1);
}
}
}
else {
checkCallableArity(callable, 2);
} else {
if (!callable.isVarArgs()) checkCallableArity(callable, 2);
}
return callable;
}
@@ -389,15 +385,19 @@ protected final T findCallableArityThree(IRubyObject self, String name, IRubyObj
// TODO: varargs?
final T[] callablesForArity;
if ( javaCallables.length <= 3 || (callablesForArity = javaCallables[3]) == null ) {
throw runtime.newArgumentError(3, javaCallables.length - 1);
if ( ( callable = matchVarArgsCallableArityThree(self, arg0, arg1, arg2) ) == null ) {
throw runtime.newArgumentError(3, javaCallables.length - 1);
}
return callable;
}
callable = CallableSelector.matchingCallableArityThree(runtime, this, callablesForArity, arg0, arg1, arg2);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, arg0, arg1, arg2);
if ( ( callable = matchVarArgsCallableArityThree(self, arg0, arg1, arg2) ) == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, arg0, arg1, arg2);
}
}
}
else {
checkCallableArity(callable, 3);
} else {
if (!callable.isVarArgs()) checkCallableArity(callable, 3);
}
return callable;
}
@@ -408,19 +408,118 @@ protected final T findCallableArityFour(IRubyObject self, String name, IRubyObje
// TODO: varargs?
final T[] callablesForArity;
if ( javaCallables.length <= 4 || (callablesForArity = javaCallables[4]) == null ) {
throw runtime.newArgumentError(4, javaCallables.length - 1);
if ( ( callable = matchVarArgsCallableArityFour(self, arg0, arg1, arg2, arg3) ) == null ) {
throw runtime.newArgumentError(4, javaCallables.length - 1);
}
return callable;
}
callable = CallableSelector.matchingCallableArityFour(runtime, this, callablesForArity, arg0, arg1, arg2, arg3);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, arg0, arg1, arg2, arg3);
if ( ( callable = matchVarArgsCallableArityFour(self, arg0, arg1, arg2, arg3) ) == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, arg0, arg1, arg2, arg3);
}
}
} else {
if (!callable.isVarArgs()) checkCallableArity(callable, 4);
}
return callable;
}

private T findCallableArityN(IRubyObject self, String name, IRubyObject[] args, int arity) {
T callable = this.javaCallable;
if ( callable == null ) {
final T[] callablesForArity;
if ( arity >= javaCallables.length || (callablesForArity = javaCallables[arity]) == null ) {
if ( ( callable = matchVarArgsCallableArityN(self, args) ) == null ) {
throw runtime.newArgumentError(args.length, javaCallables.length - 1);
}
return callable;
}
callable = CallableSelector.matchingCallableArityN(runtime, this, callablesForArity, args);
if ( callable == null ) {
if ( ( callable = matchVarArgsCallableArityN(self, args) ) == null ) {
throw newErrorDueArgumentTypeMismatch(self, callablesForArity, args);
}
}
}
else {
checkCallableArity(callable, 4);
if (!callable.isVarArgs()) checkCallableArity(callable, args.length);
}
return callable;
}

private T matchVarArgsCallableArityZero(IRubyObject self) {
final T[] varArgsCallables = this.javaVarargsCallables;
if ( varArgsCallables != null ) {
T callable = CallableSelector.matchingCallableArityZero(runtime, this, varArgsCallables);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, varArgsCallables);
}
return callable;
}
return null;
}

private T matchVarArgsCallableArityOne(IRubyObject self, IRubyObject arg0) {
final T[] varArgsCallables = this.javaVarargsCallables;
if ( varArgsCallables != null ) {
T callable = CallableSelector.matchingCallableArityOne(runtime, this, varArgsCallables, arg0);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, varArgsCallables, arg0);
}
return callable;
}
return null;
}

private T matchVarArgsCallableArityTwo(IRubyObject self, IRubyObject arg0, IRubyObject arg1) {
final T[] varArgsCallables = this.javaVarargsCallables;
if ( varArgsCallables != null ) {
T callable = CallableSelector.matchingCallableArityTwo(runtime, this, varArgsCallables, arg0, arg1);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, varArgsCallables, arg0, arg1);
}
return callable;
}
return null;
}

private T matchVarArgsCallableArityThree(IRubyObject self, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2) {
final T[] varArgsCallables = this.javaVarargsCallables;
if ( varArgsCallables != null ) {
T callable = CallableSelector.matchingCallableArityThree(runtime, this, varArgsCallables, arg0, arg1, arg2);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, varArgsCallables, arg0, arg1, arg2);
}
return callable;
}
return null;
}

private T matchVarArgsCallableArityFour(IRubyObject self, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2, IRubyObject arg3) {
final T[] varArgsCallables = this.javaVarargsCallables;
if ( varArgsCallables != null ) {
T callable = CallableSelector.matchingCallableArityFour(runtime, this, varArgsCallables, arg0, arg1, arg2, arg3);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, varArgsCallables, arg0, arg1, arg2, arg3);
}
return callable;
}
return null;
}

private T matchVarArgsCallableArityN(IRubyObject self, IRubyObject[] args) {
final T[] varArgsCallables = this.javaVarargsCallables;
if ( varArgsCallables != null ) {
T callable = CallableSelector.matchingCallableArityN(runtime, this, varArgsCallables, args);
if ( callable == null ) {
throw newErrorDueArgumentTypeMismatch(self, varArgsCallables, args);
}
return callable;
}
return null;
}

private void checkCallableArity(final T callable, final int expected) {
final int arity = callable.getArity();
if ( arity != expected ) throw runtime.newArgumentError(expected, arity);

0 comments on commit 7718d51

Please sign in to comment.