Merge "BUG-2402: add FIXMEs and javadoc"

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java
index 669c4d05abda672472ed480d1692e2afac3295d4..cb50fefc8df70e7da24a5d83e48c1dec08774646 100644 (file)
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java
@@ -43,6 +43,17 @@ abstract class AbstractImmutableDataContainerNodeBuilder<I extends YangInstanceI
 
     protected AbstractImmutableDataContainerNodeBuilder(final AbstractImmutableDataContainerNode<I> node) {
         this.nodeIdentifier = node.getIdentifier();
+
+        /*
+         * FIXME: BUG-2402: this call is not what we actually want. We are the
+         *        only user of getChildren(), and we really want this to be a
+         *        zero-copy operation if we happen to not modify the children.
+         *        If we do, we want to perform an efficient copy-on-write before
+         *        we make the change.
+         *
+         *        With this interface we end up creating a lot of short-lived
+         *        objects in case we modify the map -- see checkDirty().
+         */
         this.value = node.getChildren();
         this.dirty = true;
     }
@@ -62,6 +73,13 @@ abstract class AbstractImmutableDataContainerNodeBuilder<I extends YangInstanceI
 
     private void checkDirty() {
         if (dirty) {
+            /*
+             * FIXME: BUG-2402: This is the second part of the above. Note
+             *        that value here is usually a read-only view. Invocation
+             *        of this constructor will force instantiation of a wrapper
+             *        Map.Entry object, just to make sure this read path does
+             *        not modify the map.
+             */
             value = new HashMap<>(value);
             dirty = false;
         }

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java
index a4b426885827003dfb9f3fdbb79db6b9b9c54013..b6ac22617abcb44feb63367c750a764c5ef9a23c 100644 (file)
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java
@@ -39,6 +39,15 @@ public abstract class AbstractImmutableDataContainerNode<K extends PathArgument>
         return children.hashCode();
     }
 
+    /**
+     * DO NOT USE THIS METHOD.
+     *
+     * This is an implementation-internal API and no outside users should use it. If you do,
+     * you are asking for trouble, as the returned object is not guaranteed to conform to
+     * java.util.Map interface.
+     *
+     * @return An unmodifiable view if this node's children.
+     */
     public final Map<PathArgument, DataContainerChild<? extends PathArgument, ?>> getChildren() {
         return children;
     }