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

Commits on Jul 8, 2016

  1. Lexical constant searches should not stack targets.

    This logic was flawed. We should only stack up targets when doing
    a hierarchy search, since the LHS of a :: may change over time.
    Invalidation globally should throw the old target away, since it
    will never be executed again.
    headius committed Jul 8, 2016
    Copy the full SHA
    5763968 View commit details

Commits on Jul 9, 2016

  1. Copy the full SHA
    9778b8d View commit details
  2. Copy the full SHA
    1ae29af View commit details
  3. Rework constant caching in indy.

    This fixes #4003. I have rewritten the caching and invalidation to
    work like a call site, with bailout limits on global invalidation
    and type mismatch.
    headius committed Jul 9, 2016
    Copy the full SHA
    d15e35a View commit details
18 changes: 18 additions & 0 deletions core/src/main/java/org/jruby/RubyModule.java
Original file line number Diff line number Diff line change
@@ -37,8 +37,11 @@
***** END LICENSE BLOCK *****/
package org.jruby;

import com.headius.invokebinder.Binder;
import org.jcodings.Encoding;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
@@ -83,6 +86,7 @@
import org.jruby.ir.IRClosure;
import org.jruby.ir.IRMethod;
import org.jruby.ir.runtime.IRRuntimeHelpers;
import org.jruby.ir.targets.Bootstrap;
import org.jruby.javasupport.binding.Initializer;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Block;
@@ -300,7 +304,13 @@ public void addIncludingHierarchy(IncludedModule hierarchy) {
*/
protected RubyModule(Ruby runtime, RubyClass metaClass, boolean objectSpace) {
super(runtime, metaClass, objectSpace);

id = runtime.allocModuleId();

idTest = Binder.from(boolean.class, ThreadContext.class, IRubyObject.class)
.insert(2,id)
.invokeStaticQuiet(LOOKUP, Bootstrap.class, "testModuleMatch");

runtime.addModule(this);
// if (parent == null) parent = runtime.getObject();
setFlag(NEEDSIMPL_F, !isClass());
@@ -4474,6 +4484,12 @@ public IRubyObject initialize(Block block) {

public final int id;

/**
* Pre-built test that takes ThreadContext, IRubyObject and checks that the object is a module with the
* same ID as this one.
*/
public final MethodHandle idTest;

/**
* The class/module within whose namespace this class/module resides.
*/
@@ -4755,4 +4771,6 @@ public void setRefinements(Map<RubyClass, RubyModule> refinements) {

/** Whether this class proxies a normal Java class */
private boolean javaProxy = false;

private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
}
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/targets/Bootstrap.java
Original file line number Diff line number Diff line change
@@ -816,7 +816,7 @@ public static IRubyObject instVarNullToNil(IRubyObject value, IRubyObject nil, S
return value;
}

public static boolean testArg0ModuleMatch(IRubyObject arg0, int id) {
public static boolean testModuleMatch(ThreadContext context, IRubyObject arg0, int id) {
return arg0 instanceof RubyModule && ((RubyModule)arg0).id == id;
}

218 changes: 157 additions & 61 deletions core/src/main/java/org/jruby/ir/targets/ConstantLookupSite.java
Original file line number Diff line number Diff line change
@@ -3,11 +3,11 @@
import com.headius.invokebinder.Binder;
import org.jruby.Ruby;
import org.jruby.RubyModule;
import org.jruby.RubySymbol;
import org.jruby.ir.operands.UndefinedValue;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.util.CodegenUtils;
import org.jruby.util.cli.Options;
import org.jruby.util.log.Logger;
import org.jruby.util.log.LoggerFactory;
@@ -32,20 +32,22 @@ public class ConstantLookupSite extends MutableCallSite {
private static final Logger LOG = LoggerFactory.getLogger(ConstantLookupSite.class);
private final String name;
private final boolean publicOnly;
private final MethodHandles.Lookup lookup;

private volatile RubySymbol symbolicName;

private final SiteTracker tracker = new SiteTracker();

public static final Handle BOOTSTRAP = new Handle(Opcodes.H_INVOKESTATIC, p(ConstantLookupSite.class), "constLookup", sig(CallSite.class, MethodHandles.Lookup.class, String.class, MethodType.class, String.class, int.class));

public ConstantLookupSite(MethodHandles.Lookup lookup, MethodType type, String name, boolean publicOnly) {
public ConstantLookupSite(MethodType type, String name, boolean publicOnly) {
super(type);

this.name = name;
this.publicOnly = publicOnly;
this.lookup = lookup;
}

public static CallSite constLookup(MethodHandles.Lookup lookup, String searchType, MethodType type, String constName, int publicOnly) {
ConstantLookupSite site = new ConstantLookupSite(lookup, type, constName, publicOnly == 0 ? false : true);
ConstantLookupSite site = new ConstantLookupSite(type, constName, publicOnly == 0 ? false : true);

MethodHandle handle = Binder
.from(lookup, type)
@@ -57,6 +59,12 @@ public static CallSite constLookup(MethodHandles.Lookup lookup, String searchTyp
return site;
}

private RubySymbol getSymbolicName(ThreadContext context) {
RubySymbol symbolicName = this.symbolicName;
if (symbolicName != null) return symbolicName;
return this.symbolicName = context.runtime.fastNewSymbol(name);
}

public IRubyObject searchConst(ThreadContext context, StaticScope staticScope) {
// Lexical lookup
Ruby runtime = context.getRuntime();
@@ -73,7 +81,7 @@ public IRubyObject searchConst(ThreadContext context, StaticScope staticScope) {

// Call const_missing or cache
if (constant == null) {
return module.callMethod(context, "const_missing", context.runtime.fastNewSymbol(name));
return module.callMethod(context, "const_missing", getSymbolicName(context));
}

SwitchPoint switchPoint = (SwitchPoint) runtime.getConstantInvalidator(name).getData();
@@ -82,65 +90,89 @@ public IRubyObject searchConst(ThreadContext context, StaticScope staticScope) {
MethodHandle target = Binder.from(type())
.drop(0, 2)
.constant(constant);
MethodHandle fallback = getTarget();
if (fallback == null) {
fallback = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "searchConst");
}
MethodHandle fallback = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "searchConst");

setTarget(switchPoint.guardWithTest(target, fallback));

if (Options.INVOKEDYNAMIC_LOG_CONSTANTS.load()) {
LOG.info(name + "\tretrieved and cached from scope " + staticScope.getIRScope());// + " added to PIC" + extractSourceInfo(site));
LOG.info(name + "\tretrieved and cached from scope " + staticScope.getIRScope());
}

return constant;
}

public IRubyObject searchModuleForConst(ThreadContext context, IRubyObject cmVal) {
// Inheritance lookup
public IRubyObject searchModuleForConst(ThreadContext context, IRubyObject cmVal) throws Throwable {
RubyModule module = (RubyModule) cmVal;

if (checkForBailout(module)) {
return bail(context, cmVal, noCacheSMFC());
}

// Inheritance lookup
Ruby runtime = context.getRuntime();
IRubyObject constant = publicOnly ? module.getConstantFromNoConstMissing(name, false) : module.getConstantNoConstMissing(name);

// Call const_missing or cache
if (constant == null) {
return module.callMethod(context, "const_missing", context.runtime.fastNewSymbol(name));
return module.callMethod(context, "const_missing", getSymbolicName(context));
}

SwitchPoint switchPoint = (SwitchPoint) runtime.getConstantInvalidator(name).getData();

// bind constant until invalidated
MethodHandle target = Binder.from(type())
.drop(0, 2)
.constant(constant);
bind(runtime, module, constant, SMFC());

MethodHandle fallback = getTarget();
if (fallback == null) {
fallback = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "searchModuleForConst");
return constant;
}

public IRubyObject noCacheSearchModuleForConst(ThreadContext context, IRubyObject cmVal) {
RubyModule module = (RubyModule) cmVal;

// Inheritance lookup
IRubyObject constant = publicOnly ? module.getConstantFromNoConstMissing(name, false) : module.getConstantNoConstMissing(name);

// Call const_missing or cache
if (constant == null) {
return module.callMethod(context, "const_missing", getSymbolicName(context));
}

// test that module is same as before
MethodHandle test = Binder.from(type().changeReturnType(boolean.class))
.drop(0, 1)
.insert(1, module.id)
.invokeStaticQuiet(Bootstrap.LOOKUP, Bootstrap.class, "testArg0ModuleMatch");
return constant;
}

target = guardWithTest(test, target, fallback);
public IRubyObject inheritanceSearchConst(ThreadContext context, IRubyObject cmVal) throws Throwable {
Ruby runtime = context.runtime;
RubyModule module;

setTarget(switchPoint.guardWithTest(target, fallback));
if (cmVal instanceof RubyModule) {
module = (RubyModule) cmVal;
} else {
throw runtime.newTypeError(cmVal + " is not a type/class");
}

if (checkForBailout(module)) {
return bail(context, cmVal, noCacheISC());
}

// Inheritance lookup
IRubyObject constant = module.getConstantNoConstMissingSKipAutoload(name);

if (constant == null) {
constant = UndefinedValue.UNDEFINED;
}

// bind constant until invalidated
bind(runtime, module, constant, ISC());

tracker.addType(module.id);

if (Options.INVOKEDYNAMIC_LOG_CONSTANTS.load()) {
LOG.info(name + "\tretrieved and cached from module " + module);// + " added to PIC" + extractSourceInfo(site));
LOG.info(name + "\tconstant cached from type " + cmVal.getMetaClass());
}

return constant;
}

public IRubyObject inheritanceSearchConst(ThreadContext context, IRubyObject cmVal) {
public IRubyObject noCacheInheritanceSearchConst(ThreadContext context, IRubyObject cmVal) {
Ruby runtime = context.runtime;
RubyModule module;

@@ -150,41 +182,76 @@ public IRubyObject inheritanceSearchConst(ThreadContext context, IRubyObject cmV
throw runtime.newTypeError(cmVal + " is not a type/class");
}

// Inheritance lookup

IRubyObject constant = module.getConstantNoConstMissingSKipAutoload(name);

if (constant == null) {
constant = UndefinedValue.UNDEFINED;
}

SwitchPoint switchPoint = (SwitchPoint) runtime.getConstantInvalidator(name).getData();
return constant;
}

// bind constant until invalidated
private MethodHandle getFallback(RubyModule module, MethodHandle cachingFallback) {
MethodHandle fallback;// if we've cached any types and we haven't seen this new type...
if (tracker.seenTypesCount() > 0 && !tracker.hasSeenType(module.id)) {

// stack it up into a PIC
if (Options.INVOKEDYNAMIC_LOG_CONSTANTS.load()) LOG.info(name + "\tconstant added to PIC");

fallback = getTarget();

} else {

// wipe out site with this new type
String bound = tracker.seenTypesCount() > 0 ? "rebound" : "bound";
if (Options.INVOKEDYNAMIC_LOG_CONSTANTS.load()) LOG.info(name + "\tconstant " + bound);

fallback = cachingFallback;
tracker.clearTypes();
}
return fallback;
}

private boolean checkForBailout(RubyModule module) {
// Invalidated too many times
if (tracker.clearCount() > Options.INVOKEDYNAMIC_MAXFAIL.load()) {
if (Options.INVOKEDYNAMIC_LOG_CONSTANTS.load()) LOG.info(name + "\tinvalidated more than " + Options.INVOKEDYNAMIC_MAXFAIL.load() + " times ");
return true;
}

// Too many types encountered
if ((!tracker.hasSeenType(module.id) && tracker.seenTypesCount() + 1 > Options.INVOKEDYNAMIC_MAXPOLY.load())) {
if (Options.INVOKEDYNAMIC_LOG_CONSTANTS.load()) LOG.info(name + "\tencountered more than " + Options.INVOKEDYNAMIC_MAXPOLY.load() + " types ");
return true;
}

return false;
}

private IRubyObject bail(ThreadContext context, IRubyObject cmVal, MethodHandle noncachingFallback) throws Throwable {
setTarget(noncachingFallback);
return (IRubyObject) noncachingFallback.invokeExact(context, cmVal);
}

private void bind(Ruby runtime, RubyModule module, IRubyObject constant, MethodHandle cachingFallback) {
MethodHandle target = Binder.from(type())
.drop(0, 2)
.constant(constant);

MethodHandle fallback = getTarget();
if (fallback == null) {
fallback = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "inheritanceSearchConst");
}
// Get appropriate fallback given state of site
MethodHandle fallback = getFallback(module, cachingFallback);

// test that module is same as before
MethodHandle test = Binder.from(type().changeReturnType(boolean.class))
.drop(0, 1)
.insert(1, module.id)
.invokeStaticQuiet(Bootstrap.LOOKUP, Bootstrap.class, "testArg0ModuleMatch");
// Test that module is same as before
target = guardWithTest(module.idTest, target, fallback);

target = guardWithTest(test, target, fallback);

setTarget(switchPoint.guardWithTest(target, fallback));
// Global invalidation
SwitchPoint switchPoint = (SwitchPoint) runtime.getConstantInvalidator(name).getData();

if (Options.INVOKEDYNAMIC_LOG_CONSTANTS.load()) {
LOG.info(name + "\tretrieved and cached from type " + cmVal.getMetaClass());// + " added to PIC" + extractSourceInfo(site));
}
target = switchPoint.guardWithTest(target, fallback);

return constant;
setTarget(target);
}

public IRubyObject lexicalSearchConst(ThreadContext context, StaticScope scope) {
@@ -203,12 +270,9 @@ public IRubyObject lexicalSearchConst(ThreadContext context, StaticScope scope)
.drop(0, 2)
.constant(constant);

MethodHandle fallback = getTarget();
if (fallback == null) {
fallback = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "lexicalSearchConst");
}
MethodHandle fallback = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "lexicalSearchConst");

setTarget(switchPoint.guardWithTest(target, fallback));

@@ -218,4 +282,36 @@ public IRubyObject lexicalSearchConst(ThreadContext context, StaticScope scope)

return constant;
}

private MethodHandle _SMFC;
private MethodHandle SMFC() {
if (_SMFC != null) return _SMFC;
return _SMFC = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "searchModuleForConst");
}

private MethodHandle _noCacheSMFC;
private MethodHandle noCacheSMFC() {
if (_noCacheSMFC != null) return _noCacheSMFC;
return _noCacheSMFC = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "noCacheSearchModuleForConst");
}

private MethodHandle _ISC;
private MethodHandle ISC() {
if (_ISC != null) return _ISC;
return _ISC = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "inheritanceSearchConst");
}

private MethodHandle _noCacheISC;
private MethodHandle noCacheISC() {
if (_noCacheISC != null) return _noCacheISC;
return _noCacheISC = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "noCacheInheritanceSearchConst");
}
}
42 changes: 16 additions & 26 deletions core/src/main/java/org/jruby/ir/targets/InvokeSite.java
Original file line number Diff line number Diff line change
@@ -48,8 +48,7 @@ public abstract class InvokeSite extends MutableCallSite {
final int arity;
protected final String methodName;
final MethodHandle fallback;
private final Set<Integer> seenTypes = new HashSet<Integer>();
private int clearCount;
private final SiteTracker tracker = new SiteTracker();
private static final AtomicLong SITE_ID = new AtomicLong(1);
private final long siteID = SITE_ID.getAndIncrement();
private final int argOffset;
@@ -231,21 +230,29 @@ MethodHandle buildNewInstanceHandle(DynamicMethod method, IRubyObject self, bool

/**
* Update the given call site using the new target, wrapping with appropriate
* guard and argument-juggling logic. Return a handle suitable for invoking
* bind and argument-juggling logic. Return a handle suitable for invoking
* with the site's original method type.
*/
MethodHandle updateInvocationTarget(MethodHandle target, IRubyObject self, RubyModule testClass, DynamicMethod method, SwitchPoint switchPoint) {
if (target == null ||
clearCount > Options.INVOKEDYNAMIC_MAXFAIL.load() ||
(!hasSeenType(testClass.id)
&& seenTypesCount() + 1 > Options.INVOKEDYNAMIC_MAXPOLY.load())) {
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 + ")");
}
}
setTarget(target = prepareBinder().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 (seenTypesCount() > 0 && getTarget() != null && !hasSeenType(testClass.id)) {
if (tracker.seenTypesCount() > 0 && getTarget() != null && !tracker.hasSeenType(testClass.id)) {
// stack it up into a PIC
if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) LOG.info(methodName + "\tadded to PIC " + logMethod(method));
fallback = getTarget();
@@ -254,10 +261,10 @@ && seenTypesCount() + 1 > Options.INVOKEDYNAMIC_MAXPOLY.load())) {
String bind = boundOnce ? "rebind" : "bind";
if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) LOG.info(methodName + "\ttriggered site #" + siteID + " " + bind + " (" + file + ":" + line + ")");
fallback = this.fallback;
clearTypes();
tracker.clearTypes();
}

addType(testClass.id);
tracker.addType(testClass.id);

SmartHandle test;
SmartBinder selfTest = SmartBinder
@@ -312,23 +319,6 @@ public void setInitialTarget(MethodHandle target) {
super.setTarget(target);
}

public synchronized boolean hasSeenType(int typeCode) {
return seenTypes.contains(typeCode);
}

public synchronized void addType(int typeCode) {
seenTypes.add(typeCode);
}

public synchronized int seenTypesCount() {
return seenTypes.size();
}

public synchronized void clearTypes() {
seenTypes.clear();
clearCount++;
}

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

public IRubyObject callMethodMissing(CacheEntry entry, CallType callType, ThreadContext context, IRubyObject self, String name, IRubyObject[] args, Block block) {
33 changes: 33 additions & 0 deletions core/src/main/java/org/jruby/ir/targets/SiteTracker.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.jruby.ir.targets;

import java.util.HashSet;
import java.util.Set;

/**
* Created by headius on 7/8/16.
*/
public class SiteTracker {
private final Set<Integer> seenTypes = new HashSet<Integer>();
private volatile int clearCount = 0;

public synchronized boolean hasSeenType(int typeCode) {
return seenTypes.contains(typeCode);
}

public synchronized void addType(int typeCode) {
seenTypes.add(typeCode);
}

public synchronized int seenTypesCount() {
return seenTypes.size();
}

public synchronized void clearTypes() {
seenTypes.clear();
clearCount++;
}

public int clearCount() {
return clearCount;
}
}