@Param support for enum

[Joe Kearney]

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