RFR: 8062389,8029459,8061950: Class.getMethod() is inconsistent with Class.getMethods() results + more

[Peter Levart]

Just a note that this is still ready to be reviewed.

I was also told that JCK tests pass with the patch applied.

Regards, Peter

On 10/04/2016 03:40 PM, Peter Levart wrote:
> Hi,
>
> I have a fix for conformance (P2) bug (8062389 
> <https://bugs.openjdk.java.net/browse/JDK-8062389>) that also fixes a 
> conformance (P3) bug (8029459 
> <https://bugs.openjdk.java.net/browse/JDK-8029459>) and a performance 
> issue (8061950 <https://bugs.openjdk.java.net/browse/JDK-8061950>):
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.04/
>
> As Daniel Smith writes in 8029459 
> <https://bugs.openjdk.java.net/browse/JDK-8029459>, the following 
> situation is as expected:
>
> interface I { void m(); }
> interface J extends I { void m(); }
> interface K extends J {}
> K.class.getMethods() = [ J.m ]
>
> But the following has a different result although it should probably 
> have the same:
>
> interface I { void m(); }
> interface J extends I { void m(); }
> interface K extends I, J {}
> K.class.getMethods() = [ I.m, J.m ]
>
> He then suggests the following algorithm:
>
> An implementation of getMethods consistent with JLS 8 would include 
> the following (see Lambda Spec, Part H, 9.4.1 and 8.4.8):
> 1) The class's/interface's declared (public) methods
> 2) The getMethods() of the superclass (if this is a class), minus any 
> that have a match in (1)
> 3) The getMethods() of each superinterface, minus any that have a 
> match in (1) or a _concrete_ match in (2) or a match from a 
> more-specific class/interface in (2) or (3)
>
> But even that is not sufficient for the following situation:
>
> interface E { void m(); }
> interface F extends E { void m(); }
> abstract class G implements E {}
> abstract class H extends G implements F {}
> H.class.getMethods() = [ E.m, F.m ]
>
> The desired result of H.class.getMethods() = [ F.m ]
>
> So a more precise definition is required which is implemented in the fix.
>
> The getMethods() implementation partitions the union of the following 
> methods:
>
> 1) The class's/interface's declared public methods
> 2) The getMethods() of the superclass (if this is a class)
> 3) The non-static getMethods() of each direct superinterface
>
> ...into equivalence classes (sets) of methods with same signature 
> (return type, name, parameter types). Within each such set, only the 
> "most specific" methods are kept and others are discarded. The union 
> of the kept methods is the result.
>
> The "more specific" is defined as a partial order within a set of 
> methods of same signature:
>
> Method A is more specific than method B if:
> - A is declared by a class and B is declared by an interface; or
> - A is declared by the same type as or a subtype of B's declaring type 
> and both are either declared by classes or both by interfaces (clearly 
> if A and B are declared by the same type and have the same signature, 
> they are the same method)
>
> If A and B are distinct elements from the set of methods with same 
> signature, then either:
> - A is more specific than B; or
> - B is more specific than A; or
> - A and B are incomparable
>
> A sub-set of "most specific" methods are the methods from the set 
> where for each such method M, there is no method N != M such that N is 
> "more specific" than M.
>
> There can be more than one "most specific" method for a particular 
> signature as they can be inherited from multiple unrelated interfaces, 
> but:
> - javac prevents compilation when among multiply inherited methods 
> with same signature there is at least one default method, so in 
> practice, getMethods() will only return multiple methods with same 
> signature if they are abstract interface methods. With one exception: 
> bridge methods that are created by javac for co-variant overrides are 
> default methods in interfaces. For example:
>
>     interface I { Object m(); }
>     interface J1 extends I { Map m(); }
>     interface J2 extends I { HashMap m(); }
>     interface K extends J1, J2 {}
>
> K.class.getMethods() = [ default Object j1.m, abstract Map j1.m, 
> default Object j2.m, abstract HashMap j2.m ]
>
> But this is an extreme case where one can still expect multiple 
> non-abstract methods with same signature, but different declaring 
> type, returned from getMethods().
>
> In order to also fix 8062389 
> <https://bugs.openjdk.java.net/browse/JDK-8062389>, getMethod() and 
> getMethods() share the same consolidation logic that results in a set 
> of "most specific" methods. The partitioning in getMethods() is 
> partially performed by collecting Methods into a HashMap where the 
> keys are (name, parameter types) tuples and values are linked lists of 
> Method objects with possibly varying return and declaring types. The 
> consolidation, i.e. keeping only the set of most specific methods as 
> new methods are encountered, is performed only among methods in the 
> list that share same return type and also removes duplicates. 
> getMethod() uses only one such list, consolidates methods that match 
> given name and parameter types and returns the 1st method from the 
> resulting list that has the most specific return type.
>
> That's it for algorithms used. As partitioning partially happens using 
> HashMap with (name, parameter types) keys, lists of methods that form 
> values are typically kept short with most of them containing only a 
> single method, so this fix also fixes performance issue 8061950 
> <https://bugs.openjdk.java.net/browse/JDK-8061950>.
>
> The patch also contains some optimizations around redundant copying of 
> arrays and reflective objects.
>
> The FilterNotMostSpecific jtreg test has been updated to accommodate 
> for changed behavior. Both of the above extreme cases have been added 
> to the test.
>
> So, comments, please.
>
> Regards, Peter
>