JMH does not correctly handle Enum params that override toString()

[Anuraag Agrawal]

Thanks Alexey, I will take those pointers and try producing a regression
test tomorrow and submit a more complete patch.
For reference, the original patch I sent that got stripped is this (just
changes toString() to name() since we know the variables are Enum):

# HG changeset patch
# User Anuraag Agrawal <anuraaga at gmail.com>
# Date 1512560148 -32400
#      Wed Dec 06 20:35:48 2017 +0900
# Node ID c8e5e85046656062bb2070318e51fac4cff4b4e4
# Parent  1ddf31f810a3100b9433c3fedf24615e85b1d1a7
Use Enum.name() instead of Enum.toString() for determining the string value
of enum params. Only Enum.name() is guaranteed to round-trip through
Enum.valueOf during execution, the specification explicitly allows
overriding toString() to something that will not round-trip.

diff --git
a/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java
b/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java
---
a/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java
+++
b/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java
@@ -221,7 +221,7 @@
             try {
                 Collection<String> res = new ArrayList<>();
                 for (Object cnst : Class.forName(origQualifiedName, false,
Thread.currentThread().getContextClassLoader()).getEnumConstants()) {
-                    res.add(cnst.toString());
+                    res.add(((Enum<?>) cnst).name());
                 }
                 return res;
             } catch (ClassNotFoundException e) {
diff --git
a/jmh-generator-reflection/src/main/java/org/openjdk/jmh/generators/reflection/RFClassInfo.java
b/jmh-generator-reflection/src/main/java/org/openjdk/jmh/generators/reflection/RFClassInfo.java
---
a/jmh-generator-reflection/src/main/java/org/openjdk/jmh/generators/reflection/RFClassInfo.java
+++
b/jmh-generator-reflection/src/main/java/org/openjdk/jmh/generators/reflection/RFClassInfo.java
@@ -162,7 +162,7 @@
     public Collection<String> getEnumConstants() {
         Collection<String> res = new ArrayList<>();
         for (Object cnst : klass.getEnumConstants()) {
-            res.add(cnst.toString());
+            res.add(((Enum<?>) cnst).name());
         }
         return res;
     }


On Mon, Dec 25, 2017 at 12:57 AM, Aleksey Shipilev <shade at redhat.com> wrote:

> On 12/24/2017 04:54 PM, Aleksey Shipilev wrote:
> > On 12/24/2017 04:48 PM, Anuraag Agrawal wrote:
> >> Instead, name() should be used when round-tripping through valueOf is
> >> required
> >>
> >> https://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html#name()
> >>
> >> This affects the reflection and ASM generators. Annotation processor is
> >> unaffected since it directly reads the declaration token.
> >
> > That makes sense.
> >
> >> The patch is simple and I've attached it,
> >
> > Patch got stripped :( Please put it inline?
> >
> >> but could not find any tests that
> >> round-trip reflection/ASM benchmarks from compile -> run (the issue only
> >> affects running a benchmark, the compile succeeds). if this patch makes
> >> sense, any advice on writing a regression test would be appreciated.
> >
> > Run the build with appropriate profile, and the tests would run with
> appropriate generator:
> >
> >  $ mvn clean install -Pasm
> >  $ mvn clean install -Preflection
>
> Ah, also make the regression test that trips on this issue, and run with
> some profile above. Our CI
> runs all tests with all profiles, so that would be a valid regression
> test. Since this is runtime
> failure, put the test somewhere in jmh-core-it.
>
> Thanks,
> -Aleksey
>
>
>