Do not use singleton ImmutableMap for datatree/schematree 61/87261/2
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 29 Jan 2020 00:39:50 +0000 (01:39 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 29 Jan 2020 12:29:20 +0000 (13:29 +0100)
As it turns out singleton ImmutableMap has a few kinks, one of which
is allocating its inverse when asked for values. This is consting us
48 bytes more than with Collections.singletonMap(), hence let's
special case to bring our footprint down.

JIRA: YANGTOOLS-652
Change-Id: I7ac170c02609a15b3ec77a91e5075ec62f814232
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit e61246c419cd0655ca9dccdcd9e497440324d7b7)

yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractDeclaredEffectiveStatement.java
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractSchemaEffectiveDocumentedNode.java

index fe88538bf05baa5db1778db47e8dc8185705df4f..c6987477ee13efecda3a4e5aace2482298f1ac95 100644 (file)
@@ -12,7 +12,6 @@ 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;
@@ -84,7 +83,7 @@ public abstract class AbstractDeclaredEffectiveStatement<A, D extends DeclaredSt
             return child instanceof DataSchemaNode ? Optional.of((DataSchemaNode) child) : Optional.empty();
         }
 
-        protected abstract ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace();
+        protected abstract Map<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace();
     }
 
     /**
@@ -107,7 +106,7 @@ public abstract class AbstractDeclaredEffectiveStatement<A, D extends DeclaredSt
             return super.getNamespaceContents(namespace);
         }
 
-        protected abstract ImmutableMap<QName, DataTreeEffectiveStatement<?>> dataTreeNamespace();
+        protected abstract Map<QName, DataTreeEffectiveStatement<?>> dataTreeNamespace();
     }
 
     /**
@@ -176,7 +175,7 @@ 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 ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTree;
+        private final @NonNull Map<QName, SchemaTreeEffectiveStatement<?>> schemaTree;
         private final @NonNull D declared;
 
         protected DefaultWithSchemaTree(final D declared, final StmtContext<?, ?, ?> ctx,
@@ -192,7 +191,7 @@ public abstract class AbstractDeclaredEffectiveStatement<A, D extends DeclaredSt
         }
 
         @Override
-        protected final ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace() {
+        protected final Map<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace() {
             return schemaTree;
         }
     }
@@ -207,8 +206,8 @@ 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 ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTree;
-        private final @NonNull ImmutableMap<QName, DataTreeEffectiveStatement<?>> dataTree;
+        private final @NonNull Map<QName, SchemaTreeEffectiveStatement<?>> schemaTree;
+        private final @NonNull Map<QName, DataTreeEffectiveStatement<?>> dataTree;
         private final @NonNull D declared;
 
         protected DefaultWithDataTree(final D declared, final StmtContext<?, ?, ?> ctx,
@@ -225,12 +224,12 @@ public abstract class AbstractDeclaredEffectiveStatement<A, D extends DeclaredSt
         }
 
         @Override
-        protected final ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace() {
+        protected final Map<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace() {
             return schemaTree;
         }
 
         @Override
-        protected final ImmutableMap<QName, DataTreeEffectiveStatement<?>> dataTreeNamespace() {
+        protected final Map<QName, DataTreeEffectiveStatement<?>> dataTreeNamespace() {
             return dataTree;
         }
     }
index 79ae09fee62edd01e50e645716b1b21f0002609a..6b2ca676b52f3b7b81af355901647f0726f7d32a 100644 (file)
@@ -15,8 +15,10 @@ 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;
@@ -45,8 +47,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 ImmutableMap<QName, DataTreeEffectiveStatement<?>> dataTreeNamespace;
-    private final ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace;
+    private final Map<QName, DataTreeEffectiveStatement<?>> dataTreeNamespace;
+    private final Map<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace;
 
     protected AbstractSchemaEffectiveDocumentedNode(final StmtContext<A, D, ?> ctx) {
         super(ctx);
@@ -97,17 +99,17 @@ public abstract class AbstractSchemaEffectiveDocumentedNode<A, D extends Declare
         return child instanceof DataSchemaNode ? Optional.of((DataSchemaNode) child) : Optional.empty();
     }
 
-    static @NonNull ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> createSchemaTreeNamespace(
+    static @NonNull Map<QName, SchemaTreeEffectiveStatement<?>> createSchemaTreeNamespace(
             final StatementSourceReference ref, final Collection<? extends EffectiveStatement<?, ?>> substatements) {
         final Map<QName, SchemaTreeEffectiveStatement<?>> schemaChildren = new LinkedHashMap<>();
         substatements.stream().filter(SchemaTreeEffectiveStatement.class::isInstance)
             .forEach(child -> putChild(schemaChildren, (SchemaTreeEffectiveStatement) child, ref, "schema"));
-        return ImmutableMap.copyOf(schemaChildren);
+        return toImmutable(schemaChildren);
     }
 
-    static @NonNull ImmutableMap<QName, DataTreeEffectiveStatement<?>> createDataTreeNamespace(
+    static @NonNull Map<QName, DataTreeEffectiveStatement<?>> createDataTreeNamespace(
             final StatementSourceReference ref,
-            final ImmutableMap<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace) {
+            final Map<QName, SchemaTreeEffectiveStatement<?>> schemaTreeNamespace) {
         final Map<QName, DataTreeEffectiveStatement<?>> dataChildren = new LinkedHashMap<>();
         boolean sameAsSchema = true;
 
@@ -122,7 +124,22 @@ 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 ? (ImmutableMap) schemaTreeNamespace : ImmutableMap.copyOf(dataChildren);
+        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);
+        }
     }
 
     @SuppressWarnings("unchecked")