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: 75a39d3cf66c
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 5a212761c26d
Choose a head ref
  • 3 commits
  • 6 files changed
  • 1 contributor

Commits on Feb 21, 2017

  1. Separate varargs and specific-arity names in JIT. Fixes #4482

    Originally, varargs and specific-arity paths were emitted in toto
    as their own JVM method bodies. With recent changes, the varargs
    path now calls the specific-arity path, causing an extra
    synthetic entry in the JVM stack trace. This confounded the
    backtrace miner, since it had no marker with which to omit the
    synthetic version, and so Kernel#caller and friends contained an
    additional invalid entry. This in turn caused require_relative to
    fail to find the caller's path, resulting in a nil path value
    passed to File.realpath, which triggered this bug.
    
    It likely only affected invokedynamic because our indy logic must
    be calling no-arg methods via the varargs path (a separate issue
    to be fixed).
    
    My modification here explicitly separates the synthetic varargs
    name from the specific-arity name by adding a known varargs marker
    that can be filtered out when mining the backtrace.
    
    This is a big hacky; the state being managed by the jit is very
    ad-hoc and prone to bugs. It will need some additional cleanup
    after 9.1.8.0.
    headius committed Feb 21, 2017
    Copy the full SHA
    18cde0d View commit details
  2. Allow indirect indy calls to still call specific arity up to 3.

    Previously, indirect indy calls would always box arguments, which
    affects failed sites, Java sites, and others. This fixes #4499 by
    setting up fail paths for arities up to 3 arguments, same as the
    non-indy call paths.
    headius committed Feb 21, 2017
    Copy the full SHA
    2723012 View commit details

Commits on Feb 22, 2017

  1. Merge pull request #4498 from headius/isolate-arity-names

    Separate varargs and specific-arity names in JIT. Fixes #4482
    headius authored Feb 22, 2017
    Copy the full SHA
    5a21276 View commit details
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/compiler/BlockJITTask.java
Original file line number Diff line number Diff line change
@@ -76,7 +76,7 @@ public void run() {
JITCompiler.log(body.getImplementationClass(), body.getFile(), body.getLine(), className + "." + methodName, "done jitting");
}

String jittedName = context.getJittedName();
String jittedName = context.getVariableName();

// blocks only have variable-arity
body.completeBuild(
6 changes: 3 additions & 3 deletions core/src/main/java/org/jruby/compiler/MethodJITTask.java
Original file line number Diff line number Diff line change
@@ -112,8 +112,8 @@ public void run() {
JITCompiler.log(method.getImplementationClass(), method.getFile(), method.getLine(), className + '.' + methodName, "done jitting");
}

final String jittedName = context.getJittedName();
MethodHandle variable = JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, jittedName, context.getNativeSignature(-1));
String variableName = context.getVariableName();
MethodHandle variable = JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, variableName, context.getNativeSignature(-1));
IntHashMap<MethodType> signatures = context.getNativeSignaturesExceptVariable();

if (signatures.size() == 0) {
@@ -132,7 +132,7 @@ public void run() {
method.completeBuild(
new CompiledIRMethod(
variable,
JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, jittedName, entry.getValue()),
JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, context.getSpecificName(), entry.getValue()),
entry.getKey(),
method.getIRScope(),
method.getVisibility(),
133 changes: 122 additions & 11 deletions core/src/main/java/org/jruby/ir/targets/InvokeSite.java
Original file line number Diff line number Diff line change
@@ -114,7 +114,7 @@ public InvokeSite(MethodType type, String name, CallType callType, String file,

this.arity = arity;

this.fallback = prepareBinder().invokeVirtualQuiet(Bootstrap.LOOKUP, "invoke");
this.fallback = prepareBinder(true).invokeVirtualQuiet(Bootstrap.LOOKUP, "invoke");
}

public static CallSite bootstrap(InvokeSite site, MethodHandles.Lookup lookup) {
@@ -163,17 +163,101 @@ public IRubyObject fail(ThreadContext context, IRubyObject caller, IRubyObject s
return entry.method.call(context, self, selfClass, name, args, block);
}

public Binder prepareBinder() {
/**
* Failover version uses a monomorphic cache and DynamicMethod.call, as in non-indy.
*/
public IRubyObject fail(ThreadContext context, IRubyObject caller, IRubyObject self, Block block) throws Throwable {
return fail(context, caller, self, IRubyObject.NULL_ARRAY, block);
}

/**
* Failover version uses a monomorphic cache and DynamicMethod.call, as in non-indy.
*/
public IRubyObject fail(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg0, Block block) throws Throwable {
RubyClass selfClass = pollAndGetClass(context, self);
String name = methodName;
CacheEntry entry = cache;

if (entry.typeOk(selfClass)) {
return entry.method.call(context, self, selfClass, name, arg0, block);
}

entry = selfClass.searchWithCache(name);

if (methodMissing(entry, caller)) {
return callMethodMissing(entry, callType, context, self, name, arg0, block);
}

cache = entry;

return entry.method.call(context, self, selfClass, name, arg0, block);
}

/**
* Failover version uses a monomorphic cache and DynamicMethod.call, as in non-indy.
*/
public IRubyObject fail(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg0, IRubyObject arg1, Block block) throws Throwable {
RubyClass selfClass = pollAndGetClass(context, self);
String name = methodName;
CacheEntry entry = cache;

if (entry.typeOk(selfClass)) {
return entry.method.call(context, self, selfClass, name, arg0, arg1, block);
}

entry = selfClass.searchWithCache(name);

if (methodMissing(entry, caller)) {
return callMethodMissing(entry, callType, context, self, name, arg0, arg1, block);
}

cache = entry;

return entry.method.call(context, self, selfClass, name, arg0, arg1, block);
}

/**
* Failover version uses a monomorphic cache and DynamicMethod.call, as in non-indy.
*/
public IRubyObject fail(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2, Block block) throws Throwable {
RubyClass selfClass = pollAndGetClass(context, self);
String name = methodName;
CacheEntry entry = cache;

if (entry.typeOk(selfClass)) {
return entry.method.call(context, self, selfClass, name, arg0, arg1, arg2, block);
}

entry = selfClass.searchWithCache(name);

if (methodMissing(entry, caller)) {
return callMethodMissing(entry, callType, context, self, name, arg0, arg1, arg2, block);
}

cache = entry;

return entry.method.call(context, self, selfClass, name, arg0, arg1, arg2, block);
}

/**
* Prepare a binder for this call site's target, forcing varargs if specified
*
* @param varargs whether to only call an arg-boxed variable arity path
* @return the prepared binder
*/
public Binder prepareBinder(boolean varargs) {
SmartBinder binder = SmartBinder.from(signature);

// prepare arg[]
if (arity == -1) {
// do nothing, already have IRubyObject[] in args
} else if (arity == 0) {
binder = binder.insert(argOffset, "args", IRubyObject.NULL_ARRAY);
} else {
binder = binder
.collect("args", "arg[0-9]+");
if (varargs || arity > 3) {
// we know we want to call varargs path always, so prepare args[] here
if (arity == -1) {
// do nothing, already have IRubyObject[] in args
} else if (arity == 0) {
binder = binder.insert(argOffset, "args", IRubyObject.NULL_ARRAY);
} else {
binder = binder
.collect("args", "arg[0-9]+");
}
}

// add block if needed
@@ -246,7 +330,8 @@ MethodHandle updateInvocationTarget(MethodHandle target, IRubyObject self, RubyM
LOG.info(methodName + "\tat site #" + siteID + " encountered more than " + Options.INVOKEDYNAMIC_MAXPOLY.load() + " types; bailing out (" + file + ":" + line + ")");
}
}
setTarget(target = prepareBinder().invokeVirtualQuiet(lookup(), "fail"));
// bind to specific-arity fail method if available
setTarget(target = prepareBinder(false).invokeVirtualQuiet(lookup(), "fail"));
} else {
MethodHandle fallback;
MethodHandle gwt;
@@ -321,10 +406,36 @@ public void setInitialTarget(MethodHandle target) {

public abstract boolean methodMissing(CacheEntry entry, IRubyObject caller);

/**
* Variable arity method_missing invocation. Arity zero also passes through here.
*/
public IRubyObject callMethodMissing(CacheEntry entry, CallType callType, ThreadContext context, IRubyObject self, String name, IRubyObject[] args, Block block) {
return Helpers.selectMethodMissing(context, self, entry.method.getVisibility(), name, callType).call(context, self, self.getMetaClass(), name, args, block);
}

/**
* Arity one method_missing invocation
*/
public IRubyObject callMethodMissing(CacheEntry entry, CallType callType, ThreadContext context, IRubyObject self, String name, IRubyObject arg0, Block block) {
return Helpers.selectMethodMissing(context, self, entry.method.getVisibility(), name, callType).call(context, self, self.getMetaClass(), name, arg0, block);
}


/**
* Arity two method_missing invocation
*/
public IRubyObject callMethodMissing(CacheEntry entry, CallType callType, ThreadContext context, IRubyObject self, String name, IRubyObject arg0, IRubyObject arg1, Block block) {
return Helpers.selectMethodMissing(context, self, entry.method.getVisibility(), name, callType).call(context, self, self.getMetaClass(), name, arg0, arg1, block);
}


/**
* Arity three method_missing invocation
*/
public IRubyObject callMethodMissing(CacheEntry entry, CallType callType, ThreadContext context, IRubyObject self, String name, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2, Block block) {
return Helpers.selectMethodMissing(context, self, entry.method.getVisibility(), name, callType).call(context, self, self.getMetaClass(), name, arg0, arg1, arg2, block);
}

private static String logMethod(DynamicMethod method) {
return "[#" + method.getSerialNumber() + " " + method.getImplementationClass() + "]";
}
48 changes: 31 additions & 17 deletions core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -239,11 +239,9 @@ protected void emitScope(IRScope scope, String name, Signature signature, boolea
jvm.popmethod();
}

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

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

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

IRBytecodeAdapter m = jvmMethod();

@@ -271,8 +269,8 @@ protected void emitVarargsMethodWrapper(IRScope scope, String name, Signature va
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());
Method specificMethod = new Method(specificName, Type.getType(specificSignature.type().returnType()), IRRuntimeHelpers.typesFromSignature(specificSignature));
jvmAdapter().invokestatic(m.getClassData().clsName, specificName, specificMethod.getDescriptor());
jvmAdapter().areturn();

jvm.popmethod();
@@ -337,24 +335,38 @@ protected void emitBlockJIT(IRClosure closure, JVMVisitorMethodContext context)

emitScope(closure, name, CLOSURE_SIGNATURE, false, true);

context.setJittedName(name);
context.setBaseName(name);
context.setVariableName(name);

jvm.cls().visitEnd();
jvm.popclass();
}

private void emitWithSignatures(IRMethod method, JVMVisitorMethodContext context, String name) {
context.setJittedName(name);
context.setBaseName(name);

Signature specificSig = signatureFor(method, true);

if (specificSig == null) {
// only varargs, so use name as is
context.setVariableName(name);
Signature signature = signatureFor(method, false);
emitScope(method, name, signature, false, true);
context.addNativeSignature(-1, signature.type());
} else {
emitScope(method, name, specificSig, true, true);
String specificName = name;

context.setSpecificName(specificName);

emitScope(method, specificName, specificSig, true, true);
context.addNativeSignature(method.getStaticScope().getSignature().required(), specificSig.type());
emitVarargsMethodWrapper(method, name, METHOD_SIGNATURE_VARARGS, specificSig);

// specific arity path, so mangle the dummy varargs wrapper
String variableName = name + JavaNameMangler.VARARGS_MARKER;

context.setVariableName(variableName);

emitVarargsMethodWrapper(method, variableName, specificName, METHOD_SIGNATURE_VARARGS, specificSig);
context.addNativeSignature(-1, METHOD_SIGNATURE_VARARGS.type());
}
}
@@ -1199,13 +1211,14 @@ public void DefineClassMethodInstr(DefineClassMethodInstr defineclassmethodinstr
emitMethod(method, context);

String defSignature = pushHandlesForDef(
context.getJittedName(),
context.getVariableName(),
context.getSpecificName(),
context.getNativeSignaturesExceptVariable(),
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));

jvmAdapter().getstatic(jvm.clsData().clsName, context.getJittedName() + "_IRScope", ci(IRScope.class));
jvmAdapter().getstatic(jvm.clsData().clsName, context.getBaseName() + "_IRScope", ci(IRScope.class));
visit(defineclassmethodinstr.getContainer());

// add method
@@ -1229,32 +1242,33 @@ public void DefineInstanceMethodInstr(DefineInstanceMethodInstr defineinstanceme
assert(variable != null);

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

a.getstatic(jvm.clsData().clsName, context.getJittedName() + "_IRScope", ci(IRScope.class));
a.getstatic(jvm.clsData().clsName, context.getBaseName() + "_IRScope", ci(IRScope.class));
jvmLoadLocal(DYNAMIC_SCOPE);
jvmMethod().loadSelf();

// add method
a.invokestatic(p(IRRuntimeHelpers.class), "defCompiledInstanceMethod", defSignature);
}

private String pushHandlesForDef(String name, IntHashMap<MethodType> signaturesExceptVariable, MethodType variable, String variableOnly, String variableAndSpecific) {
private String pushHandlesForDef(String variableName, String specificName, IntHashMap<MethodType> signaturesExceptVariable, MethodType variable, String variableOnly, String variableAndSpecific) {
String defSignature;

jvmMethod().pushHandle(new Handle(Opcodes.H_INVOKESTATIC, jvm.clsData().clsName, name, sig(variable.returnType(), variable.parameterArray())));
jvmMethod().pushHandle(new Handle(Opcodes.H_INVOKESTATIC, jvm.clsData().clsName, variableName, sig(variable.returnType(), variable.parameterArray())));

if (signaturesExceptVariable.size() == 0) {
defSignature = variableOnly;
} else {
defSignature = variableAndSpecific;

for (IntHashMap.Entry<MethodType> entry : signaturesExceptVariable.entrySet()) {
jvmMethod().pushHandle(new Handle(Opcodes.H_INVOKESTATIC, jvm.clsData().clsName, name, sig(entry.getValue().returnType(), entry.getValue().parameterArray())));
jvmMethod().pushHandle(new Handle(Opcodes.H_INVOKESTATIC, jvm.clsData().clsName, specificName, sig(entry.getValue().returnType(), entry.getValue().parameterArray())));
jvmAdapter().pushInt(entry.getKey());
break; // FIXME: only supports one arity
}
Original file line number Diff line number Diff line change
@@ -8,19 +8,41 @@
* Context for JITing methods. Temporary data.
*/
public class JVMVisitorMethodContext {
// Method name in the jitted version of this method
private String jittedName;
// The base name of this method. It will match specific if non-null, otherwise varargs.
private String baseName;

// Method name in the specific-arity version of this method. May be null
private String specificName;

// Method name in the variable-arity version of this method
private String variableName;

// Signatures to the jitted versions of this method
private IntHashMap<MethodType> signatures;
private MethodType varSignature; // for arity == -1

public void setJittedName(String jittedName) {
this.jittedName = jittedName;
public void setSpecificName(String specificName) {
this.specificName = specificName;
}

public void setVariableName(String variableName) {
this.variableName = variableName;
}

public void setBaseName(String baseName) {
this.baseName = baseName;
}

public String getSpecificName() {
return specificName;
}

public String getVariableName() {
return variableName;
}

public String getJittedName() {
return jittedName;
public String getBaseName() {
return baseName;
}

public void addNativeSignature(int arity, MethodType signature) {
6 changes: 5 additions & 1 deletion core/src/main/java/org/jruby/util/JavaNameMangler.java
Original file line number Diff line number Diff line change
@@ -311,8 +311,12 @@ public static String encodeScopeForBacktrace(IRScope scope) {
throw new IllegalStateException("unknown scope type for backtrace encoding: " + scope.getClass());
}

public static final String VARARGS_MARKER = "$__VARARGS__";

public static String decodeMethodForBacktrace(String methodName) {
if ( ! methodName.startsWith("RUBY$") ) return null;
if (!methodName.startsWith("RUBY$")) return null;

if (methodName.contains(VARARGS_MARKER)) return null;

final List<String> name = StringSupport.split(methodName, '$');
final String type = name.get(1); // e.g. RUBY $ class $ methodName