Hi Paul, Alan,
I incorporated Paul's suggestions into new webrev:
http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.05/
This iteration also contains a nearly-exhaustive test of
Class.getMethods(): PublicMethodsTest. It is actually a test generator.
Given a Case template, it generates all variants of methods for each of
the types in the case. Case1 contains 4 interface method variants ^ 3
interfaces * 4 class method variants ^ 3 classes = 4^6 = 4096 different
sub-cases of which only 1379 compile. The results of those 1379
sub-cases are persisted in the Case1.results file. Running the test
compares the persisted results with actual result of executing each
sub-case. When running this test on plain JDK 9 (without patch), the
test finds 218 sub-cases where results differ:
http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/PublicMethodsTest.jtr
Looking at those differences gives the impression of the effects of the
patch.
Regards, Peter
On 10/11/2016 11:35 PM, Paul Sandoz wrote:
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
<mailto: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/
<http://cr.openjdk.java.net/%7Eplevart/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
