Hi Peter, Mandy,
On 2016-03-26 12:47, Peter Levart wrote:
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
to have something to compare with I built a version which, like you
suggest,
generates a HashSet<String> with the set of generated classes here:
http://cr.openjdk.java.net/~redestad/8152641/webrev.02/
This adds a fair bit of complexity to the plugin and requires we add a
nested
class in BoundMethodHandle that we can replace. Using the anonymous
compute Function for this seems like the best choice for this.
I've not been able to measure any statistical difference in real startup
terms,
though, and not figured out a smart way to benchmark the overhead of the
CNFE in relation to the class generation (my guess it adds a fraction to
the
total cost) and since this adds ever so little footprint and an extra
lookup to the
fast path it would seem that this is the wrong trade-off to do here.
All-in-all I lean towards moving forward with the first, simpler
revision of this
patch[1] and then evaluate if webrev.02 or a String-switch+static resolve
as a follow-up.
A compromise would be to keep the SpeciesLookup class introduced here
to allow removing all overhead in case the plugin is disabled.
Mandy: I've not found any test (under jdk/test/tools/jlink or elsewhere)
which
has to be updated when adding a plugin like this.
Thanks!
/Claes
[1] http://cr.openjdk.java.net/~redestad/8152641/webrev.01/
