Optimize CodecDataObject dispatch 08/81708/8
authorRobert Varga <robert.varga@pantheon.tech>
Mon, 22 Apr 2019 20:47:43 +0000 (22:47 +0200)
committerRobert Varga <nite@hq.sk>
Wed, 24 Apr 2019 13:27:58 +0000 (13:27 +0000)
Now that we are in control of our interface between the DataObject
facade and DataObjectCodecContext rather than being forced to work
off of method name, we can optimize the dispatch towards
NodeContextSupplier (and DataObjectCodecContext size) by using
simple offsets to identify which method is being invoked.

On the generated code side of things, this lowers the pressure we
exert on the constant pool by using simple integers instead of
strings.

On the DataObjectCodecContext side, we ditch a Map in favor of a
simple array -- thus lowering memory overhead while also speeding
up dispatch, as it no longer is a Map lookup, but rather straight
get from an offset.

This necessitates special-casing key() method a bit, but the benefits
outweigh the (slight) ugliness we bring in.

JIRA: MDSAL-442
Change-Id: Iaf227e78d2b6546d34c2133fc31912c0a8dde4b3
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObject.java
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectCustomizer.java
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/KeyedListNodeCodecContext.java
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ListNodeCodecContext.java

index a11293f8dd304cd824bedc4af039de1616c786d5..78d8bc7cb89143819a6184966a245bca6f484e3b 100644 (file)
@@ -75,9 +75,9 @@ public abstract class CodecDataObject<T extends DataObject> implements DataObjec
 
     // TODO: consider switching to VarHandles for Java 9+
     protected final Object codecMember(final AtomicReferenceFieldUpdater<CodecDataObject<?>, Object> updater,
-            final String methodName) {
+            final int offset) {
         final Object cached = updater.get(this);
-        return cached != null ? unmaskNull(cached) : loadMember(updater, methodName);
+        return cached != null ? unmaskNull(cached) : loadMember(updater, offset);
     }
 
     protected abstract int codecHashCode();
@@ -108,8 +108,8 @@ public abstract class CodecDataObject<T extends DataObject> implements DataObjec
 
     // Helpers split out of codecMember to aid its inlining
     private Object loadMember(final AtomicReferenceFieldUpdater<CodecDataObject<?>, Object> updater,
-            final String methodName) {
-        final Object decoded = context.getBindingChildValue(methodName, data);
+            final int offset) {
+        final Object decoded = context.getBindingChildValue(data, offset);
         return updater.compareAndSet(this, null, maskNull(decoded)) ? decoded : unmaskNull(updater.get(this));
     }
 
index 67568ae8847f22f66a55f8303474aaeb084da5dc..6e90037e5ee9d7e0053839dd48fe5d59836b3793 100644 (file)
@@ -20,6 +20,7 @@ import javassist.CtClass;
 import javassist.CtField;
 import javassist.CtMethod;
 import javassist.NotFoundException;
+import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.mdsal.binding.dom.codec.loader.CodecClassLoader;
 import org.opendaylight.mdsal.binding.dom.codec.loader.CodecClassLoader.Customizer;
 import org.opendaylight.mdsal.binding.dom.codec.loader.StaticClassPool;
@@ -31,6 +32,8 @@ import org.slf4j.LoggerFactory;
  * Private support for generating AbstractDataObject specializations.
  */
 final class CodecDataObjectCustomizer implements Customizer {
+    static final int KEY_OFFSET = -1;
+
     private static final Logger LOG = LoggerFactory.getLogger(CodecDataObjectCustomizer.class);
     private static final CtClass CT_ARFU = StaticClassPool.findClass(AtomicReferenceFieldUpdater.class);
     private static final CtClass CT_BOOLEAN = StaticClassPool.findClass(boolean.class);
@@ -43,50 +46,26 @@ final class CodecDataObjectCustomizer implements Customizer {
     private static final CtClass[] TOSTRING_ARGS = new CtClass[] { CT_HELPER };
 
     private final List<Method> properties;
-    private final List<Method> methods;
+    private final Method keyMethod;
 
-    CodecDataObjectCustomizer(final List<Method> properties, final List<Method> methods) {
+    CodecDataObjectCustomizer(final List<Method> properties, final @Nullable Method keyMethod) {
         this.properties = requireNonNull(properties);
-        this.methods = requireNonNull(methods);
+        this.keyMethod = keyMethod;
     }
 
     @Override
     public List<Class<?>> customize(final CodecClassLoader loader, final CtClass bindingClass, final CtClass generated)
             throws NotFoundException, CannotCompileException {
-        final String classFqn = generated.getName();
+        // Generate members for all methods ...
+        LOG.trace("Generating class {}", generated.getName());
         generated.addInterface(bindingClass);
 
-        // Generate members for all methods ...
-        LOG.trace("Generating class {}", classFqn);
-        for (Method method : methods) {
-            LOG.trace("Generating for method {}", method);
-            final String methodName = method.getName();
-            final String methodArfu = methodName + "$$$ARFU";
-
-            // AtomicReferenceFieldUpdater ...
-            final CtField arfuField = new CtField(CT_ARFU, methodArfu, generated);
-            arfuField.setModifiers(Modifier.PRIVATE | Modifier.STATIC | Modifier.FINAL);
-            generated.addField(arfuField, new StringBuilder().append(CT_ARFU.getName()).append(".newUpdater(")
-                .append(generated.getName()).append(".class, java.lang.Object.class, \"").append(methodName)
-                .append("\")").toString());
-
-            // ... corresponding volatile field ...
-            final CtField field = new CtField(CT_OBJECT, methodName, generated);
-            field.setModifiers(Modifier.PRIVATE | Modifier.VOLATILE);
-            generated.addField(field);
-
-            // ... and the getter
-            final Class<?> retType = method.getReturnType();
-            final CtMethod getter = new CtMethod(loader.findClass(retType), methodName, EMPTY_ARGS, generated);
-            final String retName = retType.isArray() ? retType.getSimpleName() : retType.getName();
-
-            getter.setBody(new StringBuilder()
-                .append("{\n")
-                .append("return (").append(retName).append(") codecMember(").append(methodArfu).append(", \"")
-                    .append(methodName).append("\");\n")
-                .append('}').toString());
-            getter.setModifiers(Modifier.PUBLIC | Modifier.FINAL);
-            generated.addMethod(getter);
+        int offset = 0;
+        for (Method method : properties) {
+            generateMethod(loader, generated, method, offset++);
+        }
+        if (keyMethod != null) {
+            generateMethod(loader, generated, keyMethod, KEY_OFFSET);
         }
 
         // Final bits: codecHashCode() ...
@@ -155,6 +134,38 @@ final class CodecDataObjectCustomizer implements Customizer {
                 .append('}').toString();
     }
 
+    private static void generateMethod(final CodecClassLoader loader, final CtClass generated, final Method method,
+            final int offset) throws CannotCompileException, NotFoundException {
+        LOG.trace("Generating for method {}", method);
+        final String methodName = method.getName();
+        final String methodArfu = methodName + "$$$ARFU";
+
+        // AtomicReferenceFieldUpdater ...
+        final CtField arfuField = new CtField(CT_ARFU, methodArfu, generated);
+        arfuField.setModifiers(Modifier.PRIVATE | Modifier.STATIC | Modifier.FINAL);
+        generated.addField(arfuField, new StringBuilder().append(CT_ARFU.getName()).append(".newUpdater(")
+            .append(generated.getName()).append(".class, java.lang.Object.class, \"").append(methodName)
+            .append("\")").toString());
+
+        // ... corresponding volatile field ...
+        final CtField field = new CtField(CT_OBJECT, methodName, generated);
+        field.setModifiers(Modifier.PRIVATE | Modifier.VOLATILE);
+        generated.addField(field);
+
+        // ... and the getter
+        final Class<?> retType = method.getReturnType();
+        final CtMethod getter = new CtMethod(loader.findClass(retType), methodName, EMPTY_ARGS, generated);
+        final String retName = retType.isArray() ? retType.getSimpleName() : retType.getName();
+
+        getter.setBody(new StringBuilder()
+            .append("{\n")
+            .append("return (").append(retName).append(") codecMember(").append(methodArfu).append(", ").append(offset)
+                .append(");\n")
+            .append('}').toString());
+        getter.setModifiers(Modifier.PUBLIC | Modifier.FINAL);
+        generated.addMethod(getter);
+    }
+
     private static String utilClass(final Method method) {
         // We can either have objects or byte[], we cannot have Object[]
         return method.getReturnType().isArray() ? "Arrays" : "Objects";
index 69b326d749ea35a740cbe6c6982e775b3f54f7e9..3ac825690dcaebeae17ead4053296cb9baf14b1a 100644 (file)
@@ -21,7 +21,6 @@ import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -88,13 +87,14 @@ abstract class DataObjectCodecContext<D extends DataObject, T extends DataNodeCo
     private static final CtClass SUPERCLASS = StaticClassPool.findClass(CodecDataObject.class);
     private static final CtClass AUGMENTABLE_SUPERCLASS = StaticClassPool.findClass(
         AugmentableCodecDataObject.class);
+    private static final NodeContextSupplier[] EMPTY_METHODS = new NodeContextSupplier[0];
 
     private final ImmutableMap<String, ValueNodeCodecContext> leafChild;
     private final ImmutableMap<YangInstanceIdentifier.PathArgument, NodeContextSupplier> byYang;
-    private final ImmutableMap<String, NodeContextSupplier> byMethod;
     private final ImmutableMap<Class<?>, DataContainerCodecPrototype<?>> byStreamClass;
     private final ImmutableMap<Class<?>, DataContainerCodecPrototype<?>> byBindingArgClass;
     private final ImmutableMap<AugmentationIdentifier, Type> possibleAugmentations;
+    private final NodeContextSupplier[] byMethod;
     private final MethodHandle proxyConstructor;
 
     @SuppressWarnings("rawtypes")
@@ -105,7 +105,11 @@ abstract class DataObjectCodecContext<D extends DataObject, T extends DataNodeCo
 
     private volatile ImmutableMap<Class<?>, DataContainerCodecPrototype<?>> mismatchedAugmented = ImmutableMap.of();
 
-    DataObjectCodecContext(final DataContainerCodecPrototype<T> prototype, final Method... additionalMethods) {
+    DataObjectCodecContext(final DataContainerCodecPrototype<T> prototype) {
+        this(prototype, null);
+    }
+
+    DataObjectCodecContext(final DataContainerCodecPrototype<T> prototype, final @Nullable Method keyMethod) {
         super(prototype);
 
         final Class<D> bindingClass = getBindingClass();
@@ -146,21 +150,19 @@ abstract class DataObjectCodecContext<D extends DataObject, T extends DataNodeCo
             }
         }
 
-        final int methodCount = tmpMethodToSupplier.size();
-        final Builder<String, NodeContextSupplier> byMethodBuilder = ImmutableMap.builderWithExpectedSize(methodCount);
-
-
-        final List<Method> properties = new ArrayList<>(methodCount);
-        for (Entry<Method, NodeContextSupplier> entry : tmpMethodToSupplier.entrySet()) {
-            final Method method = entry.getKey();
-            properties.add(method);
-            byMethodBuilder.put(method.getName(), entry.getValue());
-        }
-
         // Make sure properties are alpha-sorted
+        final List<Method> properties = new ArrayList<>(tmpMethodToSupplier.keySet());
         properties.sort(METHOD_BY_ALPHABET);
+        if (!properties.isEmpty()) {
+            byMethod = new NodeContextSupplier[properties.size()];
+            int offset = 0;
+            for (Method prop : properties) {
+                byMethod[offset++] = verifyNotNull(tmpMethodToSupplier.get(prop));
+            }
+        } else {
+            byMethod = EMPTY_METHODS;
+        }
 
-        this.byMethod = byMethodBuilder.build();
         this.byYang = ImmutableMap.copyOf(byYangBuilder);
         this.byStreamClass = ImmutableMap.copyOf(byStreamClassBuilder);
         byBindingArgClassBuilder.putAll(byStreamClass);
@@ -176,19 +178,10 @@ abstract class DataObjectCodecContext<D extends DataObject, T extends DataNodeCo
         }
         reloadAllAugmentations();
 
-        final List<Method> methods;
-        if (additionalMethods.length != 0) {
-            methods = new ArrayList<>(properties.size() + 1);
-            methods.addAll(properties);
-            methods.addAll(Arrays.asList(additionalMethods));
-        } else {
-            methods = properties;
-        }
-
         final Class<?> generatedClass;
         try {
             generatedClass = prototype.getFactory().getLoader().generateSubclass(superClass, bindingClass, "codecImpl",
-                new CodecDataObjectCustomizer(properties, methods));
+                new CodecDataObjectCustomizer(properties, keyMethod));
         } catch (CannotCompileException | IOException | NotFoundException e) {
             throw new LinkageError("Failed to generated class for " + bindingClass, e);
         }
@@ -524,9 +517,8 @@ abstract class DataObjectCodecContext<D extends DataObject, T extends DataNodeCo
     }
 
     @SuppressWarnings("rawtypes")
-    @Nullable Object getBindingChildValue(final String method, final NormalizedNodeContainer domData) {
-        final NodeCodecContext childContext = verifyNotNull(byMethod.get(method),
-            "Cannot find data handler for method %s", method).get();
+    @Nullable Object getBindingChildValue(final NormalizedNodeContainer domData, final int offset) {
+        final NodeCodecContext childContext = byMethod[offset].get();
 
         @SuppressWarnings("unchecked")
         final Optional<NormalizedNode<?, ?>> domChild = domData.getChild(childContext.getDomPathArgument());
index 0da9d7a9921fd81b14d2fe74c42103ef5ff1d443..2c16d7a49d4111ea614f62fe0dbe04bbf616f8d7 100644 (file)
@@ -61,12 +61,12 @@ final class KeyedListNodeCodecContext<D extends DataObject & Identifiable<?>> ex
 
     @Override
     @SuppressWarnings("rawtypes")
-    Object getBindingChildValue(final String methodName, final NormalizedNodeContainer dom) {
-        if (dom instanceof MapEntryNode && IDENTIFIABLE_KEY_NAME.equals(methodName)) {
+    Object getBindingChildValue(final NormalizedNodeContainer dom, final int offset) {
+        if (offset == CodecDataObjectCustomizer.KEY_OFFSET && dom instanceof MapEntryNode) {
             NodeIdentifierWithPredicates identifier = ((MapEntryNode) dom).getIdentifier();
             return codec.deserialize(identifier).getKey();
         }
-        return super.getBindingChildValue(methodName, dom);
+        return super.getBindingChildValue(dom, offset);
     }
 
     @Override
index e6c9447f9047c900f7d42b48de71766a61c6b546..7380700b8525eb7032b11038560bc6c39d358f4c 100644 (file)
@@ -19,9 +19,12 @@ import org.opendaylight.yangtools.yang.data.api.schema.UnkeyedListNode;
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
 
 class ListNodeCodecContext<D extends DataObject> extends DataObjectCodecContext<D,ListSchemaNode> {
-    protected ListNodeCodecContext(final DataContainerCodecPrototype<ListSchemaNode> prototype,
-            final Method... additionalMethods) {
-        super(prototype, additionalMethods);
+    ListNodeCodecContext(final DataContainerCodecPrototype<ListSchemaNode> prototype) {
+        super(prototype);
+    }
+
+    ListNodeCodecContext(final DataContainerCodecPrototype<ListSchemaNode> prototype, final Method keyMethod) {
+        super(prototype, keyMethod);
     }
 
     @Override