Optimize UnionTypeCodec 23/40723/1
authorRobert Varga <rovarga@cisco.com>
Wed, 8 Jun 2016 11:59:17 +0000 (13:59 +0200)
committerRobert Varga <nite@hq.sk>
Wed, 22 Jun 2016 13:33:51 +0000 (13:33 +0000)
For some reason this codec was not using MethodHandle#invokeExact()
for instantiation. Fix this and remove static fields holding public
lookups.

Change-Id: I8b170ec850aa8182cbe8bf81a0fbbef796aad741
Signed-off-by: Robert Varga <rovarga@cisco.com>
(cherry picked from commit 7f3fa0ab3883fd8d8d3913df68435e6b1d49c486)

binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/DataObjectCodecContext.java
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/IdentifiableItemCodec.java
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/UnionTypeCodec.java
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/UnionValueOptionContext.java
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/ValueContext.java

index c06fc5ce9b33e3541a32c5c59f27ebeeeb752b63..3833645e72275f9cbce14d7362a28cc830216205 100644 (file)
@@ -14,7 +14,6 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSortedMap;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.MethodType;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
@@ -57,7 +56,6 @@ import org.slf4j.LoggerFactory;
 
 abstract class DataObjectCodecContext<D extends DataObject,T extends DataNodeContainer> extends DataContainerCodecContext<D,T> {
     private static final Logger LOG = LoggerFactory.getLogger(DataObjectCodecContext.class);
-    private static final Lookup LOOKUP = MethodHandles.publicLookup();
     private static final MethodType CONSTRUCTOR_TYPE = MethodType.methodType(void.class, InvocationHandler.class);
     private static final MethodType DATAOBJECT_TYPE = MethodType.methodType(DataObject.class, InvocationHandler.class);
     private static final Comparator<Method> METHOD_BY_ALPHABET = new Comparator<Method>() {
@@ -126,7 +124,8 @@ abstract class DataObjectCodecContext<D extends DataObject,T extends DataNodeCon
 
         final Class<?> proxyClass = Proxy.getProxyClass(getBindingClass().getClassLoader(),  new Class[] { getBindingClass(), AugmentationHolder.class });
         try {
-            proxyConstructor = LOOKUP.findConstructor(proxyClass, CONSTRUCTOR_TYPE).asType(DATAOBJECT_TYPE);
+            proxyConstructor = MethodHandles.publicLookup().findConstructor(proxyClass, CONSTRUCTOR_TYPE)
+                    .asType(DATAOBJECT_TYPE);
         } catch (NoSuchMethodException | IllegalAccessException e) {
             throw new IllegalStateException("Failed to find contructor for class " + proxyClass);
         }
index 09231179d2a4731fe48f1cf8311ff7d18da13f63..11ba801b4b5691eba05c3bf8d2ec7f0d7851762e 100644 (file)
@@ -12,7 +12,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.reflect.Constructor;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -35,7 +34,6 @@ final class IdentifiableItemCodec implements Codec<NodeIdentifierWithPredicates,
             return q1.getLocalName().compareToIgnoreCase(q2.getLocalName());
         }
     };
-    private static final Lookup LOOKUP = MethodHandles.publicLookup();
     private final Map<QName, ValueContext> keyValueContexts;
     private final List<QName> keysInBindingOrder;
     private final ListSchemaNode schema;
@@ -49,7 +47,7 @@ final class IdentifiableItemCodec implements Codec<NodeIdentifierWithPredicates,
         this.identifiable = identifiable;
 
         try {
-            ctor = LOOKUP.unreflectConstructor(getConstructor(keyClass));
+            ctor = MethodHandles.publicLookup().unreflectConstructor(getConstructor(keyClass));
         } catch (IllegalAccessException e) {
             throw new IllegalArgumentException("Missing construct in class " + keyClass);
         }
@@ -126,7 +124,6 @@ final class IdentifiableItemCodec implements Codec<NodeIdentifierWithPredicates,
         return new NodeIdentifierWithPredicates(schema.getQName(), values);
     }
 
-
     @SuppressWarnings("unchecked")
     private static Constructor<? extends Identifier<?>> getConstructor(final Class<? extends Identifier<?>> clazz) {
         for (@SuppressWarnings("rawtypes") final Constructor constr : clazz.getConstructors()) {
@@ -138,5 +135,4 @@ final class IdentifiableItemCodec implements Codec<NodeIdentifierWithPredicates,
         }
         throw new IllegalArgumentException("Supplied class " + clazz + "does not have required constructor.");
     }
-
 }
\ No newline at end of file
index 3ee4a2e23278b17866405a0eda18d77bf66bb78b..bdb6a99efe67abed70c44b0c476b0b0e47095129 100644 (file)
@@ -11,8 +11,9 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.io.BaseEncoding;
 import com.google.common.util.concurrent.ExecutionError;
 import com.google.common.util.concurrent.UncheckedExecutionException;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.InvocationTargetException;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
 import java.lang.reflect.Method;
 import java.util.HashSet;
 import java.util.Set;
@@ -22,23 +23,45 @@ import org.opendaylight.yangtools.concepts.Codec;
 import org.opendaylight.yangtools.yang.binding.BindingMapping;
 import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
 import org.opendaylight.yangtools.yang.model.api.type.UnionTypeDefinition;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 final class UnionTypeCodec extends ReflectionBasedCodec {
+    private static final MethodType CHARARRAY_LOOKUP_TYPE = MethodType.methodType(void.class, char[].class);
+    private static final MethodType CHARARRAY_INVOKE_TYPE = MethodType.methodType(Object.class, char[].class);
+    private static final MethodType CLASS_LOOKUP_TYPE = MethodType.methodType(void.class, Class.class);
+    private static final MethodType CLASS_INVOKE_TYPE = MethodType.methodType(Object.class, Object.class);
+    private static final Logger LOG = LoggerFactory.getLogger(UnionTypeCodec.class);
+
+    private final Codec<Object, Object> idRefCodec;
+    private final MethodHandle idrefConstructor;
 
-    private final Codec<Object, Object> identityrefCodec;
     private final ImmutableSet<UnionValueOptionContext> typeCodecs;
-    private final Constructor<?> charConstructor;
+    private final MethodHandle charConstructor;
 
     private UnionTypeCodec(final Class<?> unionCls,final Set<UnionValueOptionContext> codecs,
                            @Nullable final Codec<Object, Object> identityrefCodec) {
         super(unionCls);
-        this.identityrefCodec = identityrefCodec;
+        this.idRefCodec = identityrefCodec;
+        if (idRefCodec != null) {
+            try {
+                idrefConstructor = MethodHandles.publicLookup().findConstructor(unionCls, CLASS_LOOKUP_TYPE)
+                        .asType(CLASS_INVOKE_TYPE);
+            } catch (IllegalAccessException | NoSuchMethodException e) {
+                throw new IllegalStateException("Failed to get identityref constructor", e);
+            }
+        } else {
+            idrefConstructor = null;
+        }
+
         try {
-            charConstructor = unionCls.getConstructor(char[].class);
-            typeCodecs = ImmutableSet.copyOf(codecs);
-        } catch (NoSuchMethodException | SecurityException e) {
-           throw new IllegalStateException("Required constructor is not available.",e);
+            charConstructor = MethodHandles.publicLookup().findConstructor(unionCls, CHARARRAY_LOOKUP_TYPE)
+                    .asType(CHARARRAY_INVOKE_TYPE);
+        } catch (IllegalAccessException | NoSuchMethodException e) {
+            throw new IllegalStateException("Failed to instantiate handle for constructor", e);
         }
+
+        typeCodecs = ImmutableSet.copyOf(codecs);
     }
 
     static Callable<UnionTypeCodec> loader(final Class<?> unionCls, final UnionTypeDefinition unionType,
@@ -63,30 +86,34 @@ final class UnionTypeCodec extends ReflectionBasedCodec {
         };
     }
 
+    private Object deserializeString(final Object input) {
+        final String str = input instanceof byte[] ? BaseEncoding.base64().encode((byte[]) input) : input.toString();
+        try {
+            return charConstructor.invokeExact(str.toCharArray());
+        } catch (Throwable e) {
+            throw new IllegalStateException("Could not construct instance", e);
+        }
+    }
+
     @Override
     public Object deserialize(final Object input) {
-        if (identityrefCodec != null) {
+        if (idRefCodec != null) {
+            final Object identityref;
             try {
-                Object identityref = identityrefCodec.deserialize(input);
-                return getTypeClass().getConstructor(Class.class).newInstance(identityref);
+                identityref = idRefCodec.deserialize(input);
             } catch (UncheckedExecutionException | ExecutionError e) {
-                // ignore this exception caused by deserialize()
-            } catch (NoSuchMethodException e) {
-                // caused by getContructor(). this case shouldn't happen.
-                throw new IllegalStateException("Could not construct instance", e);
-            } catch (InstantiationException | IllegalAccessException | InvocationTargetException e) {
-                // ignore this exception caused by newInstance()
+                LOG.debug("Deserialization of {} as identityref failed", e);
+                return deserializeString(input);
             }
-        }
-        try {
-            if (input instanceof byte[]) {
-                return charConstructor.newInstance(BaseEncoding.base64().encode((byte[]) input).toCharArray());
-            } else {
-                return charConstructor.newInstance((input.toString().toCharArray()));
+
+            try {
+                return idrefConstructor.invokeExact(identityref);
+            } catch (Throwable e) {
+                LOG.debug("Failed to instantiate based on identityref {}", identityref, e);
             }
-        } catch (InstantiationException | IllegalAccessException | InvocationTargetException e) {
-            throw new IllegalStateException("Could not construct instance",e);
         }
+
+        return deserializeString(input);
     }
 
     @Override
@@ -101,5 +128,4 @@ final class UnionTypeCodec extends ReflectionBasedCodec {
         }
         return null;
     }
-
 }
index 7c4c14453f8649ae8b55fe8002a656d567de9d2e..15ce5a33d8f03e7b3238d6348f7a137b97c50385 100644 (file)
@@ -11,16 +11,13 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Method;
 import org.opendaylight.yangtools.concepts.Codec;
 
 final class UnionValueOptionContext {
-    private static final Lookup LOOKUP = MethodHandles.publicLookup();
     private static final MethodType OBJECT_TYPE = MethodType.methodType(Object.class, Object.class);
     private final Class<?> bindingType;
-    // FIXME: migrate to invocation
     private final MethodHandle getter;
     private final Codec<Object,Object> codec;
 
@@ -29,7 +26,7 @@ final class UnionValueOptionContext {
         this.codec = Preconditions.checkNotNull(codec);
 
         try {
-            this.getter = LOOKUP.unreflect(getter).asType(OBJECT_TYPE);
+            this.getter = MethodHandles.publicLookup().unreflect(getter).asType(OBJECT_TYPE);
         } catch (IllegalAccessException e) {
             throw new IllegalStateException("Failed to access method " + getter, e);
         }
index d99c3aaed9fb328aa0dc8e9ab926920f5db9515a..ce87e7ce46991fcfc0b0fe7aafc71904919ae0a6 100644 (file)
@@ -11,12 +11,10 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.MethodType;
 import org.opendaylight.yangtools.concepts.Codec;
 
 final class ValueContext {
-    private static final Lookup LOOKUP = MethodHandles.publicLookup();
     private static final MethodType OBJECT_METHOD = MethodType.methodType(Object.class, Object.class);
     private final Codec<Object, Object> codec;
     private final MethodHandle getter;
@@ -26,7 +24,7 @@ final class ValueContext {
     ValueContext(final Class<?> identifier, final LeafNodeCodecContext <?>leaf) {
         getterName = leaf.getGetter().getName();
         try {
-            getter = LOOKUP.unreflect(identifier.getMethod(getterName)).asType(OBJECT_METHOD);
+            getter = MethodHandles.publicLookup().unreflect(identifier.getMethod(getterName)).asType(OBJECT_METHOD);
         } catch (IllegalAccessException | NoSuchMethodException | SecurityException e) {
             throw new IllegalStateException(String.format("Cannot find method %s in class %s", getterName, identifier), e);
         }