From: Robert Varga <rovarga@cisco.com>
Date: Mon, 2 Jan 2017 00:20:44 +0000 (+0100)
Subject: BUG-7464: Do not recalculate hash on LNode removal
X-Git-Tag: release/carbon~137
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=e555a0dd619a4b1ba460892cb20a708f399ea349;p=yangtools.git

BUG-7464: Do not recalculate hash on LNode removal

As it turns out all call sites performing a removal operation
actually have a hashcode handy, hence we do not need to
calculate it.

This really makes sense: in order to remove a node, we need to
find it first -- and for that we have to have the key's hash
code.

Change-Id: I4a26b5b4c07da4b38d90492884fa882362a171f1
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---

diff --git a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INode.java b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INode.java
index 752dfb9874..acb37df8ac 100644
--- a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INode.java
+++ b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INode.java
@@ -176,9 +176,7 @@ final class INode<K, V> extends BasicNode {
                 clean(parent, ct, lev - 5);
                 return false;
             } else if (m instanceof LNode) {
-                LNode<K, V> ln = (LNode<K, V>) m;
-                MainNode<K, V> nn = ln.inserted(k, v);
-                return GCAS(ln, nn, ct);
+                return insertln((LNode<K, V>) m, k, v, ct);
             } else {
                 throw new IllegalStateException("Unhandled node " + m);
             }
@@ -350,9 +348,8 @@ final class INode<K, V> extends BasicNode {
         }
     }
 
-    boolean insertln(final LNode<K, V> ln, final K k, final V v, final TrieMap<K, V> ct) {
-        final LNode<K, V> nn = ln.inserted (k, v);
-        return GCAS(ln, nn, ct);
+    private boolean insertln(final LNode<K, V> ln, final K k, final V v, final TrieMap<K, V> ct) {
+        return GCAS(ln, ln.addChild(k, v), ct);
     }
 
     /**
@@ -508,28 +505,19 @@ final class INode<K, V> extends BasicNode {
             return null;
         } else if (m instanceof LNode) {
             final LNode<K, V> ln = (LNode<K, V>) m;
-            if (v == null) {
-                final Optional<V> optv = ln.get(k);
-                final MainNode<K, V> nn = ln.removed(k, ct);
-                if (GCAS(ln, nn, ct)) {
-                    return optv;
-                }
+            final Optional<V> optv = ln.get(k);
 
-                return null;
+            if (!optv.isPresent()) {
+                // Key was not found, hence no modification is needed
+                return Optional.empty();
             }
 
-            final Optional<V> tmp = ln.get(k);
-            if (tmp.isPresent() && v.equals(tmp.get())) {
-                final MainNode<K, V> nn = ln.removed(k, ct);
-                if (GCAS(ln, nn, ct)) {
-                    return tmp;
-                }
-
-                return null;
+            if (v != null && !v.equals(optv.get())) {
+                // Value does not match
+                return Optional.empty();
             }
 
-            // Key not found or value does not match: we have not removed anything
-            return Optional.empty();
+            return GCAS(ln, ln.removeChild(k, hc, ct), ct) ? optv : null;
         } else {
             throw new IllegalStateException("Unhandled node " + m);
         }
diff --git a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/LNode.java b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/LNode.java
index 7d55a7442c..c393357ab8 100644
--- a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/LNode.java
+++ b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/LNode.java
@@ -30,12 +30,11 @@ final class LNode<K, V> extends MainNode<K, V> {
         this(ListMap.map(k1, v1, k2, v2));
     }
 
-    LNode<K, V> inserted(final K k, final V v) {
+    LNode<K, V> addChild(final K k, final V v) {
         return new LNode<>(listmap.add(k, v));
     }
 
-    // FIXME: can we also get the hashcode ?
-    MainNode<K, V> removed(final K k, final TrieMap<K, V> ct) {
+    MainNode<K, V> removeChild(final K k, final int hc, final TrieMap<K, V> ct) {
         // We only ever create ListMaps with two or more entries,  and remove them as soon as they reach one element
         // (below), so we cannot observe a null return here.
         final ListMap<K, V> map = listmap.remove(k);
@@ -43,7 +42,7 @@ final class LNode<K, V> extends MainNode<K, V> {
         if (maybeKv.isPresent()) {
             final Entry<K, V> kv = maybeKv.get();
             // create it tombed so that it gets compressed on subsequent accesses
-            return new TNode<>(kv.getKey(), kv.getValue(), ct.computeHash(kv.getKey()));
+            return new TNode<>(kv.getKey(), kv.getValue(), hc);
         }
 
         return new LNode<>(map);