Skip to content

Commit

Permalink
Centralize JI method definition logic and eliminate alias objects.
Browse files Browse the repository at this point in the history
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.
headius committed Apr 1, 2016
1 parent 851974d commit c483e89
Showing 4 changed files with 21 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
@@ -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; }

Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}

1 comment on commit c483e89

@kares
Copy link
Member

@kares kares commented on c483e89 Apr 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very najs 🐈

Sorry, something went wrong.

Please sign in to comment.