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: 77e3fa79abad
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: ef2836714177
Choose a head ref
  • 2 commits
  • 1 file changed
  • 1 contributor

Commits on May 10, 2017

  1. Also increment thresholds in indy sites for method_missing. 

    Fixes #4596.

JRuby currently does not cache anything for method_missing calls
(i.e. calls to methods that do not exist on the target class's
method table). Instead, after discovering the missing method, it
proceeds down a slow lookup path, dispatching to method_missing
with a symbolic method name.

However this logic still attempts to acquire a SwitchPoint from
the target class, since that SP must be acquired before the
initial method lookup to ensure proper invalidation ordering.

Normally, this isn't a problem. If the class is still relatively
static, the SwitchPoint will be created once, and then the cost
of calling method_missing is the same as for non-indy. However for
type very rapidly. This led to a large number of unused
SwitchPoint being created, one for each class. And since the
method_missing path never cached anything, it never updated call
site thresholds, resulting in call sites that would continue this
logic forever.

Normal calls, if they fail repeatedly, will eventually fail the
entire site and revert to non-indy method dispatch (currently a
simple monomorphic cache).

This patch lifts the threshold-bumping out of the caching logic
so it can also be used by method_missing dispatches. This allows
repeated method_missing calls against different classes to also
fail the site, reverting it to non-indy behavior. With this patch,
the benchmark in #4596 runs faster with indy than without, as
expected.
    @headius
    headius committed May 10, 2017
    Copy the full SHA
    2c211bf View commit details

  2. Merge pull request #4602 from headius/indy_mm_cache_bust 

    Also increment thresholds in indy sites for method_missing.
    @headius
    headius authored May 10, 2017
    Copy the full SHA
    ef28367 View commit details
Showing with 105 additions and 53 deletions.
  1. +105 −53 core/src/main/java/org/jruby/ir/targets/InvokeSite.java
158 changes: 105 additions & 53 deletions core/src/main/java/org/jruby/ir/targets/InvokeSite.java
View file
Original file line number Diff line number Diff line change
@@ -33,8 +33,6 @@
import java.lang.invoke.MethodType;
import java.lang.invoke.MutableCallSite;
import java.lang.invoke.SwitchPoint;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;

import static java.lang.invoke.MethodHandles.lookup;
@@ -130,6 +128,13 @@ public IRubyObject invoke(ThreadContext context, IRubyObject caller, IRubyObject
DynamicMethod method = entry.method;

if (methodMissing(entry, caller)) {
// Test thresholds so we don't do this forever (#4596)
if (testThresholds(selfClass) == CacheAction.FAIL) {
logFail();
bindToFail();
} else {
logMethodMissing();
}
return callMethodMissing(entry, callType, context, self, methodName, args, block);
}

@@ -318,76 +323,123 @@ MethodHandle buildNewInstanceHandle(DynamicMethod method, IRubyObject self, bool
* with the site's original method type.
*/
MethodHandle updateInvocationTarget(MethodHandle target, IRubyObject self, RubyModule testClass, DynamicMethod method, SwitchPoint switchPoint) {
if (target == null ||
tracker.clearCount() > Options.INVOKEDYNAMIC_MAXFAIL.load() ||
(!tracker.hasSeenType(testClass.id)
&& tracker.seenTypesCount() + 1 > Options.INVOKEDYNAMIC_MAXPOLY.load())) {

if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
if (tracker.clearCount() > Options.INVOKEDYNAMIC_MAXFAIL.load()) {
LOG.info(methodName + "\tat site #" + siteID + " failed more than " + Options.INVOKEDYNAMIC_MAXFAIL.load() + " times; bailing out (" + file + ":" + line + ")");
} else if (tracker.seenTypesCount() + 1 > Options.INVOKEDYNAMIC_MAXPOLY.load()) {
LOG.info(methodName + "\tat site #" + siteID + " encountered more than " + Options.INVOKEDYNAMIC_MAXPOLY.load() + " types; bailing out (" + file + ":" + line + ")");
}
}
// bind to specific-arity fail method if available
setTarget(target = prepareBinder(false).invokeVirtualQuiet(lookup(), "fail"));
} else {
MethodHandle fallback;
MethodHandle gwt;

// if we've cached no types, and the site is bound and we haven't seen this new type...
if (tracker.seenTypesCount() > 0 && getTarget() != null && !tracker.hasSeenType(testClass.id)) {
MethodHandle fallback;
MethodHandle gwt;
String bind = "bind";

CacheAction cacheAction = testThresholds(testClass);
switch (cacheAction) {
case FAIL:
logFail();
// bind to specific-arity fail method if available
return bindToFail();
case PIC:
// stack it up into a PIC
if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) LOG.info(methodName + "\tadded to PIC " + logMethod(method));
logPic(method);
fallback = getTarget();
} else {
break;
case REBIND:
case BIND:
// wipe out site with this new type and method
String bind = boundOnce ? "rebind" : "bind";
if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) LOG.info(methodName + "\ttriggered site #" + siteID + " " + bind + " (" + file + ":" + line + ")");
logBind(cacheAction);
fallback = this.fallback;
tracker.clearTypes();
}
break;
default:
throw new RuntimeException("invalid cache action: " + cacheAction);
}

tracker.addType(testClass.id);
// Continue with logic for PIC, BIND, and REBIND
tracker.addType(testClass.id);

SmartHandle test;
SmartBinder selfTest = SmartBinder
SmartHandle test;

if (self instanceof RubySymbol ||
self instanceof RubyFixnum ||
self instanceof RubyFloat ||
self instanceof RubyNil ||
self instanceof RubyBoolean.True ||
self instanceof RubyBoolean.False) {

test = SmartBinder
.from(signature.asFold(boolean.class))
.permute("self");
.permute("self")
.insert(1, "selfJavaType", self.getClass())
.cast(boolean.class, Object.class, Class.class)
.invoke(TEST_CLASS);

if (self instanceof RubySymbol ||
self instanceof RubyFixnum ||
self instanceof RubyFloat ||
self instanceof RubyNil ||
self instanceof RubyBoolean.True ||
self instanceof RubyBoolean.False) {
} else {

test = selfTest
.insert(1, "selfJavaType", self.getClass())
.cast(boolean.class, Object.class, Class.class)
.invoke(TEST_CLASS);
test = SmartBinder
.from(signature.changeReturn(boolean.class))
.permute("self")
.insert(0, "selfClass", RubyClass.class, testClass)
.invokeStaticQuiet(Bootstrap.LOOKUP, Bootstrap.class, "testType");
}

} else {
gwt = MethodHandles.guardWithTest(test.handle(), target, fallback);

test = SmartBinder
.from(signature.changeReturn(boolean.class))
.permute("self")
.insert(0, "selfClass", RubyClass.class, testClass)
.invokeStaticQuiet(Bootstrap.LOOKUP, Bootstrap.class, "testType");
}
// wrap in switchpoint for mutation invalidation
gwt = switchPoint.guardWithTest(gwt, fallback);

gwt = MethodHandles.guardWithTest(test.handle(), target, fallback);
setTarget(gwt);

// wrap in switchpoint for mutation invalidation
gwt = switchPoint.guardWithTest(gwt, fallback);
return target;
}

private void logMethodMissing() {
if (Options.INVOKEDYNAMIC_LOG_BINDING.load())
LOG.info(methodName + "\ttriggered site #" + siteID + " method_missing (" + file + ":" + line + ")");
}

private void logBind(CacheAction action) {
if (Options.INVOKEDYNAMIC_LOG_BINDING.load())
LOG.info(methodName + "\ttriggered site #" + siteID + " " + action + " (" + file + ":" + line + ")");
}

private void logPic(DynamicMethod method) {
if (Options.INVOKEDYNAMIC_LOG_BINDING.load())
LOG.info(methodName + "\tadded to PIC " + logMethod(method));
}

setTarget(gwt);
private void logFail() {
if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
if (tracker.clearCount() > Options.INVOKEDYNAMIC_MAXFAIL.load()) {
LOG.info(methodName + "\tat site #" + siteID + " failed more than " + Options.INVOKEDYNAMIC_MAXFAIL.load() + " times; bailing out (" + file + ":" + line + ")");
} else if (tracker.seenTypesCount() + 1 > Options.INVOKEDYNAMIC_MAXPOLY.load()) {
LOG.info(methodName + "\tat site #" + siteID + " encountered more than " + Options.INVOKEDYNAMIC_MAXPOLY.load() + " types; bailing out (" + file + ":" + line + ")");
}
}
}

private MethodHandle bindToFail() {
MethodHandle target;
setTarget(target = prepareBinder(false).invokeVirtualQuiet(lookup(), "fail"));
return target;
}

enum CacheAction { FAIL, BIND, REBIND, PIC }

CacheAction testThresholds(RubyModule testClass) {
if (tracker.clearCount() > Options.INVOKEDYNAMIC_MAXFAIL.load() ||
(!tracker.hasSeenType(testClass.id)
&& tracker.seenTypesCount() + 1 > Options.INVOKEDYNAMIC_MAXPOLY.load())) {
// Thresholds exceeded
return CacheAction.FAIL;
} else {
// if we've cached no types, and the site is bound and we haven't seen this new type...
if (tracker.seenTypesCount() > 0 && getTarget() != null && !tracker.hasSeenType(testClass.id)) {
// stack it up into a PIC
tracker.addType(testClass.id);
return CacheAction.PIC;
} else {
// wipe out site with this new type and method
tracker.clearTypes();
tracker.addType(testClass.id);
return boundOnce ? CacheAction.REBIND : CacheAction.BIND;
}
}
}

public RubyClass pollAndGetClass(ThreadContext context, IRubyObject self) {
context.callThreadPoll();
RubyClass selfType = ((RubyBasicObject)self).getMetaClass();