package org.opendaylight.mdsal.binding.dom.codec.impl;
import com.google.common.collect.Iterables;
-import javax.annotation.concurrent.GuardedBy;
-import org.opendaylight.mdsal.binding.dom.codec.api.BindingCodecTreeNode.ChildAddressabilitySummary;
+import org.checkerframework.checker.lock.qual.Holding;
+import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.mdsal.binding.dom.codec.api.BindingDataObjectCodecTreeNode.ChildAddressabilitySummary;
import org.opendaylight.mdsal.binding.dom.codec.impl.NodeCodecContext.CodecContextFactory;
import org.opendaylight.yangtools.yang.binding.DataObject;
import org.opendaylight.yangtools.yang.binding.DataRoot;
private final T schema;
private final QNameModule namespace;
private final CodecContextFactory factory;
- private final Class<?> bindingClass;
private final Item<?> bindingArg;
private final PathArgument yangArg;
private final ChildAddressabilitySummary childAddressabilitySummary;
- private volatile DataContainerCodecContext<?, T> instance = null;
+ /*
+ * FIXME: This field is now utterly in the hot path of CodecDataObject's instantiation of data. It is volatile
+ * support double-checked locking, as detailed in
+ * https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java.
+ *
+ * Removing volatile here would seem to be worthwhile, as it would mean that in the initialized case we end
+ * up with no happens-before synchronization between threads, allowing JIT to perform better.
+ *
+ * Since we are on Java 5+, DataContainerCodecContext and its subclasses play a role here. All of their
+ * fields are either final or volatile, so intuition would indicate they themselves could (already?) be
+ * acting in the same capacity as "FinalWrapper" in the above example does -- in which case we can safely
+ * remove the volatile access altogether, after documenting this fact there (and potentially placing some
+ * guards).
+ *
+ * The presence of volatile fields seems to violate this, as
+ * http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html mentions "such that all of the
+ * fields of Helper are final".
+ *
+ * There are only two violations, both relating to dynamic discovery of augmentations, which should be
+ * eliminated once we have constant class loader visibility (and thus can discover all valid augmentations
+ * and aliases). Alternatively, adding an indirection could be considered after the effects are fully
+ * understood.
+ *
+ * Alternatively we can consider the use of VarHandle acquire/release mechanics to remove this instance
+ * from global synchronization order -- that would be a Java 9+ concern, though.
+ *
+ * Finally, the benefits of addressing this are masked by CodecDataObject operating on volatiles, hence
+ * already doing a barrier very close to us -- thus reducing the scope in which code can be reordered.
+ * If CodecDataObject moves to VarHandles and uses acquire/release there, we will be put on the spot here,
+ * though.
+ */
+ private volatile DataContainerCodecContext<?, T> instance;
@SuppressWarnings("unchecked")
private DataContainerCodecPrototype(final Class<?> cls, final PathArgument arg, final T nodeSchema,
final CodecContextFactory factory) {
- this.bindingClass = cls;
+ this(Item.of((Class<? extends DataObject>) cls), arg, nodeSchema, factory);
+ }
+
+ private DataContainerCodecPrototype(final Item<?> bindingArg, final PathArgument arg, final T nodeSchema,
+ final CodecContextFactory factory) {
+ this.bindingArg = bindingArg;
this.yangArg = arg;
this.schema = nodeSchema;
this.factory = factory;
- this.bindingArg = Item.of((Class<? extends DataObject>) bindingClass);
if (arg instanceof AugmentationIdentifier) {
this.namespace = Iterables.getFirst(((AugmentationIdentifier) arg).getPossibleChildNames(), null)
return new DataContainerCodecPrototype(cls, NodeIdentifier.create(schema.getQName()), schema, factory);
}
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ static <T extends DataSchemaNode> DataContainerCodecPrototype<T> from(final Item<?> bindingArg, final T schema,
+ final CodecContextFactory factory) {
+ return new DataContainerCodecPrototype(bindingArg, NodeIdentifier.create(schema.getQName()), schema, factory);
+ }
+
@SuppressWarnings({ "rawtypes", "unchecked" })
static DataContainerCodecPrototype<?> from(final Class<?> augClass, final AugmentationIdentifier arg,
final AugmentationSchemaNode schema, final CodecContextFactory factory) {
}
protected Class<?> getBindingClass() {
- return bindingClass;
+ return bindingArg.getType();
}
protected Item<?> getBindingArg() {
}
@Override
- public DataContainerCodecContext<?,T> get() {
- DataContainerCodecContext<?,T> tmp = instance;
+ public DataContainerCodecContext<?, T> get() {
+ DataContainerCodecContext<?, T> tmp = instance;
if (tmp == null) {
synchronized (this) {
tmp = instance;
if (tmp == null) {
- tmp = createInstance();
- instance = tmp;
+ instance = tmp = createInstance();
}
}
}
return tmp;
}
- @GuardedBy("this")
+ @Holding("this")
@SuppressWarnings({ "rawtypes", "unchecked" })
- private DataContainerCodecContext<?,T> createInstance() {
+ private @NonNull DataContainerCodecContext<?, T> createInstance() {
// FIXME: make protected abstract
if (schema instanceof ContainerSchemaNode) {
return new ContainerNodeCodecContext(this);
} else if (schema instanceof ListSchemaNode) {
- return Identifiable.class.isAssignableFrom(getBindingClass()) ? new KeyedListNodeCodecContext(this)
- : new ListNodeCodecContext(this);
+ return Identifiable.class.isAssignableFrom(getBindingClass())
+ ? KeyedListNodeCodecContext.create((DataContainerCodecPrototype<ListSchemaNode>) this)
+ : new ListNodeCodecContext(this);
} else if (schema instanceof ChoiceSchemaNode) {
return new ChoiceNodeCodecContext(this);
} else if (schema instanceof AugmentationSchemaNode) {
} else if (schema instanceof CaseSchemaNode) {
return new CaseNodeCodecContext(this);
}
- throw new IllegalArgumentException("Unsupported type " + bindingClass + " " + schema);
+ throw new IllegalArgumentException("Unsupported type " + getBindingClass() + " " + schema);
}
boolean isChoice() {
Code Review / mdsal.git / blobdiff