@Param support for enum

[Joe Kearney]

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.
>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: param-for-enums.patch
Type: text/x-patch
Size: 15514 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/jmh-dev/attachments/20140609/d7011505/param-for-enums-0001.patch>