From: Robert Varga <rovarga@cisco.com>
Date: Sun, 1 Jan 2017 22:27:55 +0000 (+0100)
Subject: BUG-7464: cleanup TrieMap retry logic
X-Git-Tag: release/carbon~140
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F17%2F49917%2F8;p=yangtools.git

BUG-7464: cleanup TrieMap retry logic

We have a couple of while(true) loops, which can we written
concisely using a 'do { ... } while (retry);' pattern.

Also check if the from-serialization codepath does not encounter
a does not observe a concurrent modification.

Change-Id: I7970a3d11fcd6715fb86a3c669d4eb195d10fa26
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 42fa63ac6b..d39583e04c 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
@@ -19,6 +19,7 @@ import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 
 final class INode<K, V> extends BasicNode {
+
     static final Object KEY_PRESENT = new Object ();
     static final Object KEY_ABSENT = new Object ();
 
diff --git a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/TrieMap.java b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/TrieMap.java
index feb9539338..44cccbf185 100644
--- a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/TrieMap.java
+++ b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/TrieMap.java
@@ -18,6 +18,7 @@ package org.opendaylight.yangtools.triemap;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
+import com.google.common.base.Verify;
 import com.google.common.collect.Iterators;
 import java.io.ObjectStreamException;
 import java.io.Serializable;
@@ -162,51 +163,41 @@ public final class TrieMap<K, V> extends AbstractMap<K, V> implements Concurrent
     }
 
     private void inserthc(final K k, final int hc, final V v) {
-        while (true) {
-            final INode<K, V> r = RDCSS_READ_ROOT();
-            if (r.rec_insert(k, v, hc, 0, null, this)) {
-                // Successful, we are done
-                return;
-            }
-
-            // Tail recursion: inserthc(k, hc, v);
-        }
+        // TODO: this is called from serialization only, which means we should not be observing any races,
+        //       hence we should not need to pass down the entire tree, just equality (I think).
+        final INode<K, V> r = RDCSS_READ_ROOT();
+        final boolean success = r.rec_insert(k, v, hc, 0, null, this);
+        Verify.verify(success, "Concurrent modification during serialization of map %s", this);
     }
 
     private Optional<V> insertifhc(final K k, final int hc, final V v, final Object cond) {
-        while (true) {
-            final INode<K, V> r = RDCSS_READ_ROOT();
-            final Optional<V> ret = r.rec_insertif(k, v, hc, cond, 0, null, this);
-            if (ret != null) {
-                return ret;
-            }
+        Optional<V> res;
+        do {
+            // Keep looping as long as we do not get a reply
+            res = RDCSS_READ_ROOT().rec_insertif(k, v, hc, cond, 0, null, this);
+        } while (res == null);
 
-            // Tail recursion: return insertifhc(k, hc, v, cond);
-        }
+        return res;
     }
 
-    private Object lookuphc(final K k, final int hc) {
-        while (true) {
-            final INode<K, V> r = RDCSS_READ_ROOT();
-            final Object res = r.rec_lookup(k, hc, 0, null, this);
-            if (!INode.RESTART.equals(res)) {
-                return res;
-            }
+    private V lookuphc(final K k, final int hc) {
+        Object res;
+        do {
+            // Keep looping as long as RESTART is being indicated
+            res = RDCSS_READ_ROOT().rec_lookup(k, hc, 0, null, this);
+        } while (INode.RESTART.equals(res));
 
-            // Tail recursion: lookuphc(k, hc)
-        }
+        return (V) res;
     }
 
     private Optional<V> removehc(final K k, final V v, final int hc) {
-        while (true) {
-            final INode<K, V> r = RDCSS_READ_ROOT();
-            final Optional<V> res = r.rec_remove(k, v, hc, 0, null, this);
-            if (res != null) {
-                return res;
-            }
+        Optional<V> res;
+        do {
+            // Keep looping as long as we do not get a reply
+            res = RDCSS_READ_ROOT().rec_remove(k, v, hc, 0, null, this);
+        } while (res == null);
 
-            // Tail recursion: return removehc(k, v, hc);
-        }
+        return res;
     }
 
     /**
@@ -279,12 +270,11 @@ public final class TrieMap<K, V> extends AbstractMap<K, V> implements Concurrent
 
     @Override
     public void clear() {
-        while (true) {
+        boolean success;
+        do {
             final INode<K, V> r = RDCSS_READ_ROOT();
-            if (RDCSS_ROOT(r, r.gcasRead(this), newRootNode())) {
-                return;
-            }
-        }
+            success = RDCSS_ROOT(r, r.gcasRead(this), newRootNode());
+        } while (!success);
     }
 
     int computeHash(final K k) {
@@ -298,7 +288,7 @@ public final class TrieMap<K, V> extends AbstractMap<K, V> implements Concurrent
     @Override
     public V get(final Object key) {
         final K k = (K) checkNotNull(key);
-        return (V) lookuphc(k, computeHash(k));
+        return lookuphc(k, computeHash(k));
     }
 
     @Override