Do not expand schema tree values

When we are constructing a dataTree from a schemaTree, we do not want
to operate on the ImmutableMap result, as that results in the values
view being retained. This amounts to unnecessary overhead, as we
typically do not need this view and it should be materialized only
if it is needed.

JIRA: YANGTOOLS-652
Change-Id: I3d1d689366a48822324c442da184496d9c6d873b
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractDeclaredEffectiveStatement.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractDeclaredEffectiveStatement.java
index c6987477ee13efecda3a4e5aace2482298f1ac95..4ca71b8df160a8968c3e8920567c6790e3fdb67e 100644 (file)
--- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractDeclaredEffectiveStatement.java
+++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractDeclaredEffectiveStatement.java
@@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.Beta;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import java.util.Map;
 import java.util.Optional;
 import org.eclipse.jdt.annotation.NonNull;
@@ -175,14 +176,14 @@ public abstract class AbstractDeclaredEffectiveStatement<A, D extends DeclaredSt
      */
     public abstract static class DefaultWithSchemaTree<A, D extends DeclaredStatement<A>,
             E extends SchemaTreeAwareEffectiveStatement<A, D>> extends WithSchemaTree<A, D, E> {
-        private final @NonNull Map<QName, SchemaTreeEffectiveStatement<?>> schemaTree;
+        private final @NonNull ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTree;
         private final @NonNull D declared;
 
         protected DefaultWithSchemaTree(final D declared, final StmtContext<?, ?, ?> ctx,
                 final ImmutableList<? extends EffectiveStatement<?, ?>> substatements) {
             this.declared = requireNonNull(declared);
-            this.schemaTree = AbstractSchemaEffectiveDocumentedNode.createSchemaTreeNamespace(
-                ctx.getStatementSourceReference(), substatements);
+            this.schemaTree = ImmutableMap.copyOf(AbstractSchemaEffectiveDocumentedNode.createSchemaTreeNamespace(
+                ctx.getStatementSourceReference(), substatements));
         }
 
         @Override
@@ -206,16 +207,19 @@ public abstract class AbstractDeclaredEffectiveStatement<A, D extends DeclaredSt
      */
     public abstract static class DefaultWithDataTree<A, D extends DeclaredStatement<A>,
             E extends DataTreeAwareEffectiveStatement<A, D>> extends WithDataTree<A, D, E> {
-        private final @NonNull Map<QName, SchemaTreeEffectiveStatement<?>> schemaTree;
-        private final @NonNull Map<QName, DataTreeEffectiveStatement<?>> dataTree;
+        private final @NonNull ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTree;
+        private final @NonNull ImmutableMap<QName, DataTreeEffectiveStatement<?>> dataTree;
         private final @NonNull D declared;
 
         protected DefaultWithDataTree(final D declared, final StmtContext<?, ?, ?> ctx,
                 final ImmutableList<? extends EffectiveStatement<?, ?>> substatements) {
             this.declared = requireNonNull(declared);
             final StatementSourceReference ref = ctx.getStatementSourceReference();
-            this.schemaTree = AbstractSchemaEffectiveDocumentedNode.createSchemaTreeNamespace(ref, substatements);
-            this.dataTree = AbstractSchemaEffectiveDocumentedNode.createDataTreeNamespace(ref, schemaTree);
+            final Map<QName, SchemaTreeEffectiveStatement<?>> schema =
+                    AbstractSchemaEffectiveDocumentedNode.createSchemaTreeNamespace(ref, substatements);
+            this.schemaTree = ImmutableMap.copyOf(schema);
+            this.dataTree = AbstractSchemaEffectiveDocumentedNode.createDataTreeNamespace(ref, schema.values(),
+                schemaTree);
         }
 
         @Override

diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractSchemaEffectiveDocumentedNode.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractSchemaEffectiveDocumentedNode.java
index 6b2ca676b52f3b7b81af355901647f0726f7d32a..11354c51288940ce689c99bc881d66743fbc5c5d 100644 (file)
--- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractSchemaEffectiveDocumentedNode.java
+++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractSchemaEffectiveDocumentedNode.java
@@ -15,10 +15,8 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import java.lang.invoke.VarHandle;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Optional;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.yang.common.QName;
@@ -47,8 +45,8 @@ import org.opendaylight.yangtools.yang.parser.spi.source.StatementSourceReferenc
 @Beta
 public abstract class AbstractSchemaEffectiveDocumentedNode<A, D extends DeclaredStatement<A>>
         extends AbstractEffectiveDocumentedNode<A, D> {
-    private final Map<QName, DataTreeEffectiveStatement<?>> dataTreeNamespace;
-    private final Map<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace;
+    private final ImmutableMap<QName, DataTreeEffectiveStatement<?>> dataTreeNamespace;
+    private final ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace;
 
     protected AbstractSchemaEffectiveDocumentedNode(final StmtContext<A, D, ?> ctx) {
         super(ctx);
@@ -56,14 +54,18 @@ public abstract class AbstractSchemaEffectiveDocumentedNode<A, D extends Declare
         // This check is rather weird, but comes from our desire to lower memory footprint while providing both
         // EffectiveStatements and SchemaNode interfaces -- which do not overlap completely where child lookups are
         // concerned. This ensures that we have SchemaTree index available for use with child lookups.
+        final Map<QName, SchemaTreeEffectiveStatement<?>> schemaTree;
         if (this instanceof SchemaTreeAwareEffectiveStatement || this instanceof DataNodeContainer) {
-            schemaTreeNamespace = createSchemaTreeNamespace(ctx.getStatementSourceReference(),
+            schemaTree = createSchemaTreeNamespace(ctx.getStatementSourceReference(),
                 effectiveSubstatements());
         } else {
-            schemaTreeNamespace = ImmutableMap.of();
+            schemaTree = ImmutableMap.of();
         }
-        if (this instanceof DataTreeAwareEffectiveStatement && !schemaTreeNamespace.isEmpty()) {
-            dataTreeNamespace = createDataTreeNamespace(ctx.getStatementSourceReference(), schemaTreeNamespace);
+        schemaTreeNamespace = ImmutableMap.copyOf(schemaTree);
+
+        if (this instanceof DataTreeAwareEffectiveStatement && !schemaTree.isEmpty()) {
+            dataTreeNamespace = createDataTreeNamespace(ctx.getStatementSourceReference(), schemaTree.values(),
+                schemaTreeNamespace);
         } else {
             dataTreeNamespace = ImmutableMap.of();
         }
@@ -104,16 +106,18 @@ public abstract class AbstractSchemaEffectiveDocumentedNode<A, D extends Declare
         final Map<QName, SchemaTreeEffectiveStatement<?>> schemaChildren = new LinkedHashMap<>();
         substatements.stream().filter(SchemaTreeEffectiveStatement.class::isInstance)
             .forEach(child -> putChild(schemaChildren, (SchemaTreeEffectiveStatement) child, ref, "schema"));
-        return toImmutable(schemaChildren);
+        return schemaChildren;
     }
 
-    static @NonNull Map<QName, DataTreeEffectiveStatement<?>> createDataTreeNamespace(
+    static @NonNull ImmutableMap<QName, DataTreeEffectiveStatement<?>> createDataTreeNamespace(
             final StatementSourceReference ref,
-            final Map<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace) {
+            final Collection<SchemaTreeEffectiveStatement<?>> schemaTreeStatements,
+            // Note: this dance is needed to not retain ImmutableMap$Values
+            final ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace) {
         final Map<QName, DataTreeEffectiveStatement<?>> dataChildren = new LinkedHashMap<>();
         boolean sameAsSchema = true;
 
-        for (SchemaTreeEffectiveStatement<?> child : schemaTreeNamespace.values()) {
+        for (SchemaTreeEffectiveStatement<?> child : schemaTreeStatements) {
             if (child instanceof DataTreeEffectiveStatement) {
                 putChild(dataChildren, (DataTreeEffectiveStatement<?>) child, ref, "data");
             } else {
@@ -124,22 +128,7 @@ public abstract class AbstractSchemaEffectiveDocumentedNode<A, D extends Declare
 
         // This is a mighty hack to lower memory usage: if we consumed all schema tree children as data nodes,
         // the two maps are equal and hence we can share the instance.
-        return sameAsSchema ? (Map) schemaTreeNamespace : toImmutable(dataChildren);
-    }
-
-    private static <K, V> Map<K, V> toImmutable(final Map<K, V> map) {
-        switch (map.size()) {
-            case 0:
-                return ImmutableMap.of();
-            case 1:
-                // Special case: singleton ImmutableMap is actually SingletonImmutableBiMap, which allocates its inverse
-                //               view and its keySet() when asked for values() -- which costs us 64 bytes (40+24).
-                //               java.util.Collections can do the same job for less memory.
-                final Entry<K, V> entry = map.entrySet().iterator().next();
-                return Collections.singletonMap(entry.getKey(), entry.getValue());
-            default:
-                return ImmutableMap.copyOf(map);
-        }
+        return sameAsSchema ? (ImmutableMap) schemaTreeNamespace : ImmutableMap.copyOf(dataChildren);
     }
 
     @SuppressWarnings("unchecked")