RFR: 8152641: Plugin to generate BMH$Species classes ahead-of-time

[Claes Redestad]

Hi,

On 2016-03-25 11:11, Peter Levart wrote:
> Hi Claes, Mandy,
>
> On 03/25/2016 02:51 AM, Mandy Chung wrote:
>> Hi Claes,
>>
>> This is a good interesting work to generate 
>> BoundMethodHandle$Species_* classes at link time.  This will save the 
>> class generation time.  Instead of Class.forName, you may want to 
>> have a class (similar to SystemModules [1]) that will be updated at 
>> link time so that BoundMethodHandle can reference it statically to 
>> determine what species are generated at link time and even return 
>> statically Class<?> of the generated class.
>
> You may still want to load this classes lazily when needed. Otherwise 
> the BMH.CLASS_CACHE could simply be pre-populated with name -> 
> Class<?> entries at appropriate early time.

I think laziness is a good property here, since it won't penalize 
modular applications that choose to instruct jlink to pregenerate a lot 
of BMH classes.

Perhaps we can allow for static resolve + lazy classload by emitting a 
switch equivalent to:

switch (types) {
   case "LL":
     return Species_LL.class;
   case "L3":
     return Species_L3.class;
   ..
   default:
     return generateBMHSpeciesClass(types);
}

>
>>
>> About CNFE, the new Class.forName(Module, String) method does not 
>> throw CNFE that you could use instead (if not static reference as 
>> suggested above).  If not found, it will return null.

Thanks, this one had passed under my radar. :-)

>>
>> This plugin accesses the non-public methods in BoundMethodHandle  and 
>> has to invoke it via reflection.  With module boundary enforced now, 
>> maybe we could consider moving to some internal package to share with 
>> this jlink plugin.
>>
>> I think there are tests needed to be updated when a new plugin is added.
>>
>> Mandy
>> [1] 
>> http://hg.openjdk.java.net/jdk9/dev/jdk/file/3abd25870915/src/java.base/share/classes/jdk/internal/module/SystemModules.java
>
> One thing to consider is the following: the names of the classes obey 
> a particular convention which is now hardcoded in two places: the 
> BoundMethodHandle and the GenerateBMHClassesPlugin. You could have a 
> method with the following signature in BoundMethodHandle$Factory:
>
> Map.Entry<String, byte[]> generateConcreteBMHClassBytes(String types)
>
> ...which would also return the name of the class derived from the 
> types which you would then use to derive the path of the .class file 
> in the module of the image. Also, this method - although 
> package-private becomes part of the API and that should be written in 
> a comment.
>
> What do you think?

Definitely should add a comment about generateConcreteBMHClassBytes 
being an internal API used by the plugin.

I'm open to the idea of moving static utility code to some internal 
package, but want to hear what maintainers of java.lang.invoke think 
about factoring out utility code like this.

Thanks!

/Claes

>
> Regards, Peter
>
>>
>>> On Mar 24, 2016, at 6:42 AM, Claes Redestad 
>>> <claes.redestad at oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> please review this patch which add an enabled-by-default plugin to 
>>> generate a
>>> configurable list of BoundMethodHandle$Species_*-classes using jlink.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152641
>>> Webrev: http://cr.openjdk.java.net/~redestad/8152641/webrev.01/
>>>
>>> This plugin adds the --generate-bmh flag to jlink, which takes a
>>> comma-separated list of shortened species type strings.
>>>
>>> As an example: --generate-bmh LL,L3,L4 will generate
>>> BoundMethodHandle$Species_LL, ...$Species_L3 and ...$Species_L4 and add
>>> them to the java.base module.
>>>
>>> The Species class lookup in BoundMethodHandle will first check if 
>>> there is a
>>> generated class, otherwise generate it as previously. Adding an 
>>> exceptional
>>> path might seem problematic, but as the code generation is 
>>> magnitudes more
>>> expensive than the exception it doesn't seem fruitful to go to 
>>> lengths to avoid
>>> the CNFE.
>>>
>>> More notes along with some results can be found here:
>>>
>>> http://cr.openjdk.java.net/~redestad/scratch/bmh_species_gen.txt
>>>
>>> Thanks!
>>>
>>> /Claes
>