Beware of Enum.values()!

Shared Code
#1

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.


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

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

#2

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

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

#3

Also, varargs. :frowning:

#4

What does varargs have to do with immutable arrays?

#5

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

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.

#6

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

#7

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

#8

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:

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

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

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… :slight_smile:

#9

Wow I use values() a lot! :o

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


public void test(T[] t);
#10

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

#11

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. :wink:

#12

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! :persecutioncomplex:

#13

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.

#14

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.

#15

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.

#16

I’m… pretty sure I just said that the original post… ^^’

#17

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

#18

Also, [b]addAll/b 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(java.util.Collection)

#19

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

#20

Life is so hard…