BUG-2402: introduce CloneableChildrenMap

This exposes the appropriate interface to create a copy of the
underlying map in the most efficient manner. Makes
AbstractImmutableDataContainerNodeBuilder check for presence of the
interface and use it to create an efficient copy.

Change-Id: I8ad8159076fa199f315857e339fc83363ce73289
Signed-off-by: Robert Varga <rovarga@cisco.com>

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 ec2f5ee7bdfbb48e316fa63b45c3ccfc1e24a67d..02e6a44443989208898c2e3ec311bd64769c47d4 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
@@ -17,6 +17,7 @@ import org.opendaylight.yangtools.yang.data.api.schema.DataContainerNode;
 import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeBuilder;
 import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.NormalizedNodeContainerBuilder;
 import org.opendaylight.yangtools.yang.data.impl.schema.nodes.AbstractImmutableDataContainerNode;
+import org.opendaylight.yangtools.yang.data.impl.schema.nodes.CloneableChildrenMap;
 
 abstract class AbstractImmutableDataContainerNodeBuilder<I extends YangInstanceIdentifier.PathArgument, R extends DataContainerNode<I>> implements DataContainerNodeBuilder<I, R> {
     private static final int DEFAULT_CAPACITY = 4;
@@ -45,14 +46,11 @@ abstract class AbstractImmutableDataContainerNodeBuilder<I extends YangInstanceI
         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 quite awkward. What we actually want to be saying here is: give me
+         * a copy-on-write view of your children. The API involved in that could be
+         * a bit hairy, so we do the next best thing and rely on the fact that the
+         * returned object implements a specific interface, which leaks the functionality
+         * we need.
          */
         this.value = node.getChildren();
         this.dirty = true;
@@ -73,14 +71,11 @@ 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);
+            if (value instanceof CloneableChildrenMap) {
+                value = ((CloneableChildrenMap) value).createMutableClone();
+            } else {
+                value = new HashMap<>(value);
+            }
             dirty = false;
         }
     }

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/CloneableChildrenMap.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/CloneableChildrenMap.java
new file mode 100644 (file)
index 0000000..1b24c4a
--- /dev/null
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/CloneableChildrenMap.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2015 Cisco Systems, Inc. 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.yangtools.yang.data.impl.schema.nodes;
+
+import com.google.common.annotations.Beta;
+import java.util.Map;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
+import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild;
+
+/**
+ * Interface implemented by maps which allow efficient duplication. This interface IS NOT part of the
+ * general API contract and is <strong>an internal implementation detail</strong>. It is subject to
+ * change and/or removal at any time.
+ */
+@Beta
+public abstract class CloneableChildrenMap implements Map<PathArgument, DataContainerChild<? extends PathArgument, ?>> {
+    CloneableChildrenMap() {
+        // Hidden to prevent outside instantiation
+    }
+
+    /**
+     * Create a clone of this map's contents. This does not include the actual
+     * keys and values, just the internal map state.
+     *
+     * @return An isolated, writable map.
+     */
+    public abstract Map<PathArgument, DataContainerChild<? extends PathArgument, ?>> createMutableClone();
+}

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/UnmodifiableChildrenMap.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/UnmodifiableChildrenMap.java
index 5037dcbfe8df53d60bcc6cc97da6faf6ff322eb8..16995208489397e0e353b2909943784501ae4699 100644 (file)
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/UnmodifiableChildrenMap.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/UnmodifiableChildrenMap.java
@@ -12,6 +12,7 @@ import com.google.common.collect.ImmutableMap;
 import java.io.Serializable;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
@@ -21,7 +22,7 @@ import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild;
  * Internal equivalent of {@link Collections}' unmodifiable Map. It does not retain
  * keySet/entrySet references, thus lowering the memory overhead.
  */
-final class UnmodifiableChildrenMap implements Map<PathArgument, DataContainerChild<? extends PathArgument, ?>>, Serializable {
+final class UnmodifiableChildrenMap extends CloneableChildrenMap implements Serializable {
     private static final long serialVersionUID = 1L;
     /*
      * Do not wrap maps which are smaller than this and instead copy them into
@@ -144,4 +145,14 @@ final class UnmodifiableChildrenMap implements Map<PathArgument, DataContainerCh
     public String toString() {
         return delegate.toString();
     }
+
+    @Override
+    @SuppressWarnings("unchecked")
+    public Map<PathArgument, DataContainerChild<? extends PathArgument, ?>> createMutableClone() {
+        if (delegate instanceof HashMap) {
+            return (Map<PathArgument, DataContainerChild<? extends PathArgument, ?>>) ((HashMap<?, ?>) delegate).clone();
+        }
+
+        return new HashMap<>(delegate);
+    }
 }