From 931fbf0721c07924916f20e05dc3b3cbb1d94b71 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 17 Jan 2025 12:45:00 +0100 Subject: [PATCH 1/5] Use local variable type inference INode We have a few spots where we missed the opportunity to be more concise. Fix that up. Signed-off-by: Robert Varga --- triemap/src/main/java/tech/pantheon/triemap/INode.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/triemap/src/main/java/tech/pantheon/triemap/INode.java b/triemap/src/main/java/tech/pantheon/triemap/INode.java index f6ded9f..189e875 100644 --- a/triemap/src/main/java/tech/pantheon/triemap/INode.java +++ b/triemap/src/main/java/tech/pantheon/triemap/INode.java @@ -66,7 +66,7 @@ private MainNode gcasComplete(final MainNode oldmain, final TrieMap< if (prev instanceof FailedNode) { // try to commit to previous value - FailedNode fn = (FailedNode) prev; + final var fn = (FailedNode) prev; if (MAINNODE.compareAndSet(this, main, fn.readPrev())) { return fn.readPrev(); } @@ -139,7 +139,7 @@ private boolean recInsert(final K key, final V val, final int hc, final int lev, if (m instanceof CNode) { // 1) a multiway node - final CNode cn = (CNode) m; + final var cn = (CNode) m; final int idx = hc >>> lev & 0x1f; final int flag = 1 << idx; final int bmp = cn.bitmap; @@ -226,7 +226,7 @@ static VerifyException invalidElement(final BasicNode elem) { if (m instanceof CNode) { // 1) a multiway node - final CNode cn = (CNode) m; + final var cn = (CNode) m; final int idx = hc >>> lev & 0x1f; final int flag = 1 << idx; final int bmp = cn.bitmap; @@ -235,7 +235,7 @@ static VerifyException invalidElement(final BasicNode elem) { if ((bmp & flag) != 0) { // 1a) insert below - final BasicNode cnAtPos = cn.array[pos]; + final var cnAtPos = cn.array[pos]; if (cnAtPos instanceof INode) { @SuppressWarnings("unchecked") final var in = (INode) cnAtPos; From 18b218b2c2413990a71fdc6ceee320f52f49834f Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 17 Jan 2025 12:55:24 +0100 Subject: [PATCH 2/5] Use compareAndExchange() in casRoot() compareAndExchange() is better than compareAndSwap() in that it provides the witness value. Switching casRoot() to this paradigm allows us to speed up speed up retries in rdcssComplete(). Signed-off-by: Robert Varga --- .../tech/pantheon/triemap/MutableTrieMap.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java b/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java index e301310..1b2792b 100644 --- a/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java +++ b/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java @@ -203,13 +203,14 @@ private void inserthc(final K key, final int hc, final V value) { return res; } - private boolean casRoot(final Root ov, final Root nv) { - return ROOT.compareAndSet(this, ov, nv); + private Root casRoot(final Root ov, final Root nv) { + return (Root) ROOT.compareAndExchange(this, ov, nv); } private boolean rdcssRoot(final INode ov, final MainNode expectedmain, final INode nv) { final var desc = new RDCSS_Descriptor<>(ov, expectedmain, nv); - if (casRoot(ov, desc)) { + final var witness = casRoot(ov, desc); + if (witness == ov) { rdcssComplete(false); return /* READ */desc.committed; } @@ -219,8 +220,9 @@ private boolean rdcssRoot(final INode ov, final MainNode expectedmai @SuppressWarnings("unchecked") private INode rdcssComplete(final boolean abort) { + var r = /* READ */ root; + while (true) { - final var r = /* READ */ root; if (r instanceof INode) { return (INode) r; } @@ -234,7 +236,8 @@ private INode rdcssComplete(final boolean abort) { final var nv = desc.nv; if (abort) { - if (casRoot(desc, ov)) { + r = casRoot(desc, ov); + if (r == desc) { return ov; } @@ -244,7 +247,8 @@ private INode rdcssComplete(final boolean abort) { final var oldmain = ov.gcasRead(this); if (oldmain == exp) { - if (casRoot(desc, nv)) { + r = casRoot(desc, nv); + if (r == desc) { desc.committed = true; return nv; } @@ -253,7 +257,8 @@ private INode rdcssComplete(final boolean abort) { continue; } - if (casRoot(desc, ov)) { + r = casRoot(desc, ov); + if (r == desc) { return ov; } From 05f491e0db21853a5d5b19a59aabba1a62931fe5 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 17 Jan 2025 13:05:01 +0100 Subject: [PATCH 3/5] Improve rdcssComplete() There are only two callers, who have the current value readily available. Use that value instead of reading it again. Signed-off-by: Robert Varga --- .../java/tech/pantheon/triemap/MutableTrieMap.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java b/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java index 1b2792b..80eebe4 100644 --- a/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java +++ b/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java @@ -160,7 +160,7 @@ INode rdcssReadRoot(final boolean abort) { return (INode) r; } if (r instanceof RDCSS_Descriptor) { - return rdcssComplete(abort); + return rdcssComplete((RDCSS_Descriptor) r, abort); } throw new VerifyException("Unhandled root " + r); } @@ -211,16 +211,16 @@ private boolean rdcssRoot(final INode ov, final MainNode expectedmai final var desc = new RDCSS_Descriptor<>(ov, expectedmain, nv); final var witness = casRoot(ov, desc); if (witness == ov) { - rdcssComplete(false); - return /* READ */desc.committed; + rdcssComplete(desc, false); + return /* READ */ desc.committed; } return false; } @SuppressWarnings("unchecked") - private INode rdcssComplete(final boolean abort) { - var r = /* READ */ root; + private INode rdcssComplete(final RDCSS_Descriptor initial, final boolean abort) { + Root r = initial; while (true) { if (r instanceof INode) { From 8e9939f7f060fdb1fc3fa2e09d6bf96b525d9b89 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 17 Jan 2025 14:49:19 +0100 Subject: [PATCH 4/5] Improve rdcssComplete() loop We are entering with a RDCSS_Descriptor, so let's not lose that information. The refactored loop is also denser, as we now share the code path taken for abort case and when the expected main changes. Signed-off-by: Robert Varga --- .../tech/pantheon/triemap/MutableTrieMap.java | 46 +++++++------------ 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java b/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java index 80eebe4..e4dc35f 100644 --- a/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java +++ b/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java @@ -220,49 +220,35 @@ private boolean rdcssRoot(final INode ov, final MainNode expectedmai @SuppressWarnings("unchecked") private INode rdcssComplete(final RDCSS_Descriptor initial, final boolean abort) { - Root r = initial; + var desc = initial; while (true) { - if (r instanceof INode) { - return (INode) r; - } - if (!(r instanceof RDCSS_Descriptor)) { - throw new VerifyException("Unhandled root " + r); - } + final Root next; - final var desc = (RDCSS_Descriptor) r; final var ov = desc.old; - final var exp = desc.expectedmain; - final var nv = desc.nv; - - if (abort) { - r = casRoot(desc, ov); - if (r == desc) { + if (abort || ov.gcasRead(this) != desc.expectedmain) { + next = casRoot(desc, ov); + if (next == desc) { return ov; } - - // Tail recursion: return RDCSS_Complete(abort); - continue; - } - - final var oldmain = ov.gcasRead(this); - if (oldmain == exp) { - r = casRoot(desc, nv); - if (r == desc) { + } else { + final var nv = desc.nv; + next = casRoot(desc, nv); + if (next == desc) { desc.committed = true; return nv; } - - // Tail recursion: return RDCSS_Complete(abort); - continue; } - r = casRoot(desc, ov); - if (r == desc) { - return ov; + if (next instanceof INode) { + return (INode) next; + } + if (!(next instanceof RDCSS_Descriptor)) { + throw new VerifyException("Unhandled root " + next); } - // Tail recursion: return RDCSS_Complete(abort); + // Tail recursion: return rdcssComplete(next, abort); + desc = (RDCSS_Descriptor) next; } } From bb65e9524eff5175c6f22b781e480a0910ae5bab Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 17 Jan 2025 15:13:36 +0100 Subject: [PATCH 5/5] Use compareAndExchange() in gcasComplete() compareAndExchange() gives is the witness value, which we can use to elide a volatile read of main node. Signed-off-by: Robert Varga --- triemap/src/main/java/tech/pantheon/triemap/INode.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/triemap/src/main/java/tech/pantheon/triemap/INode.java b/triemap/src/main/java/tech/pantheon/triemap/INode.java index 189e875..8588eb8 100644 --- a/triemap/src/main/java/tech/pantheon/triemap/INode.java +++ b/triemap/src/main/java/tech/pantheon/triemap/INode.java @@ -67,12 +67,14 @@ private MainNode gcasComplete(final MainNode oldmain, final TrieMap< if (prev instanceof FailedNode) { // try to commit to previous value final var fn = (FailedNode) prev; - if (MAINNODE.compareAndSet(this, main, fn.readPrev())) { + final var witness = (MainNode) MAINNODE.compareAndExchange(this, main, fn.readPrev()); + if (witness == main) { + // TODO: second read of FailedNode.prev. Can a FailedNode move? return fn.readPrev(); } - // Tail recursion: return GCAS_Complete(/* READ */ mainnode, ct); - main = /* READ */ mainnode; + // Tail recursion: return GCAS_Complete(witness, ct); + main = witness; continue; }