RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class

[Joel Borggrén-Franck]

Hi,

Since Java 8 the methods Class.getMethod() and Class.getMethods() can in some circumstances return methods that are not members of the class. This is due to a spec change in 8 not due to a code change. For this to happen you need to have an interface hierarchy that looks something like:

interface I { void m(); } 
interface J extends I { void m(); } 

and either one of:

interface K extends ***I,*** J {} 
class C implements ***I,*** J {}

In Java 7 K.class.getMethods() would return [ I.m, J.m ]. The 7 spec is a bit vague but I have been convinced by experts that this is actually correct in 7. However this was changed in the spec for 8, now only J.m() is a member of K or C so the result should only contain [ J.m() ].

See the bug: https://bugs.openjdk.java.net/browse/JDK-8029674 for a more thorough discussion of this.

The fix proposed for 8u (and 9 at first) is a partial fix that ensures that:

- a concrete class does not have any abstract methods
- nothing is changed unless someone introduces default methods into the interface hierarchy

This is to minimise the behavioural compatibility concerns in a minor release. For 9 we might implement the full solution.  (See the 9 bug: https://bugs.openjdk.java.net/browse/JDK-8029459).

Basically in the fix proposed for 8u, default methods prune less specific methods from the result if they have the same discriptor. There are some complications due to preserving the old lookup order and the fact that we don’t want different results depending on the order of pairwise comparisons. Since this work and MethodArray in general is comparing descriptors bridges should not interact with this patch.

Bug: https://bugs.openjdk.java.net/browse/JDK-8029674
Patch: http://cr.openjdk.java.net/~jfranck/8029674/webrev.02/

*** Testing ***

I made a test with a bit over 100 cases where 40 of the tests test behaviour that shouldn’t change (IE they pass both with and without the patch). The test is a bit vast, but I have commented some of the more interesting results.

*** Performance ***

The time complexity of this is O(defaultMethods * methods). getMethods() is already somewhat quadratic in complexity due to removing interface methods that have a concrete implementation. Also the result is cached so this is a one time increase per Class.

I have done some experiments on a patched jdk without the cache and the performance looks decent:

- Loading every class in rt.jar and calling getMethods() on every Class takes 8s on my laptop both before and after the patch. Perhaps not that surprising since there aren’t that many default methods in the jdk and this is probably dominated by IO.

- Calling getMethods on a Class with 16K methods, 10% which are default methods, seem to take roughly 50% longer on my laptop (2,4 instead of 1,7 seconds). I’m working on converting this benchmark to JMH, but at least I don’t think I have made the most basic benchmark errors.

If this turns out to be a performance problem I have an idea for an index that will make this O(methods sharing name * methods) for an addition of a few allocations, but I don’t expect this to be necessary.

Thanks in advance

cheers
/Joel