BUG-7464: Hide INodeBase.mainnode 72/49872/6
authorRobert Varga <rovarga@cisco.com>
Fri, 30 Dec 2016 12:58:11 +0000 (13:58 +0100)
committerRobert Varga <rovarga@cisco.com>
Mon, 9 Jan 2017 14:17:12 +0000 (15:17 +0100)
This field is accessed either as CAS or READ, with writes being
a falso positiive, in that they are performed only at initialization
time.

Refactor access patterns to hide the updater and the field, passing
down explicit initializer from the subclass.

Also remove INodeBase.prev(), as it is not called from anywhere.

Change-Id: I3bfcc7e7a606b3a0622ed04b99abbc67e41eb407
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/CNode.java
third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INode.java
third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INodeBase.java
third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/TrieMap.java

index 4a650d3de1b8f169dea9d1d20386774391b8cfdf..581594346e0728aa21f1a4c313cc5afad63ad580 100644 (file)
@@ -216,8 +216,7 @@ final class CNode<K, V> extends CNodeBase<K, V> {
             int bmp = (1 << xidx) | (1 << yidx);
 
             if (xidx == yidx) {
-                INode<K, V> subinode = new INode<>(gen);// (TrieMap.inodeupdater)
-                subinode.mainnode = dual (x, xhc, y, yhc, lev + 5, gen);
+                INode<K, V> subinode = new INode<>(gen, dual(x, xhc, y, yhc, lev + 5, gen));
                 return new CNode<>(bmp, new BasicNode[] { subinode }, gen);
             } else {
                 if (xidx < yidx) {
index a712a81d5b807a5bdf4625dc35da2793eedf7f73..6c12cd88749ce5689fb75a52f02fdf2a986f3b02 100644 (file)
@@ -19,27 +19,20 @@ final class INode<K, V> extends INodeBase<K, V> {
     static final Object KEY_PRESENT = new Object ();
     static final Object KEY_ABSENT = new Object ();
 
+    /**
+     * Virtual result for lookup methods indicating that the lookup needs to be restarted. This is a faster version
+     * of throwing a checked exception to control the restart.
+     */
+    static final Object RESTART = new Object();
+
     static <K,V> INode<K,V> newRootNode() {
         Gen gen = new Gen ();
         CNode<K, V> cn = new CNode<> (0, new BasicNode[] {}, gen);
-        return new INode<>(cn, gen);
-    }
-
-    private INode(final MainNode<K, V> bn, final Gen g) {
-        super(g);
-        WRITE(bn);
-    }
-
-    INode(final Gen g) {
-        this(null, g);
-    }
-
-    void WRITE(final MainNode<K, V> nval) {
-        INodeBase.updater.set(this, nval);
+        return new INode<>(gen, cn);
     }
 
-    boolean CAS(final MainNode<K, V> old, final MainNode<K, V> n) {
-        return INodeBase.updater.compareAndSet(this, old, n);
+    INode(final Gen gen, final MainNode<K, V> bn) {
+        super(gen, bn);
     }
 
     MainNode<K, V> gcasRead(final TrieMap<K, V> ct) {
@@ -47,7 +40,7 @@ final class INode<K, V> extends INodeBase<K, V> {
     }
 
     MainNode<K, V> GCAS_READ(final TrieMap<K, V> ct) {
-        MainNode<K, V> m = /* READ */mainnode;
+        MainNode<K, V> m = /* READ */ READ();
         MainNode<K, V> prevval = /* READ */ m.READ_PREV();
         if (prevval == null) {
             return m;
@@ -77,7 +70,7 @@ final class INode<K, V> extends INodeBase<K, V> {
                     } else {
                         // Tailrec
                         // return GCAS_Complete (/* READ */mainnode, ct);
-                        m = /* READ */mainnode;
+                        m = /* READ */ READ();
                         continue;
                     }
                 } else if (prev instanceof MainNode) {
@@ -105,7 +98,7 @@ final class INode<K, V> extends INodeBase<K, V> {
                     } else {
                         // try to abort
                         m.CAS_PREV(prev, new FailedNode<>(prev));
-                        return GCAS_Complete(/* READ */mainnode, ct);
+                        return GCAS_Complete(/* READ */ READ(), ct);
                     }
                 }
             }
@@ -128,16 +121,11 @@ final class INode<K, V> extends INodeBase<K, V> {
     }
 
     private INode<K, V> inode(final MainNode<K, V> cn) {
-        INode<K, V> nin = new INode<>(gen);
-        nin.WRITE(cn);
-        return nin;
+        return new INode<>(gen, cn);
     }
 
     INode<K, V> copyToGen(final Gen ngen, final TrieMap<K, V> ct) {
-        INode<K, V> nin = new INode<>(ngen);
-        MainNode<K, V> main = GCAS_READ(ct);
-        nin.WRITE(main);
-        return nin;
+        return new INode<>(ngen, GCAS_READ(ct));
     }
 
     /**
@@ -412,8 +400,7 @@ final class INode<K, V> extends INodeBase<K, V> {
                                 // Tailrec
                                 continue;
                             } else {
-                                return RESTART; // used to be throw
-                                // RestartException
+                                return RESTART; // used to be throw RestartException
                             }
                         }
                     } else if (sub instanceof SNode) {
index ab8807e4c8f3eb8d6e87de223bd198f843bc71c1..d74fe104a7ab76d69b4925315c10a02130e6c90d 100644 (file)
@@ -19,20 +19,24 @@ import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 
 abstract class INodeBase<K, V> extends BasicNode {
 
-    public static final AtomicReferenceFieldUpdater<INodeBase, MainNode> updater = AtomicReferenceFieldUpdater.newUpdater(INodeBase.class, MainNode.class, "mainnode");
-
-    public static final Object RESTART = new Object();
-
-    public volatile MainNode<K, V> mainnode = null;
+    @SuppressWarnings("rawtypes")
+    private static final AtomicReferenceFieldUpdater<INodeBase, MainNode> MAINNODE_UPDATER =
+            AtomicReferenceFieldUpdater.newUpdater(INodeBase.class, MainNode.class, "mainnode");
 
     public final Gen gen;
 
-    public INodeBase(final Gen generation) {
-    gen = generation;
+    private volatile MainNode<K, V> mainnode;
+
+    INodeBase(final Gen generation, final MainNode<K, V> mainnode) {
+        gen = generation;
+        this.mainnode = mainnode;
     }
 
-    public BasicNode prev() {
-    return null;
+    final boolean CAS(final MainNode<K, V> old, final MainNode<K, V> n) {
+        return MAINNODE_UPDATER.compareAndSet(this, old, n);
     }
 
-}
\ No newline at end of file
+    final MainNode<K, V> READ() {
+        return mainnode;
+    }
+}
index ce9409e8c0999ede623be99e17cda8344c373352..5bc64245cf643c8a68417de0df1351c818f59263 100644 (file)
@@ -269,17 +269,15 @@ public class TrieMap<K, V> extends AbstractMap<K, V> implements ConcurrentMap<K,
         }
     }
 
-    private Object lookuphc (final K k, final int hc) {
+    private Object lookuphc(final K k, final int hc) {
         while (true) {
-            INode<K, V> r = RDCSS_READ_ROOT ();
-            Object res = r.rec_lookup (k, hc, 0, null, r.gen, this);
-            if (res == INodeBase.RESTART) {
-                // return lookuphc (k, hc);
-                // tailrec
-                continue;
-            } else {
+            final INode<K, V> r = RDCSS_READ_ROOT ();
+            final Object res = r.rec_lookup(k, hc, 0, null, r.gen, this);
+            if (!INode.RESTART.equals(res)) {
                 return res;
             }
+
+            // Tail recursion: lookuphc(k, hc)
         }
     }