8025965: Specialized functions with same weight replace each other in TreeSet
authorhannesw
Tue, 08 Oct 2013 13:11:15 +0200
changeset 20570 b1c8d1e8013a
parent 20569 2386d10eadd3
child 20571 f52b0abf5d88
8025965: Specialized functions with same weight replace each other in TreeSet Reviewed-by: jlaskey, sundar
nashorn/src/jdk/nashorn/internal/runtime/CompiledFunction.java
--- a/nashorn/src/jdk/nashorn/internal/runtime/CompiledFunction.java	Tue Oct 08 13:02:39 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/CompiledFunction.java	Tue Oct 08 13:11:15 2013 +0200
@@ -48,6 +48,7 @@
     }
 
     CompiledFunction(final MethodType type, final MethodHandle invoker, final MethodHandle constructor) {
+        assert type != null;
         this.type        = type;
         this.invoker     = invoker;
         this.constructor = constructor;
@@ -80,7 +81,37 @@
 
     @Override
     public int compareTo(final CompiledFunction o) {
-        return weight() - o.weight();
+        return compareMethodTypes(type(), o.type());
+    }
+
+    private static int compareMethodTypes(final MethodType ownType, final MethodType otherType) {
+        // Comparable interface demands that compareTo() should only return 0 if objects are equal.
+        // Failing to meet this requirement causes same weight functions to replace each other in TreeSet,
+        // so we go some lengths to come up with an ordering between same weight functions,
+        // first falling back to parameter count and then to hash code.
+        if (ownType.equals(otherType)) {
+            return 0;
+        }
+
+        final int diff = weight(ownType) - weight(otherType);
+        if (diff != 0) {
+            return diff;
+        }
+        if (ownType.parameterCount() != otherType.parameterCount()) {
+            return ownType.parameterCount() - otherType.parameterCount();
+        }
+        // We're just interested in not returning 0 here, not correct ordering
+        return ownType.hashCode() - otherType.hashCode();
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        return obj instanceof CompiledFunction && type().equals(((CompiledFunction)obj).type());
+    }
+
+    @Override
+    public int hashCode() {
+        return type().hashCode();
     }
 
     private int weight() {
@@ -119,14 +150,14 @@
      * a semantically equivalent linkage can be performed.
      *
      * @param mt type to check against
-     * @return
+     * @return true if types are compatible
      */
     boolean typeCompatible(final MethodType mt) {
-        final Class<?>[] wantedParams   = mt.parameterArray();
-        final Class<?>[] existingParams = type().parameterArray();
+        final int wantedParamCount   = mt.parameterCount();
+        final int existingParamCount = type.parameterCount();
 
         //if we are not examining a varargs type, the number of parameters must be the same
-        if (wantedParams.length != existingParams.length && !isVarArgsType(mt)) {
+        if (wantedParamCount != existingParamCount && !isVarArgsType(mt)) {
             return false;
         }
 
@@ -134,10 +165,10 @@
         //parameters lengths do not match is if our type ends with a varargs argument.
         //then every trailing parameter in the given callsite can be folded into it, making
         //us compatible (albeit slower than a direct specialization)
-        final int lastParamIndex = Math.min(wantedParams.length, existingParams.length);
+        final int lastParamIndex = Math.min(wantedParamCount, existingParamCount);
         for (int i = 0; i < lastParamIndex; i++) {
-            final Type w = Type.typeFor(wantedParams[i]);
-            final Type e = Type.typeFor(existingParams[i]);
+            final Type w = Type.typeFor(mt.parameterType(i));
+            final Type e = Type.typeFor(type.parameterType(i));
 
             //don't specialize on booleans, we have the "true" vs int 1 ambiguity in resolution
             //we also currently don't support boolean as a javascript function callsite type.