Lazily create now-unused idTest handle and share method reference.

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.
jruby · Jan 26, 2017 · 2b97d78 · kares · Jan 26, 2017 · 2b97d78
1 parent f792cb2
commit 2b97d78
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 5 deletions.
diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -299,10 +299,16 @@ public void addIncludingHierarchy(IncludedModule hierarchy) {
         }
     }
 
+    public MethodHandle getIdTest() {
+        MethodHandle idTest = this.idTest;
+        if (idTest != null) return idTest;
+        return 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");
 }
diff --git a/core/src/main/java/org/jruby/ir/targets/ConstantLookupSite.java b/core/src/main/java/org/jruby/ir/targets/ConstantLookupSite.java
@@ -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();