Optimize single-property ClassTemplate hashCode() 19/77919/5
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 18 Nov 2018 20:12:08 +0000 (21:12 +0100)
committerJie Han <han.jie@zte.com.cn>
Thu, 22 Nov 2018 08:45:17 +0000 (08:45 +0000)
When we have a single property, it does not make sense to generate
anything aside from delegation to the field, without any additional
arithmetic.

While this changes hashCode() results, these are still consistent
and for bonus points save ourselves some amount of bytecode and make
concentrate some common template code.

JIRA: MDSAL-400
Change-Id: I2035acd30d7c9653fcf7d9af4abcad9032ab795c
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/JavaFileTemplate.java
binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java

index f758906d8677fed2c1f5e3c90a65a3648aee2fc0..7c0658f41b6fb328e63a107197ab3a1a8c890b6f 100644 (file)
@@ -476,4 +476,12 @@ abstract class BaseTemplate extends JavaFileTemplate {
            «ENDIF»
        «ENDFOR»
     '''
+
+    def protected hashCodeResult(Collection<GeneratedProperty> properties) '''
+        final int prime = 31;
+        int result = 1;
+        «FOR property : properties»
+            result = prime * result + «property.importedUtilClass».hashCode(«property.fieldName»);
+        «ENDFOR»
+    '''
 }
index 03a421aa29052af98ba050c79f6c4115721941e3..1f7a83ea2bdf4a3bc15614127465106c5b8d6cfc 100644 (file)
@@ -11,7 +11,6 @@ import static org.opendaylight.mdsal.binding.spec.naming.BindingMapping.AUGMENTA
 import static org.opendaylight.mdsal.binding.spec.naming.BindingMapping.AUGMENTABLE_AUGMENTATION_NAME
 
 import com.google.common.collect.ImmutableMap
-import java.util.Arrays
 import java.util.List
 import java.util.Map
 import java.util.Objects
@@ -72,15 +71,7 @@ class BuilderImplTemplate extends AbstractBuilderTemplate {
                     return hash;
                 }
 
-                final int prime = 31;
-                int result = 1;
-                «FOR property : properties»
-                    «IF property.returnType.name.contains("[")»
-                    result = prime * result + «Arrays.importedName».hashCode(«property.fieldName»);
-                    «ELSE»
-                    result = prime * result + «Objects.importedName».hashCode(«property.fieldName»);
-                    «ENDIF»
-                «ENDFOR»
+                «hashCodeResult(properties)»
                 «IF augmentType !== null»
                     result = prime * result + «Objects.importedName».hashCode(«AUGMENTATION_FIELD»);
                 «ENDIF»
@@ -113,11 +104,7 @@ class BuilderImplTemplate extends AbstractBuilderTemplate {
                 «targetType.importedName» other = («targetType.importedName»)obj;
                 «FOR property : properties»
                     «val fieldName = property.fieldName»
-                    «IF property.returnType.name.contains("[")»
-                    if (!«Arrays.importedName».equals(«fieldName», other.«property.getterMethodName»())) {
-                    «ELSE»
-                    if (!«Objects.importedName».equals(«fieldName», other.«property.getterMethodName»())) {
-                    «ENDIF»
+                    if (!«property.importedUtilClass».equals(«fieldName», other.«property.getterMethodName»())) {
                         return false;
                     }
                 «ENDFOR»
index e34952a04c36b23d4841a441a3fc226922f435a8..d8568bfc8257c8953727b6f6331a990d8585c21f 100644 (file)
@@ -15,7 +15,6 @@ import com.google.common.collect.Lists
 import com.google.common.io.BaseEncoding
 import java.beans.ConstructorProperties
 import java.util.ArrayList
-import java.util.Arrays
 import java.util.Collections
 import java.util.List
 import java.util.Map
@@ -476,23 +475,23 @@ class ClassTemplate extends BaseTemplate {
      *
      * @return string with the <code>hashCode()</code> method definition in JAVA format
      */
-    def protected generateHashCode() '''
-        «IF !genTO.hashCodeIdentifiers.empty»
+    def protected generateHashCode() {
+        val size = genTO.hashCodeIdentifiers.size
+        if (size == 0) {
+            return ""
+        }
+        return '''
             @«Override.importedName»
             public int hashCode() {
-                final int prime = 31;
-                int result = 1;
-                «FOR property : genTO.hashCodeIdentifiers»
-                    «IF property.returnType.name.contains("[")»
-                    result = prime * result + «Arrays.importedName».hashCode(«property.fieldName»);
-                    «ELSE»
-                    result = prime * result + «Objects.importedName».hashCode(«property.fieldName»);
-                    «ENDIF»
-                «ENDFOR»
+            «IF size != 1»
+                «hashCodeResult(genTO.hashCodeIdentifiers)»
                 return result;
+            «ELSE»
+                return «CodeHelpers.importedName».wrapperHashCode(«genTO.hashCodeIdentifiers.get(0).fieldName»);
+            «ENDIF»
             }
-        «ENDIF»
-    '''
+        '''
+    }
 
     /**
      * Template method which generates the method <code>equals()</code>.
@@ -515,11 +514,7 @@ class ClassTemplate extends BaseTemplate {
                 «type.name» other = («type.name») obj;
                 «FOR property : genTO.equalsIdentifiers»
                     «val fieldName = property.fieldName»
-                    «IF property.returnType.name.contains("[")»
-                    if (!«Arrays.importedName».equals(«fieldName», other.«fieldName»)) {
-                    «ELSE»
-                    if (!«Objects.importedName».equals(«fieldName», other.«fieldName»)) {
-                    «ENDIF»
+                    if (!«property.importedUtilClass».equals(«fieldName», other.«fieldName»)) {
                         return false;
                     }
                 «ENDFOR»
index a414f707bd4d035c9b09e0178198ec2f2adac62f..7aee12a6f9ca04fc29136fac0cc2088efe303f2a 100644 (file)
@@ -10,6 +10,8 @@ package org.opendaylight.mdsal.binding.java.api.generator;
 import static com.google.common.base.Verify.verify;
 import static java.util.Objects.requireNonNull;
 
+import java.util.Arrays;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import org.opendaylight.mdsal.binding.model.api.ConcreteType;
@@ -95,6 +97,17 @@ class JavaFileTemplate {
                 : new ClassTemplate(innerJavaType, gto).generateAsInnerClass();
     }
 
+    /**
+     * Return imported name of java.util class, whose hashCode/equals methods we want to invoke on the property. Returns
+     * {@link Arrays} if the property is an array, {@link Objects} otherwise.
+     *
+     * @param property Generated property
+     * @return Imported class name
+     */
+    final String importedUtilClass(final GeneratedProperty property) {
+        return importedName(property.getReturnType().getName().indexOf('[') != -1 ? Arrays.class : Objects.class);
+    }
+
     static final Restrictions restrictionsForSetter(final Type actualType) {
         return actualType instanceof GeneratedType ? null : getRestrictions(actualType);
     }
index 568eb8798d3395925e547f31346ffa3021eae710..2b8ee01b9a8f0e7e7fbcc7bbbcc578a74aaf632e 100644 (file)
@@ -16,6 +16,7 @@ import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
 import java.util.regex.Pattern;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
@@ -205,4 +206,45 @@ public final class CodeHelpers {
     public static <T> List<T> nonnull(final @Nullable List<T> input) {
         return input != null ? input : ImmutableList.of();
     }
+
+    /**
+     * Return hash code of a single-property wrapper class. Since the wrapper is not null, we really want to discern
+     * this object being present, hence {@link Objects#hashCode()} is not really useful we would end up with {@code 0}
+     * for both non-present and present-with-null objects.
+     *
+     * @param obj Internal object to hash
+     * @return Wrapper object hash code
+     */
+    public static int wrapperHashCode(final @Nullable Object obj) {
+        return wrapHashCode(Objects.hashCode(obj));
+    }
+
+    /**
+     * Return hash code of a single-property wrapper class. Since the wrapper is not null, we really want to discern
+     * this object being present, hence {@link Arrays#hashCode()} is not really useful we would end up with {@code 0}
+     * for both non-present and present-with-null objects.
+     *
+     * @param obj Internal object to hash
+     * @return Wrapper object hash code
+     */
+    public static int wrapperHashCode(final byte @Nullable[] obj) {
+        return wrapHashCode(Arrays.hashCode(obj));
+    }
+
+    /**
+     * The constant '31' is the result of folding this code:
+     * <pre>
+     *     final int prime = 31;
+     *     int result = 1;
+     *     result = result * prime + Objects.hashCode(obj);
+     *     return result;
+     * </pre>
+     * when hashCode is returned as 0, such as due to obj being null or its hashCode being 0.
+     *
+     * @param hash Wrapped object hash
+     * @return Wrapper object hash
+     */
+    private static int wrapHashCode(int hash) {
+        return hash == 0 ? 31 : hash;
+    }
 }