From b08662fc364d3469e4aac44ae780a347224399f4 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 30 Dec 2016 13:58:11 +0100 Subject: [PATCH 1/1] BUG-7464: Hide INodeBase.mainnode 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 --- .../opendaylight/yangtools/triemap/CNode.java | 3 +- .../opendaylight/yangtools/triemap/INode.java | 43 +++++++------------ .../yangtools/triemap/INodeBase.java | 24 ++++++----- .../yangtools/triemap/TrieMap.java | 14 +++--- 4 files changed, 36 insertions(+), 48 deletions(-) diff --git a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/CNode.java b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/CNode.java index 4a650d3de1..581594346e 100644 --- a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/CNode.java +++ b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/CNode.java @@ -216,8 +216,7 @@ final class CNode extends CNodeBase { int bmp = (1 << xidx) | (1 << yidx); if (xidx == yidx) { - INode subinode = new INode<>(gen);// (TrieMap.inodeupdater) - subinode.mainnode = dual (x, xhc, y, yhc, lev + 5, gen); + INode subinode = new INode<>(gen, dual(x, xhc, y, yhc, lev + 5, gen)); return new CNode<>(bmp, new BasicNode[] { subinode }, gen); } else { if (xidx < yidx) { 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 a712a81d5b..6c12cd8874 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,27 +19,20 @@ final class INode extends INodeBase { 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 INode newRootNode() { Gen gen = new Gen (); CNode cn = new CNode<> (0, new BasicNode[] {}, gen); - return new INode<>(cn, gen); - } - - private INode(final MainNode bn, final Gen g) { - super(g); - WRITE(bn); - } - - INode(final Gen g) { - this(null, g); - } - - void WRITE(final MainNode nval) { - INodeBase.updater.set(this, nval); + return new INode<>(gen, cn); } - boolean CAS(final MainNode old, final MainNode n) { - return INodeBase.updater.compareAndSet(this, old, n); + INode(final Gen gen, final MainNode bn) { + super(gen, bn); } MainNode gcasRead(final TrieMap ct) { @@ -47,7 +40,7 @@ final class INode extends INodeBase { } MainNode GCAS_READ(final TrieMap ct) { - MainNode m = /* READ */mainnode; + MainNode m = /* READ */ READ(); MainNode prevval = /* READ */ m.READ_PREV(); if (prevval == null) { return m; @@ -77,7 +70,7 @@ final class INode extends INodeBase { } 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 extends INodeBase { } 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 extends INodeBase { } private INode inode(final MainNode cn) { - INode nin = new INode<>(gen); - nin.WRITE(cn); - return nin; + return new INode<>(gen, cn); } INode copyToGen(final Gen ngen, final TrieMap ct) { - INode nin = new INode<>(ngen); - MainNode main = GCAS_READ(ct); - nin.WRITE(main); - return nin; + return new INode<>(ngen, GCAS_READ(ct)); } /** @@ -412,8 +400,7 @@ final class INode extends INodeBase { // Tailrec continue; } else { - return RESTART; // used to be throw - // RestartException + return RESTART; // used to be throw RestartException } } } else if (sub instanceof SNode) { diff --git a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INodeBase.java b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INodeBase.java index ab8807e4c8..d74fe104a7 100644 --- a/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INodeBase.java +++ b/third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INodeBase.java @@ -19,20 +19,24 @@ import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; abstract class INodeBase extends BasicNode { - public static final AtomicReferenceFieldUpdater updater = AtomicReferenceFieldUpdater.newUpdater(INodeBase.class, MainNode.class, "mainnode"); - - public static final Object RESTART = new Object(); - - public volatile MainNode mainnode = null; + @SuppressWarnings("rawtypes") + private static final AtomicReferenceFieldUpdater MAINNODE_UPDATER = + AtomicReferenceFieldUpdater.newUpdater(INodeBase.class, MainNode.class, "mainnode"); public final Gen gen; - public INodeBase(final Gen generation) { - gen = generation; + private volatile MainNode mainnode; + + INodeBase(final Gen generation, final MainNode mainnode) { + gen = generation; + this.mainnode = mainnode; } - public BasicNode prev() { - return null; + final boolean CAS(final MainNode old, final MainNode n) { + return MAINNODE_UPDATER.compareAndSet(this, old, n); } -} \ No newline at end of file + final MainNode READ() { + return mainnode; + } +} 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 ce9409e8c0..5bc64245cf 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 @@ -269,17 +269,15 @@ public class TrieMap extends AbstractMap implements ConcurrentMap 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 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) } } -- 2.36.6