Optimize bytecode for combination enhanced-for and enums

[Rémi Forax]

On 03/28/2012 04:42 PM, Roel Spilker wrote:
> Rémi, are you sure the clone call can be removed? I agree that that 
> would even be better for hotspot JVMs that use escape analysis. That 
> said, apart from engineering effort, if a synthetic inner class is 
> used you get the benefit immediately, even on VMs that cannot afford 
> escape analysis.
>
> Roel

A synthetic inner class also carry it's own bloat, you have to store
a new class on disk, verify it at load time, the VM has to create an 
instance of Class
for it (and this is not a lightweight object).

clone() is a kind of allocation, so yes, it can be removed if the escape 
analysis has a special case for it.
But I don't know if Hotspot does that, for the record an array is often 
already a special case
for the escape analysis algorithm because each cell has to be considered 
has a kind of pseudo field.

Rémi

>
>
> On Wed, Mar 28, 2012 at 4:13 PM, Rémi Forax <forax at univ-mlv.fr 
> <mailto:forax at univ-mlv.fr>> wrote:
>
>     On 03/28/2012 03:39 PM, Roel Spilker wrote:
>
>         Hi all,
>
>         TL;DR: for (Edge edge : Edge.values()) copies an array every
>         time it is executed. This can be prevented. Is that a good
>         idea and how to proceed from here?
>
>         I'm new to this list, so please let me know if this is not the
>         place to post this.
>
>         When I was profiling our application I notices that we
>         allocate quite some memory when iterating over enum values.
>         The pattern used was:
>
>         class  Foo {
>          void bar() {
>            for (Edge e : Edge.values()) {
>              // do some work
>            }
>          }
>         }
>
>         The call to Edge.values() obviously creates a new clone of the
>         Edge[] containing all enum constants. So we've modified our code:
>
>         class  Foo {
>          private static final Edge[] EDGES = Edge.values();
>          void bar() {
>            for (Edge e : EDGES) {
>              // do some work
>            }
>          }
>         }
>
>         This solves our allocation problem, but it is not a nice
>         solution. Since code in the enhanced-for has no way to modify
>         the array this pattern can be done at compile-time. A
>         synthetic inner class can be generated to keep the copy of the
>         array. The desugared (pseudo) code would then look something
>         like this:
>
>         /* syncthetic */ class Foo$0 {
>          static final Edge[] EDGES = Edge.values();
>         }
>
>         class  Foo {
>          void bar() {
>            for (Edge e : Foo$0.EDGES) {
>              // do some work
>            }
>          }
>         }
>
>         There is precedence for this kind of desugaring/optimization:
>         When you use an enum-in-switch, a synthetic class is generated
>         containing an int array for the ordinals.
>
>         I have a few questions:
>         - Do you think this is a good optimization? The trade-off here
>         is creating a copy every time the enhanced-for is used (could
>         be in an inner loop) versus the overhead of loading an extra
>         class.
>         - Is there a better optimization possible/required? EnumSet
>         uses
>         SharedSecrets.getJavaLangAccess().getEnumConstantsShared(elementType),
>         but that won't work in user code.
>         - If it is a good idea, how do I proceed from here? Possibly
>         create a JEP? How can I contribute? Who wants to support?
>
>         Roel Spilker
>
>
>     Hi Roel,
>     There are several issues with your template, EDGES is initialized
>     too soon, i.e.
>     even if you don't call bar() and stay too long, i.e even if you
>     never call bar() more than once.
>
>     I think it's better to let the the escape analysis pass done by
>     the JIT to inline
>     values() and remove the call to clone().
>
>     Rémi
>
>
>
>
>