Do not use singleton ImmutableMap for datatree/schematree 97/87297/1
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 29 Jan 2020 00:39:50 +0000 (01:39 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 30 Jan 2020 10:44:17 +0000 (11:44 +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)
(cherry picked from commit bd432e731100aa299d5564f930efcb91e0ee57e9)

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 9abcb1ea07a01a1c28a00e7a8357a75b004570ab..21c44b8b47e8ba9954e3005bdb6e6a39993b030c 100644 (file)
@@ -13,8 +13,10 @@ import static java.util.Objects.requireNonNull;
 import com.google.common.annotations.Beta;
 import com.google.common.collect.ImmutableMap;
 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;
@@ -43,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 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);
@@ -90,17 +92,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;
 
@@ -115,7 +117,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);
+        }
     }
 
     private static <T extends SchemaTreeEffectiveStatement<?>> void putChild(final Map<QName, T> map,