Optimize bytecode for combination enhanced-for and enums

[Maurizio Cimadamore]

Hi,
this seems doable - note that the precedent you mention doesn't simply 
cache the values() arrays - instead it generates code like the following:

class Outer$0 {
      synthetic final int[] $EnumMap$Color = new int[Color.values().length];
      static {
          try { $EnumMap$Color[red.ordinal()] = 1; } catch 
(NoSuchFieldError ex) {}
          try { $EnumMap$Color[green.ordinal()] = 2; } catch 
(NoSuchFieldError ex) {}
      }
}

This is to guarantee that the client's view of the enum is consistent 
with the actual enum contents (because of separate compilation). 
However, it seems like this extra-consistency check could be skipped in 
your proposed optimization, as a for-loop iterating over values() 
doesn't directly rely on the enum shape as a switch statement does.

Another way to do this would be to generate a method that returns an 
Iterable<Edge> - and have the compiler rewrite

for (Edge e : Edge.values()) { ... }

into:

for (Edge : edge$Values()) { ... }

Here, the iterator creation is somewhat lighter than the array creation 
and the laziness of this scheme makes sure that nothing gets initialized 
if the method is not called at all.

Maurizio

On 28/03/12 14:39, 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
>