changeset 1024:9cc12a0ce88d

7901087: Fix AnnotationInvocationHandler equals/hashCode to enable duplicate annotations checks back
author shade
date Fri, 07 Nov 2014 15:45:28 +0300
parents 9237036291e9
children 40fcfac32653
files jmh-core-ct/src/test/java/org/openjdk/jmh/ct/collisions/CollisionTest.java jmh-core-ct/src/test/java/org/openjdk/jmh/ct/collisions/NoCollisionTest.java jmh-core-ct/src/test/java/org/openjdk/jmh/ct/collisions/SuperCollisionTest.java jmh-core-ct/src/test/java/org/openjdk/jmh/ct/collisions/SuperNoCollisionTest.java jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java jmh-core/src/main/java/org/openjdk/jmh/generators/core/MethodGroup.java jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMFieldInfo.java jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMMethodInfo.java jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/AnnotationInvocationHandler.java
diffstat 10 files changed, 410 insertions(+), 79 deletions(-) [+]
line wrap: on
line diff
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core-ct/src/test/java/org/openjdk/jmh/ct/collisions/CollisionTest.java	Fri Nov 07 15:45:28 2014 +0300
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.jmh.ct.collisions;
+
+import org.junit.Test;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Group;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.ct.CompileTest;
+
+@BenchmarkMode(Mode.All)
+public class CollisionTest {
+
+    @Benchmark
+    @Group("same")
+    @Warmup(iterations = 1)
+    public void g1() {
+
+    }
+
+    @Benchmark
+    @Group("same")
+    @Warmup(iterations = 5)
+    public void g2() {
+
+    }
+
+    @Test
+    public void compileTest() {
+        CompileTest.assertFail(this.getClass(), "Colliding annotations");
+    }
+
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core-ct/src/test/java/org/openjdk/jmh/ct/collisions/NoCollisionTest.java	Fri Nov 07 15:45:28 2014 +0300
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.jmh.ct.collisions;
+
+import org.junit.Test;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Group;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.ct.CompileTest;
+
+@BenchmarkMode(Mode.All)
+public class NoCollisionTest {
+
+    @Benchmark
+    @Group("same")
+    @Warmup(iterations = 1)
+    public void g1() {
+
+    }
+
+    @Benchmark
+    @Group("same")
+    @Warmup(iterations = 1)
+    public void g2() {
+
+    }
+
+    @Test
+    public void compileTest() {
+        CompileTest.assertOK(this.getClass());
+    }
+
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core-ct/src/test/java/org/openjdk/jmh/ct/collisions/SuperCollisionTest.java	Fri Nov 07 15:45:28 2014 +0300
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.jmh.ct.collisions;
+
+import org.junit.Test;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Group;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.ct.CompileTest;
+
+@Warmup(iterations = 1)
+@BenchmarkMode(Mode.All)
+public class SuperCollisionTest {
+
+    @Benchmark
+    @Group("same")
+    // inherited @Warmup(iterations = 1)
+    public void g1() {
+
+    }
+
+    @Benchmark
+    @Group("same")
+    @Warmup(iterations = 5)
+    public void g2() {
+
+    }
+
+    @Test
+    public void compileTest() {
+        CompileTest.assertFail(this.getClass(), "Colliding annotations");
+    }
+
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core-ct/src/test/java/org/openjdk/jmh/ct/collisions/SuperNoCollisionTest.java	Fri Nov 07 15:45:28 2014 +0300
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.jmh.ct.collisions;
+
+import org.junit.Test;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Group;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.ct.CompileTest;
+
+@Warmup(iterations = 1)
+@BenchmarkMode(Mode.All)
+public class SuperNoCollisionTest {
+
+    @Benchmark
+    @Group("same")
+    @Warmup(iterations = 1)
+    public void g1() {
+
+    }
+
+    @Benchmark
+    @Group("same")
+    @Warmup(iterations = 1)
+    public void g2() {
+
+    }
+
+    @Test
+    public void compileTest() {
+        CompileTest.assertOK(this.getClass());
+    }
+
+}
--- a/jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java	Thu Nov 06 15:52:35 2014 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java	Fri Nov 07 15:45:28 2014 +0300
@@ -155,33 +155,37 @@
         try {
             PrintWriter writer = new PrintWriter(destination.newResource(BenchmarkList.BENCHMARK_LIST.substring(1)));
             for (BenchmarkInfo info : benchmarkInfos) {
-                MethodGroup group = info.methodGroup;
-                String method = group.getName();
-                for (Mode m : group.getModes()) {
-                    BenchmarkListEntry br = new BenchmarkListEntry(
-                            info.userName + "." + method,
-                            info.generatedName + "." + method,
-                            m,
-                            group.getThreads(),
-                            group.getTotalThreadCount(),
-                            group.getWarmupIterations(),
-                            group.getWarmupTime(),
-                            group.getWarmupBatchSize(),
-                            group.getMeasurementIterations(),
-                            group.getMeasurementTime(),
-                            group.getMeasurementBatchSize(),
-                            group.getForks(),
-                            group.getWarmupForks(),
-                            group.getJvm(),
-                            group.getJvmArgs(),
-                            group.getJvmArgsPrepend(),
-                            group.getJvmArgsAppend(),
-                            group.getParams(),
-                            group.getOutputTimeUnit(),
-                            group.getOperationsPerInvocation(),
-                            group.getTimeout()
-                    );
-                    writer.println(br.toLine());
+                try {
+                    MethodGroup group = info.methodGroup;
+                    String method = group.getName();
+                    for (Mode m : group.getModes()) {
+                        BenchmarkListEntry br = new BenchmarkListEntry(
+                                info.userName + "." + method,
+                                info.generatedName + "." + method,
+                                m,
+                                group.getThreads(),
+                                group.getTotalThreadCount(),
+                                group.getWarmupIterations(),
+                                group.getWarmupTime(),
+                                group.getWarmupBatchSize(),
+                                group.getMeasurementIterations(),
+                                group.getMeasurementTime(),
+                                group.getMeasurementBatchSize(),
+                                group.getForks(),
+                                group.getWarmupForks(),
+                                group.getJvm(),
+                                group.getJvmArgs(),
+                                group.getJvmArgsPrepend(),
+                                group.getJvmArgsAppend(),
+                                group.getParams(),
+                                group.getOutputTimeUnit(),
+                                group.getOperationsPerInvocation(),
+                                group.getTimeout()
+                        );
+                        writer.println(br.toLine());
+                    }
+                } catch (GenerationException ge) {
+                    destination.printError(ge.getMessage(), ge.getElement());
                 }
             }
 
--- a/jmh-core/src/main/java/org/openjdk/jmh/generators/core/MethodGroup.java	Thu Nov 06 15:52:35 2014 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/generators/core/MethodGroup.java	Fri Nov 07 15:45:28 2014 +0300
@@ -262,11 +262,8 @@
         for (MethodInvocation mi : methods) {
             T ann = BenchmarkGeneratorUtils.getAnnSuper(mi.method, ci, annClass);
             if (ann != null) {
-                // FIXME: Temporalily disabled before we figure the proxy annotations equals/hashCode
-                if (false && finalAnn != null && !finalAnn.equals(ann)) {
-                    if (!finalAnn.equals(ann)) {
-                        throw new GenerationException("Colliding annotations: " + ann + " vs. " + finalAnn, mi.method);
-                    }
+                if (finalAnn != null && !finalAnn.equals(ann)) {
+                    throw new GenerationException("Colliding annotations: " + ann + " vs. " + finalAnn, mi.method);
                 }
                 finalAnn = ann;
             }
--- a/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java	Thu Nov 06 15:52:35 2014 +0300
+++ b/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java	Fri Nov 07 15:45:28 2014 +0300
@@ -101,8 +101,9 @@
 
     @Override
     public AnnotationVisitor visitAnnotation(final String desc, boolean visible) {
-        AnnotationInvocationHandler annHandler = new AnnotationInvocationHandler(super.visitAnnotation(desc, visible));
-        annotations.put(Type.getType(desc).getClassName(), annHandler);
+        String className = Type.getType(desc).getClassName();
+        AnnotationInvocationHandler annHandler = new AnnotationInvocationHandler(className, super.visitAnnotation(desc, visible));
+        annotations.put(className, annHandler);
         return annHandler;
     }
 
--- a/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMFieldInfo.java	Thu Nov 06 15:52:35 2014 +0300
+++ b/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMFieldInfo.java	Fri Nov 07 15:45:28 2014 +0300
@@ -88,8 +88,9 @@
 
     @Override
     public AnnotationVisitor visitAnnotation(final String desc, boolean visible) {
-        AnnotationInvocationHandler annHandler = new AnnotationInvocationHandler(super.visitAnnotation(desc, visible));
-        annotations.put(Type.getType(desc).getClassName(), annHandler);
+        String className = Type.getType(desc).getClassName();
+        AnnotationInvocationHandler annHandler = new AnnotationInvocationHandler(className, super.visitAnnotation(desc, visible));
+        annotations.put(className, annHandler);
         return annHandler;
     }
 
--- a/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMMethodInfo.java	Thu Nov 06 15:52:35 2014 +0300
+++ b/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMMethodInfo.java	Fri Nov 07 15:45:28 2014 +0300
@@ -75,8 +75,9 @@
 
     @Override
     public AnnotationVisitor visitAnnotation(final String desc, boolean visible) {
-        AnnotationInvocationHandler annHandler = new AnnotationInvocationHandler(super.visitAnnotation(desc, visible));
-        annotations.put(Type.getType(desc).getClassName(), annHandler);
+        String className = Type.getType(desc).getClassName();
+        AnnotationInvocationHandler annHandler = new AnnotationInvocationHandler(className, super.visitAnnotation(desc, visible));
+        annotations.put(className, annHandler);
         return annHandler;
     }
 
--- a/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/AnnotationInvocationHandler.java	Thu Nov 06 15:52:35 2014 +0300
+++ b/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/AnnotationInvocationHandler.java	Fri Nov 07 15:45:28 2014 +0300
@@ -32,72 +32,169 @@
 import java.lang.reflect.Array;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
-import java.util.ArrayList;
+import java.lang.reflect.Proxy;
 import java.util.Collection;
-import java.util.List;
+import java.util.HashSet;
+import java.util.Set;
 
 class AnnotationInvocationHandler extends AnnotationVisitor implements InvocationHandler {
+    private final String className;
     private final Multimap<String, Object> values;
 
-    public AnnotationInvocationHandler(AnnotationVisitor annotationVisitor) {
+    public AnnotationInvocationHandler(String className, AnnotationVisitor annotationVisitor) {
         super(Opcodes.ASM4, annotationVisitor);
+        this.className = className;
         this.values = new HashMultimap<String, Object>();
     }
 
     @Override
     public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-        String key = method.getName();
+        String member = method.getName();
         Class<?> returnType = method.getReturnType();
+        Class<?>[] paramTypes = method.getParameterTypes();
 
-        if (key.equalsIgnoreCase("toString")) {
-            return values;
+        if (member.equals("equals") && paramTypes.length == 1 &&
+            paramTypes[0] == Object.class)
+            return equalsImpl(args[0]);
+        if (paramTypes.length != 0)
+            throw new AssertionError("Too many parameters for an annotation method");
+        if (member.equals("toString")) {
+            return toStringImpl();
+        } else if (member.equals("hashCode")) {
+            return hashcodeImpl();
+        } else if (member.equals("annotationType")) {
+            throw new IllegalStateException("annotationType is not implemented");
         }
 
-        Collection<Object> vs = values.get(key);
+        /*
+          Unfortunately, we can not pre-process these values when walking the annotation
+          with ASM, since we are oblivious of exact types. Try to match the observed values
+          right here, based on the method return type.
+        */
 
-        if (returnType.isArray()) {
+        Collection<Object> vs = values.get(member);
+        if (vs == null || vs.isEmpty()) {
+            return method.getDefaultValue();
+        }
+
+        if (!returnType.isArray()) {
+            Object res = peelSingle(vs);
+
+            if (returnType.isEnum()) {
+                return parseEnum(returnType, res);
+            }
+
+            // String will return as is; primitive will auto-box
+            return res;
+        } else {
             Class<?> componentType = returnType.getComponentType();
+
             if (componentType.isEnum()) {
-                int count = vs.size();
-                List<Object> list = new ArrayList<Object>(vs);
+                // Dealing with Enum[]:
+                Object res = Array.newInstance(componentType, vs.size());
+                int c = 0;
+                for (Object v : vs) {
+                    Array.set(res, c, parseEnum(componentType, v));
+                    c++;
+                }
+                return res;
 
-                Object o = Array.newInstance(componentType, count);
-                for (int c = 0; c < count; c++) {
-                    Object v = list.get(c);
-                    if (v instanceof String) {
-                        Method m = componentType.getMethod("valueOf", String.class);
-                        v =  m.invoke(null, v);
-                    }
-                    Array.set(o, c, v);
+            } else if (componentType.isAssignableFrom(String.class)) {
+                // Dealing with String[]:
+                return vs.toArray(new String[vs.size()]);
+
+            } else {
+                // "Dealing" with primitive array:
+                // We do not have any primitive-array-valued annotations yet, so we don't bother to implement this.
+                throw new IllegalStateException("Primitive arrays are not handled yet");
+            }
+        }
+    }
+
+    private Object peelSingle(Collection<Object> vs) {
+        Object res;
+        if (vs.size() == 1) {
+            res = vs.iterator().next();
+        } else {
+            throw new IllegalStateException("Expected to see a single value, but got " + vs.size());
+        }
+        return res;
+    }
+
+    private String toStringImpl() {
+        StringBuilder sb = new StringBuilder();
+        sb.append("@");
+        sb.append(className);
+        sb.append("(");
+        for (String k : values.keys()) {
+            sb.append(k);
+            sb.append(" = ");
+            sb.append(values.get(k));
+            sb.append(", ");
+        }
+        sb.append(")");
+        return sb.toString();
+    }
+
+    private Object parseEnum(Class<?> type, Object res) throws Exception {
+        if (res == null) {
+            throw new IllegalStateException("The argument is null");
+        }
+        if (!(res instanceof String)) {
+            throw new IllegalStateException("The argument is not String, but " + res.getClass());
+        }
+        Method m = type.getMethod("valueOf", String.class);
+        return m.invoke(null, res);
+    }
+
+    private int hashcodeImpl() {
+        int result = className.hashCode();
+        for (String k : values.keys()) {
+            result = 31 * result + k.hashCode();
+        }
+        return result;
+    }
+
+    private boolean equalsImpl(Object arg) {
+        AnnotationInvocationHandler other = asOneOfUs(arg);
+        if (other != null) {
+            if (!className.equals(other.className)) {
+                return false;
+            }
+
+            Set<String> keys = new HashSet<String>();
+            keys.addAll(values.keys());
+            keys.addAll(other.values.keys());
+
+            for (String k : keys) {
+                Collection<Object> o1 = values.get(k);
+                Collection<Object> o2 = other.values.get(k);
+
+                if (o1 == null || o2 == null) {
+                    return false;
                 }
-                return o;
-            } else if (componentType.isPrimitive()) {
-                throw new IllegalStateException("Primitive arrays are not handled yet");
-            } else {
-                String[] strings = vs.toArray(new String[vs.size()]);
-                if (strings.length == 0) {
-                    strings = (String[]) method.getDefaultValue();
+
+                if (o1.size() != o2.size()) {
+                    return false;
                 }
-                return strings;
-            }
-        } else {
-            Object value;
-            if (vs == null || vs.isEmpty()) {
-                value = method.getDefaultValue();
-            } else {
-                if (vs.size() == 1) {
-                    value = vs.iterator().next();
-                } else {
-                    throw new IllegalStateException("Expected to see a single value, but got " + vs.size());
+
+                if (!o1.containsAll(o2) || !o2.containsAll(o1)) {
+                    return false;
                 }
             }
+            return true;
+        } else {
+            throw new IllegalStateException("Expected to see only AnnotationInvocationHandler-backed annotations");
+        }
+    }
 
-            if (returnType.isEnum() && (value instanceof String)) {
-                Method m = returnType.getMethod("valueOf", String.class);
-                return m.invoke(null, value);
-            }
-            return value;
+    private AnnotationInvocationHandler asOneOfUs(Object o) {
+        if (Proxy.isProxyClass(o.getClass())) {
+            InvocationHandler handler = Proxy.getInvocationHandler(o);
+            if (handler instanceof AnnotationInvocationHandler)
+                return (AnnotationInvocationHandler) handler;
         }
+        return null;
     }
 
     @Override