Hi Mandy,
Thanks for your review.
I have added a test as you suggested and switched to `.descriptorString()`.
https://cr.openjdk.java.net/~gdub/8242451/webrev.2/
Gilles
On 25/08/2020 19:23, Mandy Chung wrote:
On 8/25/20 3:16 AM, Gilles Duboscq wrote:
Since this patch caches a singleton instance in the generated class, it could
apply to the eager initialization case as well and replace the current use of
core reflection to new an instance except that the target of the returning
callsite would always be the singleton object (the result of invoking the
static getter method handle). I wonder if there is any performance difference.
This is just a thought that we can file a JBS issue to follow up.
Can you add a test case for this fix?
I could write a test that generates different output depending on whether a
singleton or a fresh instance is returned.
Then i can compare the output when running with
`jdk.internal.lambda.disableEagerInitialization` set to `true` and `false`.
What is the recommended way of comparing 2 runs like that in jtreg?
It can simply verify if the static field is only present when
disableEagerInitialization is true; otherwise such static field should not
exist.
I have updated the webrev: https://cr.openjdk.java.net/~gdub/8242451/webrev.1/
Looks okay to me.
Nit: `Class::descriptorString` can be used instead of
BytecodeDescriptor::unparse.
418 String lambdaTypeDescriptor =
BytecodeDescriptor.unparse(invokedType.returnType());
From your other mail:
Should this patch be a workaround to existing releases rather than the main
line? As Brian mentions, lambda proxy class may become inline class in
valhalla repo (Roger has a patch already). The earlier fixing those programs
the better.
Indeed if we know this is landing in this cycle in the main repo there's no
point with my fix. Could you point me at the issue number or mail thread where
this patch is being discussed?
As Brian replied, integration of valhalla is not imminent. JDK-8205668 tracks
the work to make lambda proxies as inline classes for the valhalla repo (*not*
main line).
Mandy
