Beware of Enum.values()!

[theagentd]

Although rarely used, Enum.values() is a function that returns an array holding all the possible values of the specified Enum. It generates a new array every time it's called! Since there's no way to force the immutability of the array, I guess that was their only way of doing this.

I just eliminated 200+MB/s of garbage by writing my own cached values() function which returns a precomputed static array of the values in the enum.

1   2   3   4

private static EnumName[] values = values();    public static EnumName[] valuesCached(){       return values;    }

EDIT: Modified the code example to look more sane with syntax highlighting.

[Riven]

Same for any critical method that returns supposedly immutable JVM state.

Class.getName() -> new String instance
Class.getMethods() -> new array of new Method instances

[Nate]

Also, varargs.

Games published by our own members! Check 'em out!

[concerto49]

Also, varargs.

What does varargs have to do with immutable arrays?

[nsigma]

Class.getName() -> new String instance

Why the hell would getName() have to create new Strings?

Also, varargs.

What does varargs have to do with immutable arrays?

While I'm not sure what Nate means exactly, one thing that's annoying with varargs is that they accept arrays, which means if you need to ensure it's private you need to clone the array you get, despite the fact that 99% of the time it's pointless.

[Riven]

Class.getName() -> new String instance

Why the hell would getName() have to create new Strings?

Because using reflection, you can alter the char[] that is backing the String, messing up JVM & app logic.

[nsigma]

@Riven - ah, yes. A pointless thing to guard against in (most) user code, but makes sense for the JVM.

[davedes]

Nice tip. I really like working with Enums, so I tend to use this commonly.

Regarding varargs; I think the problem NateS is talking about is when you use it like this:

1   2   3

public void draw(Texture ... textures) {     ... }

As I understand it, a new array is created whenever you call that method, regardless of parameter count:

1   2   3

draw(); // -> new array length 0 draw(tex); // -> new array length 1 draw(tex, tex2); //new array length 2

Or maybe he is talking about something else...

[ReBirth]

Wow I use values() a lot!

About varargs, passing the array reference solves don't it?

1	public void test(T[] t);

[ra4king]

Holy shitballs thank you, I just changed fixed my code to cache the values() array

Games published by our own members! Check 'em out!

[nsigma]

About varargs, passing the array reference solves don't it?

1	public void test(T[] t);

I know why that's allowed, but I wish it wasn't (see my earlier comment).

The varargs array is a prime candidate for removal via escape analysis.  This may not be the panacea you're looking for. 

[nsigma]

@Riven - ah, yes. A pointless thing to guard against in (most) user code, but makes sense for the JVM.

Follow up - except according to the code here (http://hg.openjdk.java.net/jdk7u/jdk7u-gate/jdk/file/965f6e358f61/src/share/classes/java/lang/Class.java) it actually doesn't!   

[sproingie]

The array returned by Enum.values() is not going to change at runtime.  You don't so much need to cache it automatically as simply give it a scope that's appropriate for where you're using it.  Maybe that's global, so the static cache location is appropriate then, but don't just cargo-cult valuesCached() onto your enum classes without first seeing if you can just hoist the values() array out of your loop instead.

[theagentd]

The array returned by Enum.values() is not going to change at runtime.  You don't so much need to cache it automatically as simply give it a scope that's appropriate for where you're using it.  Maybe that's global, so the static cache location is appropriate then, but don't just cargo-cult valuesCached() onto your enum classes without first seeing if you can just hoist the values() array out of your loop instead.

In my test I couldn't get it out of the loop. I'm looping over the 6 different sides of a voxel and transform them based on a rotation (all precomputed so it simply follows a few references to do the calculations), so I needed to loop over the sides. I don't know if it's a 100% valid use of enums or values() in the first place, but I did notice that it produced a shitload of garbage when I (re)generated the geometry of chunks which happens for the first few seconds when the game starts.

I just decided to post this because the fix is so ridiculously simple as long as you promise not to change the content of the array. People seem to have had the same problem with other things that return arrays too, so I just thought it was posting since someone might be calling those functions a lot like me. ^^ values() is rarely used in the first place, so of course I wouldn't just throw in valuesCached() into every enum I create. In fact I'm still using values() during load time since I only bothered to change my code to use valuesCached() when it actually mattered.

[sproingie]

Java has to make a defensive copy of the array, since it lacks the concept of an immutable array type.  It could theoretically hoist it out of the loop, but the data flow analyzer doesn't take heroic measures (the general problem of deciding a value's lifetime is undecideable) so it's going to leave the array creation in the loop most if not all of the time.

Sometimes the best use case is to cache the values right on the enum type, sure.  I'm just warning against blindly applying this everywhere (thus the "cargo-cult" reference) when simply assigning the array locally once outside the loop might sometimes be more appropriate.

[theagentd]

Java has to make a defensive copy of the array, since it lacks the concept of an immutable array type.  It could theoretically hoist it out of the loop, but the data flow analyzer doesn't take heroic measures (the general problem of deciding a value's lifetime is undecideable) so it's going to leave the array creation in the loop most if not all of the time.

I'm... pretty sure I just said that the original post... ^^'

[Nate]

Regarding varargs; I think the problem NateS is talking about is when you use it like this:

Yep, using varargs allocates anew array, which is annoying.

[noblemaster]

Also, addAll(...) in ArrayList creates a new array each time before adding the elements!

http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/util/ArrayList.java#ArrayList.addAll%28java.util.Collection%29

[Riven]

Similarily, Collections.sort(...) also creates a Object[] to do the actual sorting.

[ReBirth]

Also, addAll(...) in ArrayList creates a new array each time before adding the elements!

Similarily, Collections.sort(...) also creates a Object[] to do the actual sorting.

Life is so hard...

[Catharsis]

Since we're expanding on various oddities.. With J2SE Enum hashCode() returns the identity hash code on Android hashCode() returns the ordinal + hash code of the String name of the enum. Doh! If you try and mix a lot of Enum with other objects returning the identity hash code on Android collisions potentially occur. Also if you have two Enum classes with a duplicate name in the same position the hash codes collide. Doh!

On Android:

1   2   3   4

@Override     public final int hashCode() {         return ordinal + (name == null ? 0 : name.hashCode());     }

[Dejay]

I was a curious and made a few tests because I thought escape analysis would simply take care of this as long as you don't keep a reference to the Enum.values(). But it doesn't. It seems escape analysis only works for primitive arrays as a VarArg parameter.

VarArgs are ok IF they are primitive and smaller than 64 elements, then they are simply allocated on the stack. Here is a link to a test program, with escape analysis that specific test is a 110 times faster! But it doesn't seem to work for object arrays :/

Defensive copies of objects returned or temp objects work rather well though.

The lack of documentation and JIT compiler makes it really hard to test this stuff.

[princec]

It could be argued that if you've got a varargs argument in a performance critical section of code you're maybe not writing performance aware code in the first place of course.

Cas

[relminator]

Thanks!!! I was about to use Enum.values() for my singleton explosion class.

[nsigma]

VarArgs are ok IF they are primitive and smaller than 64 elements, then they are simply allocated on the stack. Here is a link to a test program, with escape analysis that specific test is a 110 times faster! But it doesn't seem to work for object arrays :/

I'd meant to link to that article earlier and forgot.  However, where are you getting the "doesn't seem to work for object arrays" from?  The linked article doesn't seem to imply that, so wondered about the test.  I would have thought the JIT compiler could easily inline the object references to remove the array, even if not inlining fields from the objects themselves!?

It could be argued that if you've got a varargs argument in a performance critical section of code you're maybe not writing performance aware code in the first place of course.

hmm ... yes and no ... potentially.  I read another article recently (and can't remember the damn link) which suggested varargs could be faster than passing in a cached array because escape analysis might remove the array access code too.

Have I added in enough "could"s, "suggested"s, "might"s and "pray to the magic fairy" in all that? 

[princec]

I was more thinking along the lines of, if you're calling a fairly generic function that's meant to take arbitrary parameters, but you are in performance critical code and already know in advance what your parameters are... may as well optimise that properly.

Cas