Hi Peter, Nice work here.
PublicMethods — I would be inclined to change PublicMethods to encapsulate an instance of LinkedHashMap, since it’s only the consolidate/toArray methods that really matter, and no need for the key/value types to be exposed, then no need to declare serialVersionUID, and also it can be final. The JavaDoc on consolidate would need to be tweaked. 29 * existing methods with same signature if they are less specific then added s/then/than 89 if (o == null || getClass() != o.getClass()) return false; Could be simplified to if (!(o instanceof Key)) return false Paul. > On 4 Oct 2016, at 06:40, Peter Levart <peter.lev...@gmail.com> 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 >
