Hi Claes, Mandy,
On 03/25/2016 03:58 PM, Claes Redestad wrote:
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);
}
Comparing this with proposed code from webrev:
493 try {
494 return (Class<? extends
BoundMethodHandle>)
495 Class.forName("java.lang.invoke.BoundMethodHandle$Species_" +
LambdaForm.shortenSignature(types));
496 } catch (ClassNotFoundException cnf) {
497 // Not pregenerated, generate the class
498 return generateConcreteBMHClass(types);
499 }
...note that the function passed to CLASS_CACHE.computeIfAbsent is
executed just once per distinct 'types' argument. Even if you put the
generated switch between a call to getConcreteBMHClass and
CLASS_CACHE.computeIfAbsent, getConcreteBMHClass(String types) is
executed just once per distinct 'types' argument (except in rare
occasions when VM can not initialize the loaded class).
In this respect a successful Class.forName() is not any worse than
static resolving. It's just that unsuccessful Class.forName() represents
some overhead for classes that are not pre-generated. So an alternative
way to get rid of that overhead is to have a HashSet of 'types' strings
for pre-generated classes at hand in order to decide whether to call
Class.forName or generateConcreteBMHClass.
What's easier to support is another question.
Regards, Peter
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.redes...@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