Use bindingEquals() generated for interfaces

As we are generating a default implementation of equals(), we
can defer to that implementation from runtime-generated proxies.

This results in:
- consistent results between compile-time and runtime implementations
- faster startup time, as there is only one dispatch implementation
- lower memory overhead, as runtime-generated classes are smaller
- more maintainable code, as the implementation can be examined at
  compile-time

Since this is the last method that references properties, we also
get to clean up some of the knowledge of how these need to be ordered.

JIRA: MDSAL-474
Change-Id: I1a0ca93755e670b7d8fa0834f87ee7a828843aa6
Signed-off-by: illia.ihushev <illia.ihushev@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/AugmentableCodecDataObject.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/AugmentableCodecDataObject.java
index 31a372b5cca74ebb5afe168c7ac8d20bd34a93ed..5f352490135f1726dd139b9fa92c4ad64961584e 100644 (file)
--- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/AugmentableCodecDataObject.java
+++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/AugmentableCodecDataObject.java
@@ -14,7 +14,6 @@ import java.lang.invoke.MethodHandles;
 import java.lang.invoke.VarHandle;
 import java.util.Optional;
 import org.eclipse.jdt.annotation.Nullable;
-import org.opendaylight.mdsal.binding.spec.reflect.BindingReflections;
 import org.opendaylight.yangtools.yang.binding.Augmentable;
 import org.opendaylight.yangtools.yang.binding.Augmentation;
 import org.opendaylight.yangtools.yang.binding.AugmentationHolder;
@@ -87,11 +86,6 @@ public abstract class AugmentableCodecDataObject<T extends DataObject & Augmenta
         return local != null ? local : loadAugmentations();
     }
 
-    @Override
-    final boolean codecAugmentedEquals(final T other) {
-        return super.codecAugmentedEquals(other) && augmentations().equals(BindingReflections.getAugmentations(other));
-    }
-
     private ImmutableMap<Class<? extends Augmentation<T>>, Augmentation<T>> acquireAugmentations() {
         return (ImmutableMap<Class<? extends Augmentation<T>>, Augmentation<T>>) CACHED_AUGMENTATIONS.getAcquire(this);
     }

diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObject.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObject.java
index 79c97d64d5cffec9e0ad0d58e318713ee9d213d8..b951590fcdf4b7fa5ec53b7747b409a1014f8f85 100644 (file)
--- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObject.java
+++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObject.java
@@ -10,7 +10,6 @@ package org.opendaylight.mdsal.binding.dom.codec.impl;
 import static com.google.common.base.Verify.verify;
 import static java.util.Objects.requireNonNull;
 
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.VarHandle;
 import java.util.Optional;
@@ -64,20 +63,9 @@ public abstract class CodecDataObject<T extends DataObject> implements DataObjec
     }
 
     @Override
-    @SuppressFBWarnings(value = "EQ_UNUSUAL", justification = "State is examined indirectly enough to confuse SpotBugs")
     public final boolean equals(final Object obj) {
-        if (obj == this) {
-            return true;
-        }
-        final Class<? extends DataObject> iface = implementedInterface();
-        if (!iface.isInstance(obj)) {
-            return false;
-        }
-        @SuppressWarnings("unchecked")
-        final T other = (T) iface.cast(obj);
-        // Note: we do not want to compare NormalizedNode data here, as we may be looking at different instantiations
-        //       of the same grouping -- in which case normalized node will not compare as equal.
-        return codecAugmentedEquals(other);
+        // Indirection to keep checkstyle happy
+        return codecEquals(obj);
     }
 
     @Override
@@ -105,7 +93,7 @@ public abstract class CodecDataObject<T extends DataObject> implements DataObjec
 
     protected abstract int codecHashCode();
 
-    protected abstract boolean codecEquals(T other);
+    protected abstract boolean codecEquals(Object obj);
 
     final @NonNull DataObjectCodecContext<T, ?> codecContext() {
         return context;
@@ -116,11 +104,6 @@ public abstract class CodecDataObject<T extends DataObject> implements DataObjec
         return data;
     }
 
-    // Non-final to allow specialization in AugmentableCodecDataObject
-    boolean codecAugmentedEquals(final T other) {
-        return codecEquals(other);
-    }
-
     // Helper split out of codecMember to aid its inlining
     private Object loadMember(final VarHandle handle, final NodeCodecContext childCtx) {
         @SuppressWarnings("unchecked")

diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectGenerator.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectGenerator.java
index 0d3a17e1f1a970494e9e071bcfffe90c0b2e0d7c..747af366b4145674fed891342f9f3295afcdcabd 100644 (file)
--- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectGenerator.java
+++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectGenerator.java
@@ -16,18 +16,12 @@ import static org.opendaylight.mdsal.binding.dom.codec.impl.ByteBuddyUtils.invok
 import static org.opendaylight.mdsal.binding.dom.codec.impl.ByteBuddyUtils.putField;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.VarHandle;
 import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Comparator;
-import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Objects;
 import java.util.Optional;
 import net.bytebuddy.ByteBuddy;
 import net.bytebuddy.description.field.FieldDescription;
@@ -42,11 +36,9 @@ import net.bytebuddy.implementation.bytecode.ByteCodeAppender;
 import net.bytebuddy.implementation.bytecode.StackManipulation;
 import net.bytebuddy.implementation.bytecode.assign.TypeCasting;
 import net.bytebuddy.implementation.bytecode.constant.ClassConstant;
-import net.bytebuddy.implementation.bytecode.constant.IntegerConstant;
 import net.bytebuddy.implementation.bytecode.constant.TextConstant;
 import net.bytebuddy.implementation.bytecode.member.MethodReturn;
 import net.bytebuddy.implementation.bytecode.member.MethodVariableAccess;
-import net.bytebuddy.jar.asm.Label;
 import net.bytebuddy.jar.asm.Opcodes;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.mdsal.binding.dom.codec.impl.ClassGeneratorBridge.LocalNameProvider;
@@ -185,11 +177,6 @@ abstract class CodecDataObjectGenerator<T extends CodecDataObject<?>> implements
             return tmp;
         }
 
-        @Override
-        ArrayList<Method> getterMethods() {
-            return new ArrayList<>(properties.keySet());
-        }
-
         @Override
         public NodeContextSupplier resolveNodeContextSupplier(final String methodName) {
             final Optional<Entry<Method, NodeContextSupplier>> found = properties.entrySet().stream()
@@ -234,14 +221,6 @@ abstract class CodecDataObjectGenerator<T extends CodecDataObject<?>> implements
             return tmp;
         }
 
-        @Override
-        ArrayList<Method> getterMethods() {
-            final ArrayList<Method> ret = new ArrayList<>(simpleProperties.size() + daoProperties.size());
-            ret.addAll(simpleProperties.keySet());
-            ret.addAll(daoProperties.keySet());
-            return ret;
-        }
-
         @Override
         public String resolveLocalName(final String methodName) {
             final Optional<Entry<Method, ValueNodeCodecContext>> found = simpleProperties.entrySet().stream()
@@ -253,17 +232,12 @@ abstract class CodecDataObjectGenerator<T extends CodecDataObject<?>> implements
 
     private static final Logger LOG = LoggerFactory.getLogger(CodecDataObjectGenerator.class);
     private static final Generic BB_BOOLEAN = TypeDefinition.Sort.describe(boolean.class);
-    private static final Generic BB_DATAOBJECT = TypeDefinition.Sort.describe(DataObject.class);
+    private static final Generic BB_OBJECT = TypeDefinition.Sort.describe(Object.class);
     private static final Generic BB_INT = TypeDefinition.Sort.describe(int.class);
     private static final Generic BB_STRING = TypeDefinition.Sort.describe(String.class);
     private static final TypeDescription BB_CDO = ForLoadedType.of(CodecDataObject.class);
     private static final TypeDescription BB_ACDO = ForLoadedType.of(AugmentableCodecDataObject.class);
-    private static final Comparator<Method> METHOD_BY_ALPHABET = Comparator.comparing(Method::getName);
 
-    private static final StackManipulation ARRAYS_EQUALS = invokeMethod(Arrays.class, "equals",
-        byte[].class, byte[].class);
-    private static final StackManipulation OBJECTS_EQUALS = invokeMethod(Objects.class, "equals",
-        Object.class, Object.class);
     private static final StackManipulation FIRST_ARG_REF = MethodVariableAccess.REFERENCE.loadFrom(1);
 
     private static final int PROT_FINAL = Opcodes.ACC_PROTECTED | Opcodes.ACC_FINAL | Opcodes.ACC_SYNTHETIC;
@@ -314,23 +288,14 @@ abstract class CodecDataObjectGenerator<T extends CodecDataObject<?>> implements
                 new KeyMethodImplementation(methodName, retType));
         }
 
-        // Index all property methods, turning them into "getFoo()" invocations, retaining order. We will be using
-        // those invocations in each of the three methods. Note that we do not glue the invocations to 'this', as we
-        // will be invoking them on 'other' in codecEquals()
-        final ArrayList<Method> properties = getterMethods();
-        // Make sure properties are alpha-sorted
-        properties.sort(METHOD_BY_ALPHABET);
-        final ImmutableMap<StackManipulation, Method> methods = Maps.uniqueIndex(properties,
-            ByteBuddyUtils::invokeMethod);
-
         // Final bits:
         return GeneratorResult.of(builder
                 // codecHashCode() ...
                 .defineMethod("codecHashCode", BB_INT, PROT_FINAL)
                 .intercept(codecHashCode(bindingInterface))
-                // ... codecEquals() ...
-                .defineMethod("codecEquals", BB_BOOLEAN, PROT_FINAL).withParameter(BB_DATAOBJECT)
-                .intercept(codecEquals(methods))
+                // ... equals(Object) ...
+                .defineMethod("codecEquals", BB_BOOLEAN, PROT_FINAL).withParameter(BB_OBJECT)
+                .intercept(codecEquals(bindingInterface))
                 // ... toString() ...
                 .defineMethod("toString", BB_STRING, PUB_FINAL)
                 .intercept(toString(bindingInterface))
@@ -340,38 +305,6 @@ abstract class CodecDataObjectGenerator<T extends CodecDataObject<?>> implements
 
     abstract Builder<T> generateGetters(Builder<T> builder);
 
-    abstract ArrayList<Method> getterMethods();
-
-    private static Implementation codecEquals(final ImmutableMap<StackManipulation, Method> properties) {
-        // Label for 'return false;'
-        final Label falseLabel = new Label();
-        // Condition for 'if (!...)'
-        final StackManipulation ifFalse = ByteBuddyUtils.ifEq(falseLabel);
-
-        final List<StackManipulation> manipulations = new ArrayList<>(properties.size() * 6 + 5);
-        for (Entry<StackManipulation, Method> entry : properties.entrySet()) {
-            // if (!java.util.(Objects|Arrays).equals(getFoo(), other.getFoo())) {
-            //     return false;
-            // }
-            manipulations.add(THIS);
-            manipulations.add(entry.getKey());
-            manipulations.add(FIRST_ARG_REF);
-            manipulations.add(entry.getKey());
-            manipulations.add(entry.getValue().getReturnType().isArray() ? ARRAYS_EQUALS : OBJECTS_EQUALS);
-            manipulations.add(ifFalse);
-        }
-
-        // return true;
-        manipulations.add(IntegerConstant.ONE);
-        manipulations.add(MethodReturn.INTEGER);
-        // L0: return false;
-        manipulations.add(ByteBuddyUtils.markLabel(falseLabel));
-        manipulations.add(IntegerConstant.ZERO);
-        manipulations.add(MethodReturn.INTEGER);
-
-        return new Implementation.Simple(manipulations.toArray(new StackManipulation[0]));
-    }
-
     private static Implementation codecHashCode(final Class<?> bindingInterface) {
         return new Implementation.Simple(
             // return Foo.bindingHashCode(this);
@@ -380,6 +313,15 @@ abstract class CodecDataObjectGenerator<T extends CodecDataObject<?>> implements
             MethodReturn.INTEGER);
     }
 
+    private static Implementation codecEquals(final Class<?> bindingInterface) {
+        return new Implementation.Simple(
+            // return Foo.bindingEquals(this, obj);
+            THIS,
+            FIRST_ARG_REF,
+            invokeMethod(bindingInterface, BindingMapping.BINDING_EQUALS_NAME, bindingInterface, Object.class),
+            MethodReturn.INTEGER);
+    }
+
     private static Implementation toString(final Class<?> bindingInterface) {
         return new Implementation.Simple(
             // return Foo.bindingToString(this);