Centralize JI method definition logic and eliminate alias objects.

Aliased methods throughout most of JRuby (and MRI's core) are just defined multiple times with the same method object, rather than with an alias wrapper. While working on perforamnce I noticed hundreds of thousands of AliasMethod being created for JI. This change eliminates those, centralizes "installer" method definition logic, and reduces overhead and allocation during installation.
jruby · Apr 1, 2016 · c483e89 · c483e89 · kares · Apr 4, 2016
1 parent 851974d
commit c483e89
Showing 4 changed files with 21 additions and 19 deletions.
diff --git a/core/src/main/java/org/jruby/javasupport/binding/InstanceMethodInvokerInstaller.java b/core/src/main/java/org/jruby/javasupport/binding/InstanceMethodInvokerInstaller.java
@@ -12,11 +12,7 @@ public class InstanceMethodInvokerInstaller extends MethodInstaller {
 
     @Override void install(final RubyModule proxy) {
         if ( hasLocalMethod() ) {
-            proxy.addMethod(name, new InstanceMethodInvoker(proxy, methods));
-            if ( aliases != null && isPublic() ) {
-                proxy.defineAliases(aliases, this.name);
-                //aliases = null;
-            }
+            defineMethods(proxy, new InstanceMethodInvoker(proxy, methods), true);
         }
     }
 }
diff --git a/core/src/main/java/org/jruby/javasupport/binding/MethodInstaller.java b/core/src/main/java/org/jruby/javasupport/binding/MethodInstaller.java
@@ -1,5 +1,9 @@
 package org.jruby.javasupport.binding;
 
+import org.jruby.RubyModule;
+import org.jruby.internal.runtime.methods.DynamicMethod;
+import org.jruby.java.invokers.MethodInvoker;
+
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
@@ -32,6 +36,20 @@ void addAlias(final String alias) {
         if ( ! aliases.contains(alias) ) aliases.add(alias);
     }
 
+    protected void defineMethods(RubyModule target, DynamicMethod invoker, boolean checkDups) {
+        String oldName = this.name;
+        target.addMethod(oldName, invoker);
+
+        List<String> aliases = this.aliases;
+        if ( aliases != null && isPublic() ) {
+            for (int i = 0; i < aliases.size(); i++) {
+                String name = aliases.get(i);
+                if (checkDups && oldName.equals(name)) continue;
+                target.addMethod(name, invoker);
+            }
+        }
+    }
+
     @Override
     boolean hasLocalMethod () { return localMethod; }
 

diff --git a/core/src/main/java/org/jruby/javasupport/binding/SingletonMethodInvokerInstaller.java b/core/src/main/java/org/jruby/javasupport/binding/SingletonMethodInvokerInstaller.java
@@ -2,7 +2,6 @@
 
 import org.jruby.RubyClass;
 import org.jruby.RubyModule;
-import org.jruby.internal.runtime.methods.DynamicMethod;
 import org.jruby.java.invokers.SingletonMethodInvoker;
 
 /**
@@ -21,11 +20,6 @@ public SingletonMethodInvokerInstaller(String name, Object singleton) {
         // we don't check haveLocalMethod() here because it's not local and we know
         // that we always want to go ahead and install it
         final RubyClass singletonClass = proxy.getSingletonClass();
-        DynamicMethod method = new SingletonMethodInvoker(this.singleton, singletonClass, methods);
-        singletonClass.addMethod(name, method);
-        if ( aliases != null && isPublic() ) {
-            singletonClass.defineAliases(aliases, name);
-            //aliases = null;
-        }
+        defineMethods(singletonClass, new SingletonMethodInvoker(this.singleton, singletonClass, methods), false);
     }
 }
diff --git a/core/src/main/java/org/jruby/javasupport/binding/StaticMethodInvokerInstaller.java b/core/src/main/java/org/jruby/javasupport/binding/StaticMethodInvokerInstaller.java
@@ -2,7 +2,6 @@
 
 import org.jruby.RubyClass;
 import org.jruby.RubyModule;
-import org.jruby.internal.runtime.methods.DynamicMethod;
 import org.jruby.java.invokers.StaticMethodInvoker;
 
 /**
@@ -15,12 +14,7 @@ public class StaticMethodInvokerInstaller extends MethodInstaller {
     @Override void install(final RubyModule proxy) {
         if ( hasLocalMethod() ) {
             final RubyClass singletonClass = proxy.getSingletonClass();
-            DynamicMethod method = new StaticMethodInvoker(singletonClass, methods);
-            singletonClass.addMethod(name, method);
-            if ( aliases != null && isPublic() ) {
-                singletonClass.defineAliases(aliases, name);
-                //aliases = null;
-            }
+            defineMethods(singletonClass, new StaticMethodInvoker(singletonClass, methods), false);
         }
     }
 }