Skip to content

Commit

Permalink
Lazily create now-unused idTest handle and share method reference.
Browse files Browse the repository at this point in the history
The handle here was originally intended for use in checking that
the target module for a constant cache is the same as when the
value was cached. However the code that used it is no longer
active, but we still created these handles for every module,
class, singleton class, included module, etc in the system.

This commit makes the acquisition of this handle lazy, and also
modifies it to reuse the direct handle reference to the test
method rather than recreating that every time. This should reduce
the number of method handles in flight.

For #4391.
headius committed Jan 26, 2017
1 parent f792cb2 commit 2b97d78
Showing 2 changed files with 16 additions and 5 deletions.
19 changes: 15 additions & 4 deletions core/src/main/java/org/jruby/RubyModule.java
Original file line number Diff line number Diff line change
@@ -299,10 +299,16 @@ public void addIncludingHierarchy(IncludedModule hierarchy) {
}
}

public MethodHandle getIdTest() {
MethodHandle idTest = this.idTest;
if (idTest != null) return idTest;
return idTest = newIdTest();

This comment has been minimized.

Copy link
@kares

kares Jan 26, 2017

Member

it seems like the handle is not meant to be re-created on every getIdTest() call
... return this.idTest = newIdTest();

}

protected MethodHandle newIdTest() {
return Binder.from(boolean.class, ThreadContext.class, IRubyObject.class)
.insert(2,id)
.invokeStaticQuiet(LOOKUP, Bootstrap.class, "testModuleMatch");
.invoke(testModuleMatch);
}

/** separate path for MetaClass construction
@@ -313,8 +319,6 @@ protected RubyModule(Ruby runtime, RubyClass metaClass, boolean objectSpace) {

id = runtime.allocModuleId();

idTest = newIdTest();

runtime.addModule(this);
// if (parent == null) parent = runtime.getObject();
setFlag(NEEDSIMPL_F, !isClass());
@@ -4504,7 +4508,7 @@ public IRubyObject initialize(Block block) {
* 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;
private MethodHandle idTest;

/**
* The class/module within whose namespace this class/module resides.
@@ -4789,4 +4793,11 @@ public void setRefinements(Map<RubyClass, RubyModule> refinements) {
private boolean javaProxy = false;

private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();

/**
* A handle for invoking the module ID test, to be reused for all idTest handles below.
*/
private static final MethodHandle testModuleMatch = Binder
.from(boolean.class, ThreadContext.class, IRubyObject.class, int.class)
.invokeStaticQuiet(LOOKUP, Bootstrap.class, "testModuleMatch");
}
Original file line number Diff line number Diff line change
@@ -244,7 +244,7 @@ private void bind(Ruby runtime, RubyModule module, IRubyObject constant, MethodH
MethodHandle fallback = getFallback(module, cachingFallback);

// Test that module is same as before
target = guardWithTest(module.idTest, target, fallback);
target = guardWithTest(module.getIdTest(), target, fallback);

// Global invalidation
SwitchPoint switchPoint = (SwitchPoint) runtime.getConstantInvalidator(name).getData();

0 comments on commit 2b97d78

Please sign in to comment.