From 259957ffe47414cc197f4a819361e8a6dcf50b19 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 22 Apr 2019 22:47:43 +0200 Subject: [PATCH] Optimize CodecDataObject dispatch 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 --- .../dom/codec/impl/CodecDataObject.java | 8 +- .../codec/impl/CodecDataObjectCustomizer.java | 81 +++++++++++-------- .../codec/impl/DataObjectCodecContext.java | 48 +++++------ .../codec/impl/KeyedListNodeCodecContext.java | 6 +- .../dom/codec/impl/ListNodeCodecContext.java | 9 ++- 5 files changed, 79 insertions(+), 73 deletions(-) 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 a11293f8dd..78d8bc7cb8 100644 --- 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 @@ -75,9 +75,9 @@ public abstract class CodecDataObject implements DataObjec // TODO: consider switching to VarHandles for Java 9+ protected final Object codecMember(final AtomicReferenceFieldUpdater, 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 implements DataObjec // Helpers split out of codecMember to aid its inlining private Object loadMember(final AtomicReferenceFieldUpdater, 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)); } diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectCustomizer.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectCustomizer.java index 67568ae884..6e90037e5e 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectCustomizer.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectCustomizer.java @@ -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 properties; - private final List methods; + private final Method keyMethod; - CodecDataObjectCustomizer(final List properties, final List methods) { + CodecDataObjectCustomizer(final List properties, final @Nullable Method keyMethod) { this.properties = requireNonNull(properties); - this.methods = requireNonNull(methods); + this.keyMethod = keyMethod; } @Override public List> 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"; diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java index 69b326d749..3ac825690d 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java @@ -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 leafChild; private final ImmutableMap byYang; - private final ImmutableMap byMethod; private final ImmutableMap, DataContainerCodecPrototype> byStreamClass; private final ImmutableMap, DataContainerCodecPrototype> byBindingArgClass; private final ImmutableMap possibleAugmentations; + private final NodeContextSupplier[] byMethod; private final MethodHandle proxyConstructor; @SuppressWarnings("rawtypes") @@ -105,7 +105,11 @@ abstract class DataObjectCodecContext, DataContainerCodecPrototype> mismatchedAugmented = ImmutableMap.of(); - DataObjectCodecContext(final DataContainerCodecPrototype prototype, final Method... additionalMethods) { + DataObjectCodecContext(final DataContainerCodecPrototype prototype) { + this(prototype, null); + } + + DataObjectCodecContext(final DataContainerCodecPrototype prototype, final @Nullable Method keyMethod) { super(prototype); final Class bindingClass = getBindingClass(); @@ -146,21 +150,19 @@ abstract class DataObjectCodecContext byMethodBuilder = ImmutableMap.builderWithExpectedSize(methodCount); - - - final List properties = new ArrayList<>(methodCount); - for (Entry 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 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 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> domChild = domData.getChild(childContext.getDomPathArgument()); diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/KeyedListNodeCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/KeyedListNodeCodecContext.java index 0da9d7a992..2c16d7a49d 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/KeyedListNodeCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/KeyedListNodeCodecContext.java @@ -61,12 +61,12 @@ final class KeyedListNodeCodecContext> 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 diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ListNodeCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ListNodeCodecContext.java index e6c9447f90..7380700b85 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ListNodeCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ListNodeCodecContext.java @@ -19,9 +19,12 @@ import org.opendaylight.yangtools.yang.data.api.schema.UnkeyedListNode; import org.opendaylight.yangtools.yang.model.api.ListSchemaNode; class ListNodeCodecContext extends DataObjectCodecContext { - protected ListNodeCodecContext(final DataContainerCodecPrototype prototype, - final Method... additionalMethods) { - super(prototype, additionalMethods); + ListNodeCodecContext(final DataContainerCodecPrototype prototype) { + super(prototype); + } + + ListNodeCodecContext(final DataContainerCodecPrototype prototype, final Method keyMethod) { + super(prototype, keyMethod); } @Override -- 2.36.6