From: Robert Varga Date: Wed, 1 May 2019 20:46:04 +0000 (+0200) Subject: Do not inline NodeContextSuppliers X-Git-Tag: v4.0.1~11 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=97adfd595ac4812dc2a612a1bf56af4952e17e4d;p=mdsal.git Do not inline NodeContextSuppliers As it turns out, NodeContextSuppliers really need to be coming from parent, as we can be rehosting objects into different namespaces -- in which case a single object will need to be interpreted in terms of the NodeContextSupplier in its parent, i.e. with a different schema. This patch undoes most of Iaf227e78d2b6546d34c2133fc31912c0a8dde4b3 and I9e24c30535c536d70b4eb60035b5aa744fc6ec9d, in that NodeContextSuppliers are not inlined as constants, but rather looked up. Unlike the previous lookup by method name, we are using localName and Class, just as we do in stremers and hence we are reusing indices which support streamers. The ability to inlike NodeContextSuppliers is retained, but since DataObjectContext does not have enough information from BindingRuntimeContext, this facility is not used. JIRA: MDSAL-443 Change-Id: I82bff8657845bdab27aa3aa8c24a949c4758f1bb Signed-off-by: Robert Varga --- 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 0ab5ef23f9..939348edc8 100644 --- 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 com.google.common.collect.ImmutableMap; import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; -import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.mdsal.binding.dom.codec.util.AugmentationReader; import org.opendaylight.mdsal.binding.spec.reflect.BindingReflections; @@ -34,8 +33,6 @@ import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; */ public abstract class AugmentableCodecDataObject> extends CodecDataObject implements Augmentable, AugmentationHolder { - private final @NonNull DataObjectCodecContext context; - @SuppressWarnings("rawtypes") private static final AtomicReferenceFieldUpdater CACHED_AUGMENTATIONS_UPDATER = AtomicReferenceFieldUpdater.newUpdater(AugmentableCodecDataObject.class, @@ -45,7 +42,6 @@ public abstract class AugmentableCodecDataObject context, final NormalizedNodeContainer data) { super(context, data); - this.context = requireNonNull(context, "Context must not be null"); } @SuppressWarnings("unchecked") @@ -59,7 +55,7 @@ public abstract class AugmentableCodecDataObject> optAugCtx = context.possibleStreamChild( + final Optional> optAugCtx = codecContext().possibleStreamChild( (Class) augmentationType); if (optAugCtx.isPresent()) { final DataContainerCodecContext augCtx = optAugCtx.get(); @@ -85,7 +81,7 @@ public abstract class AugmentableCodecDataObject DataObject type */ public abstract class CodecDataObject implements DataObject { + // An object representing a null value in a member field. private static final @NonNull Object NULL_VALUE = new Object(); + private final @NonNull DataObjectCodecContext context; @SuppressWarnings("rawtypes") private final @NonNull NormalizedNodeContainer data; @@ -38,6 +41,7 @@ public abstract class CodecDataObject implements DataObjec protected CodecDataObject(final DataObjectCodecContext context, final NormalizedNodeContainer data) { this.data = requireNonNull(data, "Data must not be null"); + this.context = requireNonNull(context, "Context must not be null"); } @Override @@ -78,15 +82,26 @@ public abstract class CodecDataObject implements DataObjec // TODO: consider switching to VarHandles for Java 9+, as that would disconnect us from the need to use a volatile // field and use acquire/release mechanics -- see http://gee.cs.oswego.edu/dl/html/j9mm.html for details. protected final Object codecMember(final AtomicReferenceFieldUpdater, Object> updater, - final NodeContextSupplier supplier) { + final String localName) { + final Object cached = updater.get(this); + return cached != null ? unmaskNull(cached) : loadMember(updater, context.getLeafChild(localName)); + } + + protected final Object codecMember(final AtomicReferenceFieldUpdater, Object> updater, + final Class bindingClass) { final Object cached = updater.get(this); - return cached != null ? unmaskNull(cached) : loadMember(updater, supplier); + return cached != null ? unmaskNull(cached) : loadMember(updater, context.streamChild(bindingClass)); } protected final Object codecMember(final AtomicReferenceFieldUpdater, Object> updater, - final IdentifiableItemCodec codec) { + final NodeContextSupplier supplier) { + final Object cached = updater.get(this); + return cached != null ? unmaskNull(cached) : loadMember(updater, supplier.get()); + } + + protected final Object codecKey(final AtomicReferenceFieldUpdater, Object> updater) { final Object cached = updater.get(this); - return cached != null ? unmaskNull(cached) : loadKey(updater, codec); + return cached != null ? cached : loadKey(updater); } protected abstract int codecHashCode(); @@ -95,6 +110,10 @@ public abstract class CodecDataObject implements DataObjec protected abstract ToStringHelper codecFillToString(ToStringHelper helper); + final @NonNull DataObjectCodecContext codecContext() { + return context; + } + @SuppressWarnings("rawtypes") final @NonNull NormalizedNodeContainer codecData() { return data; @@ -117,28 +136,24 @@ public abstract class CodecDataObject implements DataObjec // Helpers split out of codecMember to aid its inlining private Object loadMember(final AtomicReferenceFieldUpdater, Object> updater, - final NodeContextSupplier supplier) { - final NodeCodecContext context = supplier.get(); - + final NodeCodecContext childCtx) { @SuppressWarnings("unchecked") - final Optional> child = data.getChild(context.getDomPathArgument()); + final Optional> child = data.getChild(childCtx.getDomPathArgument()); // We do not want to use Optional.map() here because we do not want to invoke defaultObject() when we have // normal value because defaultObject() may end up throwing an exception intentionally. - return updateCache(updater, child.isPresent() ? context.deserializeObject(child.get()) - : context.defaultObject()); + final Object obj = child.isPresent() ? childCtx.deserializeObject(child.get()) : childCtx.defaultObject(); + return updater.compareAndSet(this, null, maskNull(obj)) ? obj : unmaskNull(updater.get(this)); } // Helpers split out of codecMember to aid its inlining - private Object loadKey(final AtomicReferenceFieldUpdater, Object> updater, - final IdentifiableItemCodec codec) { + private Object loadKey(final AtomicReferenceFieldUpdater, Object> updater) { verify(data instanceof MapEntryNode, "Unsupported value %s", data); - return updateCache(updater, codec.deserialize(((MapEntryNode) data).getIdentifier()).getKey()); - } - - private Object updateCache(final AtomicReferenceFieldUpdater, Object> updater, - final Object obj) { - return updater.compareAndSet(this, null, maskNull(obj)) ? obj : unmaskNull(updater.get(this)); + verify(context instanceof KeyedListNodeCodecContext, "Unexpected context %s", context); + final Identifier key = ((KeyedListNodeCodecContext) context).deserialize( + ((MapEntryNode) data).getIdentifier()); + // key is known to be non-null, no need to mask it + return updater.compareAndSet(this, null, key) ? key : updater.get(this); } private static @NonNull Object maskNull(final @Nullable Object unmasked) { diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectBridge.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectBridge.java index 696937edeb..d79340cdd6 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectBridge.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectBridge.java @@ -12,6 +12,7 @@ import static com.google.common.base.Verify.verifyNotNull; import com.google.common.annotations.Beta; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; +import org.opendaylight.mdsal.binding.dom.codec.impl.CodecDataObjectGenerator.Fixed; /** * Bridge for initializing {@link CodecDataObject} instance constants during class loading time. This class is public @@ -19,7 +20,7 @@ import org.eclipse.jdt.annotation.Nullable; */ @Beta public final class CodecDataObjectBridge { - private static final ThreadLocal> CURRENT_CUSTOMIZER = new ThreadLocal<>(); + private static final ThreadLocal> CURRENT_CUSTOMIZER = new ThreadLocal<>(); private CodecDataObjectBridge() { @@ -29,17 +30,13 @@ public final class CodecDataObjectBridge { return current().resolve(methodName); } - public static @NonNull IdentifiableItemCodec resolveKey(final @NonNull String methodName) { - return current().resolveKey(methodName); - } - - static @Nullable CodecDataObjectGenerator setup(final @NonNull CodecDataObjectGenerator next) { - final CodecDataObjectGenerator prev = CURRENT_CUSTOMIZER.get(); + static @Nullable Fixed setup(final @NonNull Fixed next) { + final Fixed prev = CURRENT_CUSTOMIZER.get(); CURRENT_CUSTOMIZER.set(verifyNotNull(next)); return prev; } - static void tearDown(final @Nullable CodecDataObjectGenerator prev) { + static void tearDown(final @Nullable Fixed prev) { if (prev == null) { CURRENT_CUSTOMIZER.remove(); } else { @@ -47,7 +44,7 @@ public final class CodecDataObjectBridge { } } - private static @NonNull CodecDataObjectGenerator current() { + private static @NonNull Fixed current() { return verifyNotNull(CURRENT_CUSTOMIZER.get(), "No customizer attached"); } } 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 94b4803fbd..4bee13e5ef 100644 --- 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 @@ -22,7 +22,9 @@ import com.google.common.collect.Maps; 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; @@ -86,7 +88,6 @@ import org.slf4j.LoggerFactory; * 
  *     public final class Foo$$$codecImpl extends CodecDataObject implements Foo {
  *         private static final AtomicRefereceFieldUpdater<Foo$$$codecImpl, Object> getBar$$$A;
- *         private static final NodeContextSupplier getBar$$$C;
  *         private volatile Object getBar;
  *
  *         public Foo$$$codecImpl(NormalizedNodeContainer data) {
@@ -94,7 +95,7 @@ import org.slf4j.LoggerFactory;
  *         }
  *
  *         public Bar getBar() {
- *             return (Bar) codecMember(getBar$$$A, getBar$$$C);
+ *             return (Bar) codecMember(getBar$$$A, "bar");
  *         }
  *     }
  *
@@ -104,20 +105,25 @@ import org.slf4j.LoggerFactory; * single place in a maintainable form. The glue code is extremely light (~6 instructions), which is beneficial on both * sides of invocation: * - generated method can readily be inlined into the caller - * - it forms a call site into which codeMember() can be inlined with both AtomicReferenceFieldUpdater and - * NodeContextSupplier being constant + * - it forms a call site into which codeMember() can be inlined with AtomicReferenceFieldUpdater being constant * *

* The second point is important here, as it allows the invocation logic around AtomicRefereceFieldUpdater to completely - * disappear, becoming synonymous with operations of a volatile field. NodeContextSupplier being constant also means - * it will resolve to one of its two implementations, allowing NodeContextSupplier.get() to be resolved to a constant - * (pointing to the supplier itself) or to a simple volatile read (which will be non-null after first access). + * disappear, becoming synonymous with operations of a volatile field. * *

- * The sticky point here is the NodeContextSupplier, as it is a heap object which cannot normally be looked up from the - * static context in which the static class initializer operates -- so we need perform some sort of a trick here. + * Furthermore there are distinct {@code codecMember} methods, each of which supports a different invocation style: + *

    + *
  • with {@code String}, which ends up looking up a {@link ValueNodeCodecContext}
    • + *
  • with {@code Class}, which ends up looking up a {@link DataContainerCodecContext}
    • + *
  • with {@code NodeContextSupplier}, which performs a direct load
    • + *
+ * The third mode of operation requires that the object being implemented is not defined in a {@code grouping}, because + * it welds the object to a particular namespace -- hence it trades namespace mobility for access speed. * *

+ * The sticky point here is the NodeContextSupplier, as it is a heap object which cannot normally be looked up from the + * static context in which the static class initializer operates -- so we need perform some sort of a trick here. * Eventhough ByteBuddy provides facilities for bridging references to type fields, those facilities operate on volatile * fields -- hence they do not quite work for us. * @@ -147,23 +153,121 @@ import org.slf4j.LoggerFactory; * This strategy works due to close cooperation with the target ClassLoader, as the entire code generation and loading * block runs with the class loading lock for this FQCN and the reference is not leaked until the process completes. */ -final class CodecDataObjectGenerator> implements ClassGenerator { +abstract class CodecDataObjectGenerator> implements ClassGenerator { + // Not reusable defintion: we can inline NodeContextSuppliers without a problem + static final class Fixed> extends CodecDataObjectGenerator { + private final ImmutableMap properties; + + private Fixed(final Builder template, final ImmutableMap properties, + final @Nullable Method keyMethod) { + super(template, keyMethod); + this.properties = requireNonNull(properties); + } + + @Override + Builder generateGetters(final Builder builder) { + Builder tmp = builder; + for (Method method : properties.keySet()) { + LOG.trace("Generating for fixed method {}", method); + final String methodName = method.getName(); + final TypeDescription retType = TypeDescription.ForLoadedType.of(method.getReturnType()); + tmp = tmp.defineMethod(methodName, retType, PUB_FINAL).intercept( + new SupplierGetterMethodImplementation(methodName, retType)); + } + return tmp; + } + + @Override + ArrayList getterMethods() { + return new ArrayList<>(properties.keySet()); + } + + @Override + public Class customizeLoading(final @NonNull Supplier> loader) { + final Fixed prev = CodecDataObjectBridge.setup(this); + try { + final Class result = loader.get(); + + /* + * This a bit of magic to support NodeContextSupplier constants. These constants need to be resolved + * while we have the information needed to find them -- that information is being held in this instance + * and we leak it to a thread-local variable held by CodecDataObjectBridge. + * + * By default the JVM will defer class initialization to first use, which unfortunately is too late for + * us, and hence we need to force class to initialize. + */ + try { + Class.forName(result.getName(), true, result.getClassLoader()); + } catch (ClassNotFoundException e) { + throw new LinkageError("Failed to find newly-defined " + result, e); + } + + return result; + } finally { + CodecDataObjectBridge.tearDown(prev); + } + } + + @NonNull NodeContextSupplier resolve(final @NonNull String methodName) { + final Optional> found = properties.entrySet().stream() + .filter(entry -> methodName.equals(entry.getKey().getName())).findAny(); + verify(found.isPresent(), "Failed to find property for %s in %s", methodName, this); + return verifyNotNull(found.get().getValue()); + } + } + + // Reusable definition: we have to rely on context lookups + private static final class Reusable> extends CodecDataObjectGenerator { + private final ImmutableMap simpleProperties; + private final Map> daoProperties; + + private Reusable(final Builder template, + final ImmutableMap simpleProperties, + final Map> daoProperties, final @Nullable Method keyMethod) { + super(template, keyMethod); + this.simpleProperties = requireNonNull(simpleProperties); + this.daoProperties = requireNonNull(daoProperties); + } + + @Override + Builder generateGetters(final Builder builder) { + Builder tmp = builder; + for (Entry entry : simpleProperties.entrySet()) { + final Method method = entry.getKey(); + LOG.trace("Generating for simple method {}", method); + final String methodName = method.getName(); + final TypeDescription retType = TypeDescription.ForLoadedType.of(method.getReturnType()); + tmp = tmp.defineMethod(methodName, retType, PUB_FINAL).intercept( + new SimpleGetterMethodImplementation(methodName, retType, + entry.getValue().getSchema().getQName().getLocalName())); + } + for (Entry> entry : daoProperties.entrySet()) { + final Method method = entry.getKey(); + LOG.trace("Generating for structured method {}", method); + final String methodName = method.getName(); + final TypeDescription retType = TypeDescription.ForLoadedType.of(method.getReturnType()); + tmp = tmp.defineMethod(methodName, retType, PUB_FINAL).intercept( + new StructuredGetterMethodImplementation(methodName, retType, entry.getValue())); + } + + return tmp; + } + + @Override + ArrayList getterMethods() { + final ArrayList ret = new ArrayList<>(simpleProperties.size() + daoProperties.size()); + ret.addAll(simpleProperties.keySet()); + ret.addAll(daoProperties.keySet()); + return ret; + } + } + 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_HELPER = TypeDefinition.Sort.describe(ToStringHelper.class); private static final Generic BB_INT = TypeDefinition.Sort.describe(int.class); - private static final Generic BB_IIC = TypeDefinition.Sort.describe(IdentifiableItemCodec.class); - private static final Generic BB_NCS = TypeDefinition.Sort.describe(NodeContextSupplier.class); - - private static final StackManipulation BRIDGE_RESOLVE = invokeMethod(CodecDataObjectBridge.class, - "resolve", String.class); - private static final StackManipulation BRIDGE_RESOLVE_KEY = invokeMethod(CodecDataObjectBridge.class, - "resolveKey", String.class); - private static final StackManipulation CODEC_MEMBER = invokeMethod(CodecDataObject.class, - "codecMember", AtomicReferenceFieldUpdater.class, NodeContextSupplier.class); - private static final StackManipulation CODEC_MEMBER_KEY = invokeMethod(CodecDataObject.class, - "codecMember", AtomicReferenceFieldUpdater.class, IdentifiableItemCodec.class); + private static final Comparator METHOD_BY_ALPHABET = Comparator.comparing(Method::getName); private static final StackManipulation ARRAYS_EQUALS = invokeMethod(Arrays.class, "equals", byte[].class, byte[].class); @@ -186,63 +290,54 @@ final class CodecDataObjectGenerator> implements Cl ACDO = bb.subclass(AugmentableCodecDataObject.class).visit(ByteBuddyUtils.computeFrames()); } - private final ImmutableMap properties; - private final Entry keyMethod; private final Builder template; + private final Method keyMethod; - private CodecDataObjectGenerator(final Builder template, - final ImmutableMap properties, - final @Nullable Entry keyMethod) { + private CodecDataObjectGenerator(final Builder template, final @Nullable Method keyMethod) { this.template = requireNonNull(template); - this.properties = requireNonNull(properties); this.keyMethod = keyMethod; } static > Class generate(final CodecClassLoader loader, - final Class bindingInterface, final ImmutableMap properties, - final Entry keyMethod) { + final Class bindingInterface, final ImmutableMap simpleProperties, + final Map> daoProperties, final Method keyMethod) { return loader.generateClass(bindingInterface, "codecImpl", - new CodecDataObjectGenerator<>(CDO, properties, keyMethod)); + new Reusable<>(CDO, simpleProperties, daoProperties, keyMethod)); } static > Class generateAugmentable( final CodecClassLoader loader, final Class bindingInterface, - final ImmutableMap properties, - final Entry keyMethod) { + final ImmutableMap simpleProperties, + final Map> daoProperties, final Method keyMethod) { return loader.generateClass(bindingInterface, "codecImpl", - new CodecDataObjectGenerator<>(ACDO, properties, keyMethod)); + new Reusable<>(ACDO, simpleProperties, daoProperties, keyMethod)); } @Override - public GeneratorResult generateClass(final CodecClassLoader loeader, final String fqcn, + public final GeneratorResult generateClass(final CodecClassLoader loeader, final String fqcn, final Class bindingInterface) { LOG.trace("Generating class {}", fqcn); @SuppressWarnings("unchecked") Builder builder = (Builder) template.name(fqcn).implement(bindingInterface); - for (Method method : properties.keySet()) { - LOG.trace("Generating for method {}", method); - final String methodName = method.getName(); - final TypeDescription retType = TypeDescription.ForLoadedType.of(method.getReturnType()); - builder = builder.defineMethod(methodName, retType, PUB_FINAL) - .intercept(new MethodImplementation(BB_NCS, BRIDGE_RESOLVE, CODEC_MEMBER, methodName, retType)); - } + builder = generateGetters(builder); if (keyMethod != null) { LOG.trace("Generating for key {}", keyMethod); - final Method method = keyMethod.getKey(); - final String methodName = method.getName(); - final TypeDescription retType = TypeDescription.ForLoadedType.of(method.getReturnType()); - builder = builder.defineMethod(methodName, retType, PUB_FINAL) - .intercept(new MethodImplementation(BB_IIC, BRIDGE_RESOLVE_KEY, CODEC_MEMBER_KEY, methodName, - retType)); + final String methodName = keyMethod.getName(); + final TypeDescription retType = TypeDescription.ForLoadedType.of(keyMethod.getReturnType()); + builder = builder.defineMethod(methodName, retType, PUB_FINAL).intercept( + 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 ImmutableMap methods = Maps.uniqueIndex(properties.keySet(), + final ArrayList properties = getterMethods(); + // Make sure properties are alpha-sorted + properties.sort(METHOD_BY_ALPHABET); + final ImmutableMap methods = Maps.uniqueIndex(properties, ByteBuddyUtils::invokeMethod); // Final bits: @@ -260,43 +355,9 @@ final class CodecDataObjectGenerator> implements Cl .make()); } - @Override - public Class customizeLoading(final @NonNull Supplier> loader) { - final CodecDataObjectGenerator prev = CodecDataObjectBridge.setup(this); - try { - final Class result = loader.get(); - - /* - * This a bit of magic to support NodeContextSupplier constants. These constants need to be resolved while - * we have the information needed to find them -- that information is being held in this instance and we - * leak it to a thread-local variable held by CodecDataObjectBridge. - * - * By default the JVM will defer class initialization to first use, which unfortunately is too late for - * us, and hence we need to force class to initialize. - */ - try { - Class.forName(result.getName(), true, result.getClassLoader()); - } catch (ClassNotFoundException e) { - throw new LinkageError("Failed to find newly-defined " + result, e); - } - - return result; - } finally { - CodecDataObjectBridge.tearDown(prev); - } - } - - @NonNull NodeContextSupplier resolve(final @NonNull String methodName) { - final Optional> found = properties.entrySet().stream() - .filter(entry -> methodName.equals(entry.getKey().getName())).findAny(); - verify(found.isPresent(), "Failed to find property for %s in %s", methodName, this); - return verifyNotNull(found.get().getValue()); - } + abstract Builder generateGetters(Builder builder); - @NonNull IdentifiableItemCodec resolveKey(final @NonNull String methodName) { - return verifyNotNull(verifyNotNull(keyMethod, "No key method attached for %s in %s", methodName, this) - .getValue()); - } + abstract ArrayList getterMethods(); private static Implementation codecEquals(final ImmutableMap properties) { // Label for 'return false;' @@ -345,38 +406,27 @@ final class CodecDataObjectGenerator> implements Cl return new Implementation.Simple(manipulations.toArray(new StackManipulation[0])); } - private static final class MethodImplementation implements Implementation { + private abstract static class AbstractMethodImplementation implements Implementation { private static final Generic BB_ARFU = TypeDefinition.Sort.describe(AtomicReferenceFieldUpdater.class); private static final Generic BB_OBJECT = TypeDefinition.Sort.describe(Object.class); private static final StackManipulation OBJECT_CLASS = ClassConstant.of(TypeDescription.OBJECT); private static final StackManipulation ARFU_NEWUPDATER = invokeMethod(AtomicReferenceFieldUpdater.class, "newUpdater", Class.class, Class.class, String.class); - private static final int PRIV_CONST = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL + static final int PRIV_CONST = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL | Opcodes.ACC_SYNTHETIC; private static final int PRIV_VOLATILE = Opcodes.ACC_PRIVATE | Opcodes.ACC_VOLATILE | Opcodes.ACC_SYNTHETIC; - private final Generic contextType; - private final StackManipulation resolveMethod; - private final StackManipulation codecMember; - private final TypeDescription retType; - + final TypeDescription retType; // getFoo - private final String methodName; + final String methodName; // getFoo$$$A - private final String arfuName; - // getFoo$$$C - private final String contextName; + final String arfuName; - MethodImplementation(final Generic contextType, final StackManipulation resolveMethod, - final StackManipulation codecMember, final String methodName, final TypeDescription retType) { - this.contextType = requireNonNull(contextType); - this.resolveMethod = requireNonNull(resolveMethod); - this.codecMember = requireNonNull(codecMember); + AbstractMethodImplementation(final String methodName, final TypeDescription retType) { this.methodName = requireNonNull(methodName); this.retType = requireNonNull(retType); this.arfuName = methodName + "$$$A"; - this.contextName = methodName + "$$$C"; } @Override @@ -384,26 +434,118 @@ final class CodecDataObjectGenerator> implements Cl final InstrumentedType tmp = instrumentedType // private static final AtomicReferenceFieldUpdater getFoo$$$A; .withField(new FieldDescription.Token(arfuName, PRIV_CONST, BB_ARFU)) - // private static final getFoo$$$C; - .withField(new FieldDescription.Token(contextName, PRIV_CONST, contextType)) // private volatile Object getFoo; .withField(new FieldDescription.Token(methodName, PRIV_VOLATILE, BB_OBJECT)); - // "getFoo" - final TextConstant methodNameText = new TextConstant(methodName); - - return tmp - .withInitializer(new ByteCodeAppender.Simple( - // getFoo$$$A = AtomicReferenceFieldUpdater.newUpdater(This.class, Object.class, "getFoo"); - ClassConstant.of(tmp), - OBJECT_CLASS, - methodNameText, - ARFU_NEWUPDATER, - putField(tmp, arfuName), - // getFoo$$$C = CodecDataObjectBridge.("getFoo"); - methodNameText, - resolveMethod, - putField(tmp, contextName))); + return tmp.withInitializer(new ByteCodeAppender.Simple( + // getFoo$$$A = AtomicReferenceFieldUpdater.newUpdater(This.class, Object.class, "getFoo"); + ClassConstant.of(tmp), + OBJECT_CLASS, + new TextConstant(methodName), + ARFU_NEWUPDATER, + putField(tmp, arfuName))); + } + } + + private static final class KeyMethodImplementation extends AbstractMethodImplementation { + private static final StackManipulation CODEC_KEY = invokeMethod(CodecDataObject.class, + "codecKey", AtomicReferenceFieldUpdater.class); + + KeyMethodImplementation(final String methodName, final TypeDescription retType) { + super(methodName, retType); + } + + @Override + public ByteCodeAppender appender(final Target implementationTarget) { + final TypeDescription instrumentedType = implementationTarget.getInstrumentedType(); + return new ByteCodeAppender.Simple( + // return (FooType) codecKey(getFoo$$$A); + THIS, + getField(instrumentedType, arfuName), + CODEC_KEY, + TypeCasting.to(retType), + MethodReturn.REFERENCE); + } + } + + private static final class SimpleGetterMethodImplementation extends AbstractMethodImplementation { + private static final StackManipulation CODEC_MEMBER = invokeMethod(CodecDataObject.class, + "codecMember", AtomicReferenceFieldUpdater.class, String.class); + + private final String localName; + + SimpleGetterMethodImplementation(final String methodName, final TypeDescription retType, + final String localName) { + super(methodName, retType); + this.localName = requireNonNull(localName); + } + + @Override + public ByteCodeAppender appender(final Target implementationTarget) { + final TypeDescription instrumentedType = implementationTarget.getInstrumentedType(); + return new ByteCodeAppender.Simple( + // return (FooType) codecMember(getFoo$$$A, "foo"); + THIS, + getField(instrumentedType, arfuName), + new TextConstant(localName), + CODEC_MEMBER, + TypeCasting.to(retType), + MethodReturn.REFERENCE); + } + } + + private static final class StructuredGetterMethodImplementation extends AbstractMethodImplementation { + private static final StackManipulation CODEC_MEMBER = invokeMethod(CodecDataObject.class, + "codecMember", AtomicReferenceFieldUpdater.class, Class.class); + + private final Class bindingClass; + + StructuredGetterMethodImplementation(final String methodName, final TypeDescription retType, + final Class bindingClass) { + super(methodName, retType); + this.bindingClass = requireNonNull(bindingClass); + } + + @Override + public ByteCodeAppender appender(final Target implementationTarget) { + final TypeDescription instrumentedType = implementationTarget.getInstrumentedType(); + return new ByteCodeAppender.Simple( + // return (FooType) codecMember(getFoo$$$A, FooType.class); + THIS, + getField(instrumentedType, arfuName), + ClassConstant.of(TypeDefinition.Sort.describe(bindingClass).asErasure()), + CODEC_MEMBER, + TypeCasting.to(retType), + MethodReturn.REFERENCE); + } + } + + private static final class SupplierGetterMethodImplementation extends AbstractMethodImplementation { + private static final StackManipulation CODEC_MEMBER = invokeMethod(CodecDataObject.class, + "codecMember", AtomicReferenceFieldUpdater.class, NodeContextSupplier.class); + private static final StackManipulation BRIDGE_RESOLVE = invokeMethod(CodecDataObjectBridge.class, + "resolve", String.class); + private static final Generic BB_NCS = TypeDefinition.Sort.describe(NodeContextSupplier.class); + + // getFoo$$$C + private final String contextName; + + SupplierGetterMethodImplementation(final String methodName, final TypeDescription retType) { + super(methodName, retType); + contextName = methodName + "$$$C"; + } + + @Override + public InstrumentedType prepare(final InstrumentedType instrumentedType) { + final InstrumentedType tmp = super.prepare(instrumentedType) + // private static final NodeContextSupplier getFoo$$$C; + .withField(new FieldDescription.Token(contextName, PRIV_CONST, BB_NCS)); + + return tmp.withInitializer(new ByteCodeAppender.Simple( + // getFoo$$$C = CodecDataObjectBridge.resolve("getFoo"); + new TextConstant(methodName), + BRIDGE_RESOLVE, + putField(tmp, contextName))); } @Override @@ -414,7 +556,7 @@ final class CodecDataObjectGenerator> implements Cl THIS, getField(instrumentedType, arfuName), getField(instrumentedType, contextName), - codecMember, + CODEC_MEMBER, TypeCasting.to(retType), MethodReturn.REFERENCE); } 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 167667c8b4..e49b5d4e1d 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 @@ -9,7 +9,6 @@ package org.opendaylight.mdsal.binding.dom.codec.impl; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Verify.verify; -import static com.google.common.base.Verify.verifyNotNull; import static java.util.Objects.requireNonNull; import com.google.common.annotations.Beta; @@ -20,8 +19,6 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.lang.reflect.Method; -import java.util.Arrays; -import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -82,7 +79,6 @@ public abstract class DataObjectCodecContext METHOD_BY_ALPHABET = Comparator.comparing(Method::getName); private static final Augmentations EMPTY_AUGMENTATIONS = new Augmentations(ImmutableMap.of(), ImmutableMap.of()); private final ImmutableMap leafChild; @@ -106,17 +102,15 @@ public abstract class DataObjectCodecContext prototype, - final Entry keyMethod) { + DataObjectCodecContext(final DataContainerCodecPrototype prototype, final Method keyMethod) { super(prototype); final Class bindingClass = getBindingClass(); - final Map tmpLeaves = factory().getLeafNodes(bindingClass, getSchema()); + final ImmutableMap tmpLeaves = factory().getLeafNodes(bindingClass, getSchema()); final Map, Method> clsToMethod = BindingReflections.getChildrenClassToMethod(bindingClass); final Map byYangBuilder = new HashMap<>(); - final Map tmpMethodToSupplier = new HashMap<>(); final Map, DataContainerCodecPrototype> byStreamClassBuilder = new HashMap<>(); final Map, DataContainerCodecPrototype> byBindingArgClassBuilder = new HashMap<>(); @@ -125,12 +119,12 @@ public abstract class DataObjectCodecContext entry : tmpLeaves.entrySet()) { final ValueNodeCodecContext leaf = entry.getValue(); - tmpMethodToSupplier.put(entry.getKey(), leaf); leafChildBuilder.put(leaf.getSchema().getQName().getLocalName(), leaf); byYangBuilder.put(leaf.getDomPathArgument(), leaf); } this.leafChild = leafChildBuilder.build(); + final Map> tmpDataObjects = new HashMap<>(); for (final Entry, Method> childDataObj : clsToMethod.entrySet()) { final Method method = childDataObj.getValue(); verify(!method.isDefault(), "Unexpected default method %s in %s", method, bindingClass); @@ -142,7 +136,7 @@ public abstract class DataObjectCodecContext childProto = loadChildPrototype(retClass); - tmpMethodToSupplier.put(method, childProto); + tmpDataObjects.put(method, childProto.getBindingClass()); byStreamClassBuilder.put(childProto.getBindingClass(), childProto); byYangBuilder.put(childProto.getYangArg(), childProto); if (childProto.isChoice()) { @@ -153,15 +147,6 @@ public abstract class DataObjectCodecContext propBuilder = ImmutableMap.builderWithExpectedSize( - properties.length); - for (Method prop : properties) { - propBuilder.put(prop, verifyNotNull(tmpMethodToSupplier.get(prop))); - } - this.byYang = ImmutableMap.copyOf(byYangBuilder); this.byStreamClass = ImmutableMap.copyOf(byStreamClassBuilder); @@ -176,11 +161,11 @@ public abstract class DataObjectCodecContext> { +abstract class IdentifiableItemCodec implements Codec> { private static final class SingleKey extends IdentifiableItemCodec { private static final MethodType CTOR_TYPE = MethodType.methodType(Identifier.class, Object.class); @@ -151,14 +149,7 @@ public abstract class IdentifiableItemCodec implements Codec deserialize(final NodeIdentifierWithPredicates input) { - final Identifier identifier; - try { - identifier = deserializeIdentifier(input.getKeyValues()); - } catch (Throwable e) { - Throwables.throwIfUnchecked(e); - throw new IllegalStateException("Failed to deserialize " + input, e); - } - + final Identifier identifier = deserializeIdentifier(input); return IdentifiableItem.of((Class) identifiable, (Identifier) identifier); } @@ -167,10 +158,20 @@ public abstract class IdentifiableItemCodec implements Codec deserializeIdentifier(final NodeIdentifierWithPredicates input) { + try { + return deserializeIdentifier(input.getKeyValues()); + } catch (Throwable e) { + Throwables.throwIfUnchecked(e); + throw new IllegalStateException("Failed to deserialize " + input, e); + } + } + @SuppressWarnings("checkstyle:illegalThrows") - abstract Identifier deserializeIdentifier(Map keyValues) throws Throwable; + abstract @NonNull Identifier deserializeIdentifier(Map keyValues) throws Throwable; - abstract NodeIdentifierWithPredicates serializeIdentifier(QName qname, Identifier key); + abstract @NonNull NodeIdentifierWithPredicates serializeIdentifier(QName qname, Identifier key); static MethodHandle getConstructor(final Class> clazz) { for (@SuppressWarnings("rawtypes") final Constructor constr : clazz.getConstructors()) { 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 166cad8b44..62dd9b74c1 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 @@ -11,8 +11,8 @@ import static java.util.Objects.requireNonNull; import static org.opendaylight.mdsal.binding.spec.naming.BindingMapping.IDENTIFIABLE_KEY_NAME; import java.lang.reflect.Method; -import java.util.AbstractMap.SimpleImmutableEntry; import java.util.List; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.Identifiable; import org.opendaylight.yangtools.yang.binding.Identifier; @@ -27,7 +27,7 @@ final class KeyedListNodeCodecContext> ex private KeyedListNodeCodecContext(final DataContainerCodecPrototype prototype, final Method keyMethod, final IdentifiableItemCodec codec) { - super(prototype, new SimpleImmutableEntry<>(keyMethod, codec)); + super(prototype, keyMethod); this.codec = requireNonNull(codec); } @@ -79,6 +79,10 @@ final class KeyedListNodeCodecContext> ex return codec.serialize(IdentifiableItem.of((Class)getBindingClass(), (Identifier)key)); } + @NonNull Identifier deserialize(final NodeIdentifierWithPredicates arg) { + return codec.deserializeIdentifier(arg); + } + @Override public YangInstanceIdentifier.PathArgument serializePathArgument(final InstanceIdentifier.PathArgument arg) { if (arg instanceof IdentifiableItem) { 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 a3755a31b8..3675e79e15 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 @@ -10,7 +10,6 @@ package org.opendaylight.mdsal.binding.dom.codec.impl; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; -import java.util.Map.Entry; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.MapNode; @@ -25,7 +24,7 @@ class ListNodeCodecContext extends DataObjectCodecContext< } ListNodeCodecContext(final DataContainerCodecPrototype prototype, - final Entry keyMethod) { + final Method keyMethod) { super(prototype, keyMethod); } diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/test/KeyInheritenceTest.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/test/KeyInheritenceTest.java new file mode 100644 index 0000000000..542d79b567 --- /dev/null +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/test/KeyInheritenceTest.java @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2019 PANTHEON.tech, s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.mdsal.binding.dom.codec.test; + +import static org.junit.Assert.assertEquals; + +import com.google.common.collect.ImmutableList; +import java.util.Map.Entry; +import org.junit.Test; +import org.opendaylight.yang.gen.v1.mdsal442.keydef.norev.Def; +import org.opendaylight.yang.gen.v1.mdsal442.keydef.norev.DefBuilder; +import org.opendaylight.yang.gen.v1.mdsal442.keydef.norev.grp.LstBuilder; +import org.opendaylight.yang.gen.v1.mdsal442.keydef.norev.grp.LstKey; +import org.opendaylight.yang.gen.v1.mdsal442.keyuse.norev.Use; +import org.opendaylight.yang.gen.v1.mdsal442.keyuse.norev.UseBuilder; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; + +public class KeyInheritenceTest extends AbstractBindingCodecTest { + private static final LstKey KEY = new LstKey("foo"); + private static final InstanceIdentifier DEF_IID = InstanceIdentifier.create(Def.class); + private static final InstanceIdentifier USE_IID = InstanceIdentifier.create(Use.class); + + private static final Def DEF = new DefBuilder() + .setLst(ImmutableList.of(new LstBuilder().setFoo("foo").withKey(KEY).build())) + .build(); + private static final Use USE = new UseBuilder() + .setLst(ImmutableList.of(new LstBuilder().setFoo("foo").withKey(KEY).build())) + .build(); + + @Test + public void testFromBinding() { + final Entry> domDef = registry.toNormalizedNode(DEF_IID, DEF); + Entry, DataObject> entry = registry.fromNormalizedNode(domDef.getKey(), + domDef.getValue()); + assertEquals(DEF_IID, entry.getKey()); + final Def codecDef = (Def) entry.getValue(); + + final Entry> domUse = registry.toNormalizedNode(USE_IID, USE); + entry = registry.fromNormalizedNode(domUse.getKey(), domUse.getValue()); + assertEquals(USE_IID, entry.getKey()); + final Use codecUse = (Use) entry.getValue(); + + Use copiedUse = new UseBuilder(DEF).build(); + assertEquals(USE, copiedUse); + assertEquals(domUse.getValue(), registry.toNormalizedNode(USE_IID, copiedUse).getValue()); + copiedUse = new UseBuilder(codecDef).build(); + assertEquals(USE, copiedUse); + assertEquals(domUse.getValue(), registry.toNormalizedNode(USE_IID, copiedUse).getValue()); + copiedUse = new UseBuilder(codecUse).build(); + assertEquals(USE, copiedUse); + assertEquals(domUse.getValue(), registry.toNormalizedNode(USE_IID, copiedUse).getValue()); + } +} diff --git a/binding/mdsal-binding-test-model/src/main/yang/mdsal442-keydef.yang b/binding/mdsal-binding-test-model/src/main/yang/mdsal442-keydef.yang new file mode 100644 index 0000000000..d107153e1c --- /dev/null +++ b/binding/mdsal-binding-test-model/src/main/yang/mdsal442-keydef.yang @@ -0,0 +1,18 @@ +module mdsal442-keydef { + namespace mdsal442-keydef; + prefix keydef; + + grouping grp { + list lst { + key foo; + leaf foo { + type string; + } + } + } + + container def { + uses grp; + } +} + diff --git a/binding/mdsal-binding-test-model/src/main/yang/mdsal442-keyuse.yang b/binding/mdsal-binding-test-model/src/main/yang/mdsal442-keyuse.yang new file mode 100644 index 0000000000..d76e6b2c05 --- /dev/null +++ b/binding/mdsal-binding-test-model/src/main/yang/mdsal442-keyuse.yang @@ -0,0 +1,13 @@ +module mdsal442-keyuse { + namespace mdsal442-keyuse; + prefix keyuse; + + import mdsal442-keydef { + prefix keydef; + } + + container use { + uses keydef:grp; + } +} +