tags:

views:

351

answers:

5

I looked under the hood for EnumSet.allOf and it looks very efficient, especially for enums with less than 64 values.

Basically all sets share the single array of all possible enum values and the only other piece of information is a bitmask which in case of allOf is set in one swoop.

On the other hand Enum.values() seems to be a bit of black magic. Moreover it returns an array, not a collection, so in many cases it must be decorated with Arrays.asList( ) to be usable in any place that expects collection.

So, should EnumSet.allOf be more preferable to Enum.values?

More specifically, which form of for iterator should be used:

for ( final MyEnum val: MyEnum.values( ) );

or

for ( final MyEnum val: EnumSet.allOf( MyEnum.values ) );
A: 

You should use the approach which is simplest and clearest to you. Performance shouldn't be a consideration in most situations.

IMHO: neither option performs very well as they both create objects. One in the first case and three in the second. You could construct a constant which holds all the values for performance reasons.

Peter Lawrey
Creating three objects as a performance consideration? Mate, it's not 1995 anymore...
Michael Borgwardt
Its 2010, and creating an object is still not free. For most programming, creating objects doesn't matter, but if performance really does matter, the number of objects you create can make still make a difference.
Peter Lawrey
I have worked on one project where each object created in the critical path costs over $200 per year. So three objects can sound expensive in some contexts, especially if you do this more than once.
Peter Lawrey
+3  A: 

The values() method is more clear and performant if you just want to iterate over all possible enum values. The values are cached by the class (see Class.getEnumConstants())

If you need a subset of values, you should use an EnumSet. Start with allOf() or noneOf() and add or remove values or use just of() as you need.

Arne Burmeister
The `values()` cannot be cached by the class, because it's an array and nothing will prevent the user from changing its values. Hence, I suspect it must be a clone. `EnumSet.allOf`, on the other hand does use a shared value for array, hence there is definitely less memory allocation here. So, `values` may be more clear, but I suspect it's not more performant.
Alexander Pogrebnyak
@Alexander: You are right, the array is cloned, but clone() is native. A little debugging shows me that getEnumConstants() uses values() not the other way round.
Arne Burmeister
+1  A: 

Not that I went through the entire implementation, but it seems to me that EnumSet.allOf() is basically using the same infrastructure as .values(). So I'd expect EnumSet.allOf() requires some (probably negligible) additional steps (see http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6276988).

It seems clear to me that the intended use of foreach is for(MyEnum val : MyEnum.values()) why do it differently? You will only confuse the maintenance programmer.

I mean, if you need a collection, you should get one. If you want to use a foreach, arrays are good enough. I'd even prefer arrays if pressed! Why wrap anything with anything, if what you got (array) is good enough? Simple things are normally faster.

In anyways, Peter Lawrey is right. Don't bother about the performance of this.. It's fast enough, and chances are there are million other bottlenecks that render that tiny theoretical performance difference as totally irrelevant (Don't see his "object creation" point though. To me the first example seems to be 100% OK).

Enno Shioji
@Zwei: see my comment to Arne's post
Alexander Pogrebnyak
@Alexander: OK, so they fixed the bug (see link) in JDK6? Well, I see your point then, but still I maintain the answer to your question "More specifically, which form of for iterator should be used" as "use the first example". I mean, I don't know. If you are doing some development in embedded, real time app. or something, maybe it's justifiable. But in a normal, general context? No.
Enno Shioji
A: 

There is also Class.getEnumConstants()

under the hood they all call values() methods of enum types anyway, through reflection.

irreputable
How does this relate to the question I've asked?
Alexander Pogrebnyak
A: 

Because I did not receive the answer to my question on which one is more efficient, I've decided to do some testing of my own.

I've tested iteration over values(), Arrays.asList( values() ) and EnumSet.allOf( ). I've repeated these tests 10,000,000 times for different enum sizes. Here are the test results:

oneValueEnum_testValues         1.328
oneValueEnum_testList           1.687
oneValueEnum_testEnumSet        0.578

TwoValuesEnum_testValues        1.360
TwoValuesEnum_testList          1.906
TwoValuesEnum_testEnumSet       0.797

ThreeValuesEnum_testValues      1.343
ThreeValuesEnum_testList        2.141
ThreeValuesEnum_testEnumSet     1.000

FourValuesEnum_testValues       1.375
FourValuesEnum_testList         2.359
FourValuesEnum_testEnumSet      1.219

TenValuesEnum_testValues        1.453
TenValuesEnum_testList          3.531
TenValuesEnum_testEnumSet       2.485

TwentyValuesEnum_testValues     1.656
TwentyValuesEnum_testList       5.578
TwentyValuesEnum_testEnumSet    4.750

FortyValuesEnum_testValues      2.016
FortyValuesEnum_testList        9.703
FortyValuesEnum_testEnumSet     9.266

These are results for tests ran from command line. When I ran these tests from Eclipse, I got overwhelming support for testValues. Basically it was smaller than EnumSet even for small enums. I believe that the performance gain comes from optimization of array iterator in for ( val : array ) loop.

On the other hand, as soon as you need a java.util.Collection to pass around, Arrays.asList( ) looses over to EnumSet.allOf, especially for small enums, which I believe will be a majority in any given codebase.

So, I would say you should use

for ( final MyEnum val: MyEnum.values( ) )

but

Iterables.filter(
    EnumSet.allOf( MyEnum.class ),
    new Predicate< MyEnum >( ) {...}
)

And only use Arrays.asList( MyEnum.values( ) ) where java.util.List is absolutely required.

Alexander Pogrebnyak