RFR: JDK-8241798: Allow enums to have more constants

[forax at univ-mlv.fr]

> De: "Liam Miller-Cushon" <cushon at google.com>
> À: "Brian Goetz" <brian.goetz at oracle.com>
> Cc: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>, "compiler-dev"
> <compiler-dev at openjdk.java.net>, "Remi Forax" <forax at univ-mlv.fr>
> Envoyé: Mercredi 1 Avril 2020 02:58:32
> Objet: Re: RFR: JDK-8241798: Allow enums to have more constants

> Deferring the code golf approach makes sense to me, thanks. It might be an
> interesting option for code generators that are running into this limit, and
> that are willing to generate bytecode directly instead of source.
> I did some benchmarking of the original patch, and found that the difference is
> within measurement error and that the assembly appears to be identical, as
> expected. The benchmark is just looking at steady-state calls to values(), I'm
> not sure what the best way to measure the effect of the additional method call
> in the clinit would be. The code is here: [
> http://cr.openjdk.java.net/~cushon/benchmark/JDK-8241798/ |
> http://cr.openjdk.java.net/~cushon/benchmark/JDK-8241798/ ]

> Remy, you pointed out that this could make it harder for AOT compilers to find
> the value of the constants without executing the clinit. Do you have
> suggestions for quantifying that risk? I tried running a demo through
> GraalVM/native-image for fun and it didn't seem to cause any problems.

I was talking about the code proposed by Brian (create the array first and then initialize the enum constants later), 
I don't think this is an issue with your proposed patch (delegate the creation of the array to an helper method). 

Also playing with native image (for something else), i've found it very smart, at least smarter than the other AOT system i've used previously. 
I think native-image works too well to fairly represent the different other existing AOT systems that consume bytecodes. 

> Also, I fixed up the unnecessary `final` modifier in the synthetic values
> methods, and uploaded a new webrev: [
> http://cr.openjdk.java.net/~cushon/8241798/webrev.01/ |
> http://cr.openjdk.java.net/~cushon/8241798/webrev.01/ ]

Thank you for that, 
You have forgotten to update the corresponding comment 
// private static final synthetic T[] #values() { return new T[] { a, b, c }; } 

Otherwise, patch looks good to me. 

cheers, 
Rémi 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20200401/c767630b/attachment-0001.htm>