RFR: 8219713: Reduce work in DefaultMethods::generate_default_methods

[Claes Redestad]

On 2019-03-01 17:24, Karen Kinnear wrote:
> Claes,
> 
> Thank you for looking at this speedup and cleanup.
> 
> For the j.l.Object - the logic you have changed “looks” right to me - could you possibly
> run an example with multiple interfaces with the defaultmethods logging at debug level to show us the before and after?

For something like this:

public class A {
     private interface I1 {
     }
     private interface I2 extends I1 {
         default int foo() { return 1; }
     }
     private static class B implements I1, I2 {}
     public static void main(String ... args) {
         new B();
     }
}

-Xlog:defaultmethods=debug outputs this now:
[0.093s][debug][defaultmethods] Interface A$I2 requires default method 
processing
[0.093s][debug][defaultmethods] A$I2
[0.093s][debug][defaultmethods]   java/lang/Object
[0.093s][debug][defaultmethods]   A$I1
[0.093s][debug][defaultmethods]     java/lang/Object
[0.093s][debug][defaultmethods] Slots that need filling:
[0.093s][debug][defaultmethods]   <clinit>()V
[0.093s][debug][defaultmethods]   registerNatives()V
[0.093s][debug][defaultmethods]   Looking for default methods for slot 
<clinit>()V
[0.093s][debug][defaultmethods]   Looking for default methods for slot 
registerNatives()V
[0.093s][debug][defaultmethods] Creating defaults and overpasses...
[0.093s][debug][defaultmethods] Created 0 overpass methods
[0.093s][debug][defaultmethods] Created 0 default  methods
[0.093s][debug][defaultmethods] Default method processing complete
[0.093s][debug][defaultmethods] Class A$B requires default method processing
[0.093s][debug][defaultmethods] A$B
[0.093s][debug][defaultmethods]   java/lang/Object
[0.093s][debug][defaultmethods]   A$I1
[0.093s][debug][defaultmethods]     java/lang/Object
[0.093s][debug][defaultmethods]   A$I2
[0.093s][debug][defaultmethods]     java/lang/Object
[0.093s][debug][defaultmethods]     A$I1
[0.093s][debug][defaultmethods]       java/lang/Object
[0.093s][debug][defaultmethods] Slots that need filling:
[0.093s][debug][defaultmethods]   foo()I
[0.093s][debug][defaultmethods]   <clinit>()V
[0.093s][debug][defaultmethods]   registerNatives()V
[0.093s][debug][defaultmethods]   Looking for default methods for slot 
foo()I
[0.093s][debug][defaultmethods]   Looking for default methods for slot 
<clinit>()V
[0.093s][debug][defaultmethods]   Looking for default methods for slot 
registerNatives()V
[0.093s][debug][defaultmethods] Creating defaults and overpasses...
[0.093s][debug][defaultmethods] for slot: foo()I
[0.093s][debug][defaultmethods]   Selected method: A$I2.foo()I
[0.093s][debug][defaultmethods] Created 0 overpass methods
[0.093s][debug][defaultmethods] Created 1 default  methods
[0.093s][debug][defaultmethods] Default method processing complete

and with my patch:
[0.087s][debug][defaultmethods] Interface A$I2 requires default method 
processing
[0.087s][debug][defaultmethods] A$I2
[0.087s][debug][defaultmethods]   A$I1
[0.087s][debug][defaultmethods] Slots that need filling:
[0.087s][debug][defaultmethods] Default method processing complete
[0.087s][debug][defaultmethods] Class A$B requires default method processing
[0.087s][debug][defaultmethods] A$B
[0.087s][debug][defaultmethods]   java/lang/Object
[0.087s][debug][defaultmethods]   A$I1
[0.087s][debug][defaultmethods]   A$I2
[0.087s][debug][defaultmethods]     A$I1
[0.087s][debug][defaultmethods] Slots that need filling:
[0.087s][debug][defaultmethods]   foo()I
[0.087s][debug][defaultmethods]   Looking for default methods for slot 
foo()I
[0.087s][debug][defaultmethods] Creating defaults and overpasses...
[0.087s][debug][defaultmethods] for slot: foo()I
[0.087s][debug][defaultmethods]   Selected method: A$I2.foo()I
[0.087s][debug][defaultmethods] Created 0 overpass methods
[0.087s][debug][defaultmethods] Created 1 default  methods
[0.087s][debug][defaultmethods] Default method processing complete

I've manually tested numerous examples and verified the actual number of
overpass and default methods generated doesn't change in a number of
more complex applications.

> 
> I am good with the reordering of the lookup vs. !already_in_vtable_slots change.

Thanks!

> 
> I suspect the above changes (and the cleanup) are worth doing, and leave out the find_empty_vtable_slot
> filtering changes.

I understand your reluctance with these changes. An alternative that
comes pretty close is to specifically filter out all non-public methods
in Object (something we explicitly do in FindMethodsByErasedSig::visit
already):

@@ -627,14 +612,14 @@
    while (super != NULL) {
      for (int i = 0; i < super->methods()->length(); ++i) {
        Method* m = super->methods()->at(i);
-      if (m->is_overpass() || m->is_static()) {
+      if (m->is_overpass() || (m->is_static() && 
!SystemDictionary::is_nonpublic_Object_method(m))) {
...

This keeps the property of removing examination of Object::<clinit> and
Object::registerNatives on every type, but nothing more. Output for the
A example above is unchanged, but private static methods and class
initializers on all other types and interfaces would still trigger 
examination and behave the same as before.

http://cr.openjdk.java.net/~redestad/8219713/open.02/

What do you think?

For reference, the change that added m->is_static() to the examination
was: https://bugs.openjdk.java.net/browse/JDK-8028438 Lambda:
default methods static superclass method masking default -

Thanks!

/Claes