RFR: 8219713: Reduce work in DefaultMethods::generate_default_methods

[Karen Kinnear]

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?

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

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

------

I am not following the find_empty_vtable_slot filtering.

I am not positive we have sufficient test coverage, even in the DefMeth tests, to ensure that 
the change correctly handles all corner cases, especially those that are not generated by javac.

Can you please check if the defMeth tests cover cases in which we have a local static, or a superclass static with
the same name/sig as a super interface default method, and same for abstract method?

And again - run each of the defMeth tests with defaultmethods logging on at debug level before and
after to ensure you are getting the changes you want?

Your comment is that you “filter out” <clinit>, but the change seems to filter in <clinit> and
filter out the rest of the static methods. Did I read this backwards?

The logic for determining the result of default method computation is complex. 
See klassVtable::is_miranda for part of the explanation. 

The logic in DefaultMethods is dealing with the preliminary miranda list created by
the Pass 1 is_miranda exercise, so when calling generate_default_methods  time, it includes:
non-private (public) non-static abstract interface methods.
non-private (public) non-static concrete (“default”) interface methods.

The goal of the find_empty_vtable_slots exercise is to examine any methods inherited from superinterfaces
to determine if there is a single max-specific method (local, superclass, max-specific superinterface)
or if we need to create an overpass for either no implementation or for no uniquely max-specific.

Here is the “before” logic: 

Step 1: add to the slots, all potential methods inherited from superinterfaces.
First we add all mirandas (non-duplicated). This is the _all_mirandas list back from compute_vtable_size_and_num_mirandas from Pass 1
   - so it includes from this class and its superclasses, in theory all the public non-static interface methods

Step 2: Add to that list:
  1. If there are any overpasses in our superclass, those exception results are local to the superclass, so we need to re-examine the name/sig for default method rules relative to the current class.  (Unchanged)
  2. If there are any static methods in our superclass, we re-examine, i.e. add them to the list. (Changed to add <clinit> only)
For both sets of candidates (overpasses and statics in super)
     a. if the current class has no local implementation -> re-examine (unchanged)
     b. OR the current class has an overpass -> re-examine (unchanged)
     c. OR the current class has a static implementation (Changed to add if current class has non-private static impl only?)

Step 3: Add to that list:
   1. Any default methods in the super class, since they would have filtered out miranda methods, but default methods need to be recalculated relative to the current starting class
For this set of candidates - same logic/same change as above


Summary: this set of filtering changes does not make sense to me.

Note: I am trying to reconstruct why we added the static filters, why we thought we needed
those for reexamination. I do recognize that resolution/selection lookups starting from the current class
WILL find local and superclass statics.

That said if we dig deeper, is_miranda was probably fixed after this code was written - so the
logic in is_miranda will skip the local static, so a question would be - if we were to ensure a full set of test examples, are
we actually adding anything to the list other than overpasses in step 2 and step 3?

Static methods are messy - if we were designing the language from scratch, the static and instance
members would be in different name spaces and we would not find static methods when searching for
instance methods. That is how Dan Smith added the static methods in interfaces, but not how they
work in classes. Javac can restrict what can be generated, but we deal with class files which come
from multiple sources, so we can’t count on that.

The handling of static methods for default methods was changed very late in the game, so it may be that we need to
revisit this logic in terms of requirements.

I would recommend an RFE that requires serious study - if you think there are significant performance
benefits to removing the statics from the list of slots to re-examine.

thanks,
Karen


> On Feb 27, 2019, at 9:23 AM, Claes Redestad <claes.redestad at oracle.com> wrote:
> 
> Hi,
> 
> on modern, larger applications, DefaultMethods::generate_default_methods
> can show up prominently in startup profiles, so I took a stab at dealing
> with some apparent inefficiencies:
> 
> - java.lang.Object is scanned at least once for every interface in the
> hierarchy - this will be a no-op after the first so we should filter
> out Object except the first time we encounter it.
> 
> - the resolveNatives and <clinit> are added and checked for every
> class/interface scanned, but then explicitly filtered out later on. It
> seems that all static initializers and private static methods can be
> safely filtered out from the first pass since they wouldn't mask default
> methods anyhow.
> 
> - a few minor cleanups and simplifications, e.g., removing never
> exercised code to cancel and reset an iteration.
> 
> Bug:    https://bugs.openjdk.java.net/browse/JDK-8219713
> Webrev: http://cr.openjdk.java.net/~redestad/8219713/open.01/
> 
> Testing: tier1-3, measured a speed-up on certain startup application
> 
> Thanks!
> 
> /Claes