# HG changeset patch # User hannesw # Date 1381230675 -7200 # Node ID b1c8d1e8013a14b9c776563bc82b3455121bfd37 # Parent 2386d10eadd3c365ea0d85a394f68e8ead57463e 8025965: Specialized functions with same weight replace each other in TreeSet Reviewed-by: jlaskey, sundar diff -r 2386d10eadd3 -r b1c8d1e8013a 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.