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

Commits on Jul 26, 2016

  1. Copy the full SHA
    cc7cd9a View commit details
  2. Use new call site caching for builtin check. Fixes #3976.

    This allows us to use short-circuited logic without calling <=>
    without hardcoding a particular <=> implementation.
    headius committed Jul 26, 2016
    Copy the full SHA
    d2e6c5b View commit details
Showing with 41 additions and 14 deletions.
  1. +16 −9 core/src/main/java/org/jruby/RubyComparable.java
  2. +16 −4 core/src/main/java/org/jruby/RubyString.java
  3. +9 −1 core/src/main/java/org/jruby/runtime/JavaSites.java
25 changes: 16 additions & 9 deletions core/src/main/java/org/jruby/RubyComparable.java
Original file line number Diff line number Diff line change
@@ -36,6 +36,8 @@
import org.jruby.anno.JRubyMethod;
import org.jruby.anno.JRubyModule;
import org.jruby.runtime.CallSite;
import org.jruby.runtime.JavaSites;
import org.jruby.runtime.JavaSites.ComparableSites;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.callsite.RespondToCallSite;
@@ -84,8 +86,9 @@ public static int cmpint(ThreadContext context, IRubyObject val, IRubyObject a,

RubyFixnum zero = RubyFixnum.zero(context.runtime);

if (val.callMethod(context, ">", zero).isTrue()) return 1;
if (val.callMethod(context, "<", zero).isTrue()) return -1;
ComparableSites sites = sites(context);
if (sites.op_gt.call(context, val, val, zero).isTrue()) return 1;
if (sites.op_lt.call(context, val, val, zero).isTrue()) return -1;

return 0;
}
@@ -114,8 +117,8 @@ public static IRubyObject invcmp(final ThreadContext context, final IRubyObject
private static final Ruby.RecursiveFunctionEx DEFAULT_INVCMP = new Ruby.RecursiveFunctionEx<IRubyObject>() {
@Override
public IRubyObject call(ThreadContext context, IRubyObject recv, IRubyObject other, boolean recur) {
if (recur || !other.respondsTo("<=>")) return context.runtime.getNil();
return invokedynamic(context, other, OP_CMP, recv);
if (recur || !sites(context).respond_to_op_cmp.respondsTo(context, other, other)) return context.runtime.getNil();
return sites(context).op_cmp.call(context, other, other, recv);
}
};

@@ -158,7 +161,7 @@ private static IRubyObject callCmpMethod(final ThreadContext context, final IRub
@Override
public IRubyObject call(IRubyObject obj, boolean recur) {
if (recur) return runtime.getNil();
return invokedynamic(context, recv, OP_CMP, other);
return sites(context).op_cmp.call(context, recv, recv, other);
}
}, recv);

@@ -174,7 +177,7 @@ public IRubyObject call(IRubyObject obj, boolean recur) {
// <=> may return nil in many circumstances, e.g. 3 <=> NaN
@JRubyMethod(name = ">", required = 1)
public static RubyBoolean op_gt(ThreadContext context, IRubyObject recv, IRubyObject other) {
IRubyObject result = invokedynamic(context, recv, OP_CMP, other);
IRubyObject result = sites(context).op_cmp.call(context, recv, recv, other);

if (result.isNil()) cmperr(recv, other);

@@ -186,7 +189,7 @@ public static RubyBoolean op_gt(ThreadContext context, IRubyObject recv, IRubyOb
*/
@JRubyMethod(name = ">=", required = 1)
public static RubyBoolean op_ge(ThreadContext context, IRubyObject recv, IRubyObject other) {
IRubyObject result = invokedynamic(context, recv, OP_CMP, other);
IRubyObject result = sites(context).op_cmp.call(context, recv, recv, other);

if (result.isNil()) cmperr(recv, other);

@@ -198,7 +201,7 @@ public static RubyBoolean op_ge(ThreadContext context, IRubyObject recv, IRubyOb
*/
@JRubyMethod(name = "<", required = 1)
public static RubyBoolean op_lt(ThreadContext context, IRubyObject recv, IRubyObject other) {
IRubyObject result = invokedynamic(context, recv, OP_CMP, other);
IRubyObject result = sites(context).op_cmp.call(context, recv, recv, other);

if (result.isNil()) cmperr(recv, other);

@@ -218,7 +221,7 @@ public static RubyBoolean op_lt(ThreadContext context, CallSite cmp, IRubyObject
*/
@JRubyMethod(name = "<=", required = 1)
public static RubyBoolean op_le(ThreadContext context, IRubyObject recv, IRubyObject other) {
IRubyObject result = invokedynamic(context, recv, OP_CMP, other);
IRubyObject result = sites(context).op_cmp.call(context, recv, recv, other);

if (result.isNil()) cmperr(recv, other);

@@ -232,4 +235,8 @@ public static RubyBoolean op_le(ThreadContext context, IRubyObject recv, IRubyOb
public static RubyBoolean between_p(ThreadContext context, IRubyObject recv, IRubyObject first, IRubyObject second) {
return context.runtime.newBoolean(op_lt(context, recv, first).isFalse() && op_gt(context, recv, second).isFalse());
}

private static ComparableSites sites(ThreadContext context) {
return context.sites.Comparable;
}
}
20 changes: 16 additions & 4 deletions core/src/main/java/org/jruby/RubyString.java
Original file line number Diff line number Diff line change
@@ -1692,7 +1692,9 @@ public IRubyObject op_ge(ThreadContext context, IRubyObject other) {

@JRubyMethod(name = ">=")
public IRubyObject op_ge19(ThreadContext context, IRubyObject other) {
if (other instanceof RubyString) return context.runtime.newBoolean(op_cmp((RubyString) other) >= 0);
if (other instanceof RubyString && cmpIsBuiltin(context)) {
return context.runtime.newBoolean(op_cmp((RubyString) other) >= 0);
}
return RubyComparable.op_ge(context, this, other);
}

@@ -1702,7 +1704,9 @@ public IRubyObject op_gt(ThreadContext context, IRubyObject other) {

@JRubyMethod(name = ">")
public IRubyObject op_gt19(ThreadContext context, IRubyObject other) {
if (other instanceof RubyString) return context.runtime.newBoolean(op_cmp((RubyString) other) > 0);
if (other instanceof RubyString && cmpIsBuiltin(context)) {
return context.runtime.newBoolean(op_cmp((RubyString) other) > 0);
}
return RubyComparable.op_gt(context, this, other);
}

@@ -1712,7 +1716,9 @@ public IRubyObject op_le(ThreadContext context, IRubyObject other) {

@JRubyMethod(name = "<=")
public IRubyObject op_le19(ThreadContext context, IRubyObject other) {
if (other instanceof RubyString) return context.runtime.newBoolean(op_cmp((RubyString) other) <= 0);
if (other instanceof RubyString && cmpIsBuiltin(context)) {
return context.runtime.newBoolean(op_cmp((RubyString) other) <= 0);
}
return RubyComparable.op_le(context, this, other);
}

@@ -1722,10 +1728,16 @@ public IRubyObject op_lt(ThreadContext context, IRubyObject other) {

@JRubyMethod(name = "<")
public IRubyObject op_lt19(ThreadContext context, IRubyObject other) {
if (other instanceof RubyString) return context.runtime.newBoolean(op_cmp((RubyString) other) < 0);
if (other instanceof RubyString && cmpIsBuiltin(context)) {
return context.runtime.newBoolean(op_cmp((RubyString) other) < 0);
}
return RubyComparable.op_lt(context, sites(context).cmp, this, other);
}

private boolean cmpIsBuiltin(ThreadContext context) {
return sites(context).cmp.retrieveCache(metaClass).method.isBuiltin();
}

public IRubyObject str_eql_p(ThreadContext context, IRubyObject other) {
return str_eql_p19(context, other);
}
10 changes: 9 additions & 1 deletion core/src/main/java/org/jruby/runtime/JavaSites.java
Original file line number Diff line number Diff line change
@@ -26,6 +26,7 @@ public class JavaSites {
public final BignumSites Bignum = new BignumSites();
public final TimeSites Time = new TimeSites();
public final EnumerableSites Enumerable = new EnumerableSites();
public final ComparableSites Comparable = new ComparableSites();
public final IOSites IO = new IOSites();
public final TypeConverterSites TypeConverter = new TypeConverterSites();
public final HelpersSites Helpers = new HelpersSites();
@@ -89,7 +90,7 @@ public static class StringSites {
public final RespondToCallSite respond_to_cmp = new RespondToCallSite("<=>");
public final RespondToCallSite respond_to_to_str = new RespondToCallSite("to_str");
public final CallSite equals = new FunctionalCachingCallSite("==");
public final CallSite cmp = new FunctionalCachingCallSite("<=>");
public final CachingCallSite cmp = new FunctionalCachingCallSite("<=>");
public final CallSite hash = new FunctionalCachingCallSite("hash");

public final Ruby.RecursiveFunctionEx recursive_cmp = new Ruby.RecursiveFunctionEx<IRubyObject>() {
@@ -222,6 +223,13 @@ public static class EnumerableSites {
public final CheckedSites size_checked = new CheckedSites("size");
}

public static class ComparableSites {
public final RespondToCallSite respond_to_op_cmp = new RespondToCallSite("<=>");
public final CallSite op_cmp = new FunctionalCachingCallSite("<=>");
public final CallSite op_lt = new FunctionalCachingCallSite("<");
public final CallSite op_gt = new FunctionalCachingCallSite(">");
}

public static class IOSites {
public final CheckedSites closed_checked = new CheckedSites("closed?");
public final CheckedSites close_checked = new CheckedSites("close");