BUG-7464: Switch to use forked TrieMap

[yangtools.git] / common / util / src / main / java / org / opendaylight / yangtools / util / MapAdaptor.java

diff --git a/common/util/src/main/java/org/opendaylight/yangtools/util/MapAdaptor.java b/common/util/src/main/java/org/opendaylight/yangtools/util/MapAdaptor.java
index 48241cd5a638322dec8013aa54e32bca9be3de6b..6ca8710b185827002033224bd930a8450c4bd39c 100644 (file)
--- a/common/util/src/main/java/org/opendaylight/yangtools/util/MapAdaptor.java
+++ b/common/util/src/main/java/org/opendaylight/yangtools/util/MapAdaptor.java
@@ -10,11 +10,12 @@ package org.opendaylight.yangtools.util;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
-import com.romix.scala.collection.concurrent.TrieMap;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
+import org.opendaylight.yangtools.triemap.MutableTrieMap;
+import org.opendaylight.yangtools.triemap.TrieMap;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -44,24 +45,13 @@ public final class MapAdaptor {
                 DEFAULT_INSTANCE.persistMinItems, DEFAULT_INSTANCE.copyMaxItems);
     }
 
-    private static final int getProperty(final String name, final int defaultValue) {
-        try {
-            final String p = System.getProperty(name);
-            if (p != null) {
-                try {
-                    int pi = Integer.valueOf(p);
-                    if (pi <= 0) {
-                        LOG.warn("Ignoring illegal value of {}: has to be a positive number", name);
-                    } else {
-                        return pi;
-                    }
-                } catch (NumberFormatException e) {
-                    LOG.warn("Ignoring non-numerical value of {}", name, e);
-                }
-            }
-        } catch (Exception e) {
-            LOG.debug("Failed to get {}", name, e);
+    private static int getProperty(final String name, final int defaultValue) {
+        final int val = Integer.getInteger(name, defaultValue).intValue();
+        if (val > 0) {
+            return val;
         }
+
+        LOG.warn("Ignoring illegal value of {}: has to be a positive number", name);
         return defaultValue;
     }
 
@@ -80,10 +70,12 @@ public final class MapAdaptor {
         return DEFAULT_INSTANCE;
     }
 
-    public static MapAdaptor getInstance(final boolean useSingleton, final int copyMaxItems, final int persistMinItems) {
+    public static MapAdaptor getInstance(final boolean useSingleton, final int copyMaxItems,
+            final int persistMinItems) {
         Preconditions.checkArgument(copyMaxItems >= 0, "copyMaxItems has to be a non-negative integer");
         Preconditions.checkArgument(persistMinItems >= 0, "persistMinItems has to be a positive integer");
-        Preconditions.checkArgument(persistMinItems <= copyMaxItems, "persistMinItems must be less than or equal to copyMaxItems");
+        Preconditions.checkArgument(persistMinItems <= copyMaxItems,
+                "persistMinItems must be less than or equal to copyMaxItems");
         return new MapAdaptor(useSingleton, copyMaxItems, persistMinItems);
     }
 
@@ -100,15 +92,19 @@ public final class MapAdaptor {
             return new ReadWriteTrieMap<>();
         }
 
+        if (expectedSize < 2) {
+            return new HashMap<>(1);
+        }
+        if (expectedSize == 2) {
+            return new HashMap<>(2);
+        }
         return Maps.newHashMapWithExpectedSize(expectedSize);
     }
 
     /**
      * Input is treated is supposed to be left unmodified, result must be mutable.
-     *
-     * @param input
-     * @return
      */
+    @SuppressWarnings("static-method")
     public <K, V> Map<K, V> takeSnapshot(final Map<K, V> input) {
         if (input instanceof ReadOnlyTrieMap) {
             return ((ReadOnlyTrieMap<K, V>)input).toReadWrite();
@@ -116,9 +112,43 @@ public final class MapAdaptor {
 
         LOG.trace("Converting input {} to a HashMap", input);
 
-        // FIXME: be a bit smart about allocation based on observed size
+        /*
+         * The default HashMap copy constructor performs a bad thing for small maps, using the default capacity of 16
+         * as the minimum sizing hint, which can lead to wasted memory. Since the HashMap grows in powers-of-two, we
+         * only kick this in if we are storing 6 entries or less, as that results in 8-entry map -- the next power is
+         * 16, which is the default.
+         */
+        final Map<K, V> ret;
+        final int size = input.size();
+        if (size <= 6) {
+            final int target;
+            switch (size) {
+                case 0:
+                case 1:
+                    target = 1;
+                    break;
+                case 2:
+                    target = 2;
+                    break;
+                case 3:
+                    target = 4;
+                    break;
+                default:
+                    target = 8;
+            }
+
+            ret = new HashMap<>(target);
+            ret.putAll(input);
+        } else if (input instanceof HashMap) {
+            // HashMap supports cloning, but we want to make sure we trim it down if entries were removed, so we do
+            // this only after having checked for small sizes.
+            @SuppressWarnings("unchecked")
+            final Map<K, V> tmp = (Map<K, V>) ((HashMap<K, V>) input).clone();
+            ret = tmp;
+        } else {
+            ret = new HashMap<>(input);
+        }
 
-        final Map<K, V> ret = new HashMap<>(input);
         LOG.trace("Read-write HashMap is {}", ret);
         return ret;
     }
@@ -127,8 +157,8 @@ public final class MapAdaptor {
      * Input will be thrown away, result will be retained for read-only access or
      * {@link #takeSnapshot(Map)} purposes.
      *
-     * @param input
-     * @return
+     * @param input non-optimized (read-write) map
+     * @return  optimized read-only map
      */
     public <K, V> Map<K, V> optimize(final Map<K, V> input) {
         if (input instanceof ReadOnlyTrieMap) {
@@ -188,7 +218,7 @@ public final class MapAdaptor {
          * which will maintain the size for us.
          */
         LOG.trace("Copying input {} to a TrieMap ({} entries)", input, size);
-        final TrieMap<K, V> map = new TrieMap<>();
+        final MutableTrieMap<K, V> map = TrieMap.create();
         map.putAll(input);
         final Map<K, V> ret = new ReadOnlyTrieMap<>(map, size);
         LOG.trace("Read-only TrieMap is {}", ret);