@Param support for enum

[Joe Kearney]

The bugfix broke IterationBlackhole2Test, I'll have a quick look before
committing that.


On 9 June 2014 22:03, Joe Kearney <mail at joekearney.co.uk> wrote:

> New patch attached for enum params.
>
>
> On 9 June 2014 21:36, Joe Kearney <mail at joekearney.co.uk> wrote:
>
>> Static methods in BenchmarkGenerator - I didn't move them because they
>> were only used locally. Happy to move them out if you'd prefer.
>>
>> Fair point on predicate length. I'm inclined to pull
>> isEmptyEnumParam(FieldInfo, String[]) into BenchmarkGenerator as there are
>> already a couple of uses of that. I considered an inner if, but the else
>> branch of that would be the same as the non-enum case, from what I could
>> see.
>>
>> Patch to follow, thanks for feedback.
>>
>>
>> On 9 June 2014 21:14, Aleksey Shipilev <aleksey.shipilev at oracle.com>
>> wrote:
>>
>>> Hi Joe,
>>>
>>> On 06/10/2014 12:02 AM, Joe Kearney wrote:
>>> > Aleksey, as discussed here is the patch. It includes the main change in
>>> > BenchmarkGenerator and StateObjectHandler, plus runtime and
>>> compile-time
>>> > tests.
>>>
>>> Thanks!
>>>
>>> > Couple of notes:
>>> > * I factored out addParameterValuesToGroup because the code was
>>> > duplicated between step 1 and 2 of @Param discovery.
>>>
>>> Looks good, but please move both static methods into BenchmarkUtils.
>>>
>>> > * I'd advocate renaming getEnumConstants to getEnumConstantNames, but
>>> > maybe that's just a matter of taste.
>>>
>>> Truly, a matter of taste :)
>>>
>>> > * I'm pretty sure there's a typo-bug in BenchmarkGenerator line 311, it
>>> > should loop over fields from state#getFields, not over fields in clazz,
>>> > an unused local variable warning.
>>>
>>> D'oh! Thanks for catching it, can you submit a separate patch?
>>>
>>> > Please let me know any feedback you have. I'm around tonight for any
>>> > changes you'd like to make.
>>>
>>> Okay, here it goes:
>>>
>>>  * Static utility methods better be in BenchmarkUtils.
>>>  * Please avoid long predicates: I understand it is easier to write out
>>> " && !fi.getType().isEnum()", but I think these checks deserve a
>>> separate inner if.
>>>  * ...this, AFAIU, will save you from additional condition in
>>> "isParamValueConforming": the control should not go there for blank
>>> arguments
>>>
>>> Otherwise looks good. I'll wait for your new patch.
>>>
>>> Thanks,
>>> -Aleksey.
>>>
>>>
>>
>