Improve calling TrieMap calling convention

rovarga · rovarga · commit 7ff546ae6268 · 2025-01-25T02:52:41.000+01:00
Rather than passing as the last argument, always pass it as the first,
making for more navigable code.

Signed-off-by: Robert Varga &lt;robert.varga@pantheon.tech&gt;
diff --git a/triemap/src/main/java/tech/pantheon/triemap/CNode.java b/triemap/src/main/java/tech/pantheon/triemap/CNode.java
@@ -144,19 +144,19 @@ boolean insert(final MutableTrieMap<K, V> ct, final INode<K, V> in, final int po
             final K key, final V val, final int hc, final int lev) {
         final CNode<K, V> next;
         if (!sn.matches(hc, key)) {
-            final var rn = gen == in.gen ? this : renewed(gen, ct);
+            final var rn = gen == in.gen ? this : renewed(ct, gen);
             next = rn.updatedAt(pos, new INode<>(in, sn, key, val, hc, lev), gen);
         } else {
             next = updatedAt(pos, key, val, hc, gen);
         }
-        return in.gcasWrite(next, ct);
+        return in.gcasWrite(ct, next);
     }
 
     boolean insert(final MutableTrieMap<K, V> ct, final INode<K, V> in, final int pos, final int flag, final K key,
             final V val, final int hc) {
         final var ngen = in.gen;
-        final var rn = gen == ngen ? this : renewed(ngen, ct);
-        return in.gcasWrite(rn.toInsertedAt(this, ngen, pos, flag, key, val, hc), ct);
+        final var rn = gen == ngen ? this : renewed(ct, ngen);
+        return in.gcasWrite(ct, rn.toInsertedAt(this, ngen, pos, flag, key, val, hc));
     }
 
     @Nullable Result<V> insertIf(final MutableTrieMap<K, V> ct, final Gen startGen, final int hc, final K key,
@@ -194,16 +194,16 @@ boolean insert(final MutableTrieMap<K, V> ct, final INode<K, V> in, final int po
         if (!sn.matches(hc, key)) {
             if (cond == null || cond == ABSENT) {
                 final var ngen = in.gen;
-                final var rn = gen == ngen ? this : renewed(ngen, ct);
-                return in.gcasWrite(rn.toUpdatedAt(this, pos, new INode<>(in, sn, key, val, hc, lev), ngen), ct)
+                final var rn = gen == ngen ? this : renewed(ct, ngen);
+                return in.gcasWrite(ct, rn.toUpdatedAt(this, pos, new INode<>(in, sn, key, val, hc, lev), ngen))
                     ? Result.empty() : null;
             }
             return Result.empty();
         }
         if (cond == ABSENT) {
             return sn.toResult();
         } else if (cond == null || cond == PRESENT || cond.equals(sn.value())) {
-            return in.gcasWrite(updatedAt(pos, key, val, hc, gen), ct) ? sn.toResult() : null;
+            return in.gcasWrite(ct, updatedAt(pos, key, val, hc, gen)) ? sn.toResult() : null;
         }
         return Result.empty();
     }
@@ -226,7 +226,7 @@ boolean insert(final MutableTrieMap<K, V> ct, final INode<K, V> in, final int po
             if (!sn.matches(hc, key) || cond != null && !cond.equals(sn.value())) {
                 return Result.empty();
             }
-            return parent.gcasWrite(toRemoved(ct, flag, pos, lev), ct) ? sn.toResult() : null;
+            return parent.gcasWrite(ct, toRemoved(ct, flag, pos, lev)) ? sn.toResult() : null;
         } else {
             throw invalidElement(sub);
         }
@@ -252,7 +252,7 @@ MainNode<K, V> toCompressed(final TrieMap<K, V> ct, final Gen ngen, final int le
         final var narr = newArray(len);
         for (int i = 0; i < len; i++) {
             final var tmp = arr[i];
-            narr[i] = tmp instanceof INode<K, V> in ? resurrect(in, ct) : tmp;
+            narr[i] = tmp instanceof INode<K, V> in ? in.resurrect(ct) : tmp;
         }
 
         return toUpdated(ngen, lev, narr, bitmap);
@@ -275,7 +275,7 @@ private MainNode<K, V> toUpdated(final Gen ngen, final int lev, final Branch<K,
 
     // tries to gcasWrite() a copy of this CNode renewed to ngen
     private boolean renew(final TrieMap<K, V> ct, final INode<K, V> in, final Gen ngen) {
-        return in.gcasWrite(renewed(ngen, ct), ct);
+        return in.gcasWrite(ct, renewed(ct, ngen));
     }
 
     @Override
@@ -298,22 +298,22 @@ private int computeSize(final ImmutableTrieMap<?, ?> ct) {
         final int len = array.length;
         return switch (len) {
             case 0 -> 0;
-            case 1 ->  elementSize(array[0], ct);
+            case 1 -> elementSize(ct, array[0]);
             default -> {
                 final int offset = ThreadLocalRandom.current().nextInt(len);
                 int sz = 0;
                 for (int i = offset; i < len; ++i) {
-                    sz += elementSize(array[i], ct);
+                    sz += elementSize(ct, array[i]);
                 }
                 for (int i = 0; i < offset; ++i) {
-                    sz += elementSize(array[i], ct);
+                    sz += elementSize(ct, array[i]);
                 }
                 yield sz;
             }
         };
     }
 
-    private static int elementSize(final Branch<?, ?> elem, final ImmutableTrieMap<?, ?> ct) {
+    private static int elementSize(final ImmutableTrieMap<?, ?> ct, final Branch<?, ?> elem) {
         if (elem instanceof SNode) {
             return 1;
         } else if (elem instanceof INode<?, ?> inode) {
@@ -353,13 +353,13 @@ private CNode<K, V> toUpdatedAt(final CNode<K, V> prev, final int pos, final Bra
      * Returns a copy of this cnode such that all the i-nodes below it are copied to the specified generation
      * {@code ngen}.
      */
-    private CNode<K, V> renewed(final Gen ngen, final TrieMap<K, V> ct) {
+    private CNode<K, V> renewed(final TrieMap<K, V> ct, final Gen ngen) {
         final var arr = array;
         final int len = arr.length;
         final var narr = newArray(len);
         for (int i = 0; i < len; i++) {
             final var tmp = arr[i];
-            narr[i] = tmp instanceof INode<K, V> in ? in.copyToGen(ngen, ct) : tmp;
+            narr[i] = tmp instanceof INode<K, V> in ? in.copyToGen(ct, ngen) : tmp;
         }
         return new CNode<>(this, ngen, bitmap, narr);
     }
@@ -374,10 +374,6 @@ private Branch<K, V>[] newArray(final int size) {
         return new Branch[size];
     }
 
-    private static <K, V> Branch<K, V> resurrect(final INode<K, V> in, final TrieMap<K, V> ct) {
-        return in.gcasReadNonNull(ct) instanceof TNode<K, V> tn ? new SNode<>(tn) : in;
-    }
-
     // Visible for testing
     static VerifyException invalidElement(final Branch<?, ?> elem) {
         throw new VerifyException("A CNode can contain only INodes and SNodes, not " + elem);
diff --git a/triemap/src/main/java/tech/pantheon/triemap/INode.java b/triemap/src/main/java/tech/pantheon/triemap/INode.java
@@ -128,20 +128,20 @@ private TryGcas(final Gcas<K, V> prev) {
     }
 
     MainNode<K, V> gcasRead(final TrieMap<?, ?> ct) {
-        return gcasComplete((MainNode<K, V>) MAIN_VH.getVolatile(this), ct);
+        return gcasComplete(ct, (MainNode<K, V>) MAIN_VH.getVolatile(this));
     }
 
-    boolean gcasWrite(final MainNode<K, V> next, final TrieMap<?, ?> ct) {
+    boolean gcasWrite(final TrieMap<?, ?> ct, final MainNode<K, V> next) {
         // note: plain read of 'next', i.e. take a look at what we are about to CAS-out
         if (MAIN_VH.compareAndSet(this, VerifyException.throwIfNull(PREV_VH.get(next)), next)) {
             // established as write, now try to complete it and report if we have succeeded
-            gcasComplete(next, ct);
+            gcasComplete(ct, next);
             return PREV_VH.getVolatile(next) == null;
         }
         return false;
     }
 
-    private MainNode<K, V> gcasComplete(final MainNode<K, V> firstMain, final TrieMap<?, ?> ct) {
+    private MainNode<K, V> gcasComplete(final TrieMap<?, ?> ct, final MainNode<K, V> firstMain) {
         // complete the GCAS starting at firstMain
         var currentMain = firstMain;
 
@@ -211,7 +211,7 @@ private MainNode<K, V> gcasComplete(final MainNode<K, V> firstMain, final TrieMa
         }
     }
 
-    INode<K, V> copyToGen(final Gen ngen, final TrieMap<K, V> ct) {
+    INode<K, V> copyToGen(final TrieMap<K, V> ct, final Gen ngen) {
         return new INode<>(ngen, gcasRead(ct));
     }
 
@@ -329,10 +329,16 @@ boolean insert(final MutableTrieMap<K, V> ct, final Gen startGen, final int hc,
 
     private void clean(final TrieMap<K, V> ct, final INode<K, V> parent, final int lev) {
         if (parent.gcasRead(ct) instanceof CNode<K, V> cn) {
-            parent.gcasWrite(cn.toCompressed(ct, gen, lev - LEVEL_BITS), ct);
+            parent.gcasWrite(ct, cn.toCompressed(ct, gen, lev - LEVEL_BITS));
         }
     }
 
+    // gcasRead() and if it is a TNode convert it to an SNode instead. Called indirectly via cn.toCompressed() from
+    // clean() just above
+    Branch<K, V> resurrect(final TrieMap<K, V> ct) {
+        return gcasReadNonNull(ct) instanceof TNode<K, V> tn ? new SNode<>(tn) : this;
+    }
+
     private void cleanParent(final MutableTrieMap<K, V> ct, final Gen startGen, final int hc, final TNode<K, V> tn,
             final INode<K, V> parent, final int lev) {
         while (true) {
@@ -357,7 +363,7 @@ private void cleanParent(final MutableTrieMap<K, V> ct, final Gen startGen, fina
             }
 
             // retry while we make progress and the tree is does not move to next generation
-            if (parent.gcasWrite(cn.toContracted(gen, pos, tn, lev - LEVEL_BITS), ct)
+            if (parent.gcasWrite(ct, cn.toContracted(gen, pos, tn, lev - LEVEL_BITS))
                 || ct.readRoot().gen != startGen) {
                 return;
             }
diff --git a/triemap/src/main/java/tech/pantheon/triemap/ImmutableTrieMap.java b/triemap/src/main/java/tech/pantheon/triemap/ImmutableTrieMap.java
@@ -109,7 +109,7 @@ public int size() {
 
     @Override
     public MutableTrieMap<K, V> mutableSnapshot() {
-        return new MutableTrieMap<>(root.copyToGen(new Gen(), this));
+        return new MutableTrieMap<>(root.copyToGen(this, new Gen()));
     }
 
     @Override
diff --git a/triemap/src/main/java/tech/pantheon/triemap/LNodeEntries.java b/triemap/src/main/java/tech/pantheon/triemap/LNodeEntries.java
@@ -86,19 +86,19 @@ static <K,V> LNodeEntries<K, V> of(final K k1, final V v1, final K k2, final V v
     final boolean insert(final MutableTrieMap<K, V> ct, final INode<K, V> in, final LNode<K, V> ln, final K key,
             final V val) {
         final var entry = findEntry(key);
-        return in.gcasWrite(entry == null ? toInserted(ln, key, val) : toReplaced(ln, entry, val), ct);
+        return in.gcasWrite(ct, entry == null ? toInserted(ln, key, val) : toReplaced(ln, entry, val));
     }
 
     @Nullable Result<V> insertIf(final MutableTrieMap<K, V> ct, final INode<K, V> in, final LNode<K, V> ln, final K key,
             final V val, final Object cond) {
         final var entry = findEntry(key);
         if (entry == null) {
-            return cond != null && cond != ABSENT || in.gcasWrite(toInserted(ln, key, val), ct) ? Result.empty() : null;
+            return cond != null && cond != ABSENT || in.gcasWrite(ct, toInserted(ln, key, val)) ? Result.empty() : null;
         }
         if (cond == ABSENT) {
             return entry.toResult();
         } else if (cond == null || cond == PRESENT || cond.equals(entry.value())) {
-            return in.gcasWrite(toReplaced(ln, entry, val), ct) ? entry.toResult() : null;
+            return in.gcasWrite(ct, toReplaced(ln, entry, val)) ? entry.toResult() : null;
         }
         return Result.empty();
     }
@@ -124,7 +124,7 @@ final boolean insert(final MutableTrieMap<K, V> ct, final INode<K, V> in, final
         final var size = ln.size;
         final var next = size == 2 ? new TNode<>(ln, map.key(), map.value(), hc) : new LNode<>(ln, map, size - 1);
 
-        return in.gcasWrite(next, ct) ? entry.toResult() : null;
+        return in.gcasWrite(ct, next) ? entry.toResult() : null;
     }
 
     private LNode<K, V> toInserted(final LNode<K, V> ln, final K key, final V val) {
diff --git a/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java b/triemap/src/main/java/tech/pantheon/triemap/MutableTrieMap.java
@@ -141,7 +141,7 @@ private INode<K, V> snapshot() {
         INode<K, V> localRoot;
         do {
             localRoot = readRoot();
-        } while (!rdcssRoot(localRoot, localRoot.gcasRead(this), localRoot.copyToGen(new Gen(), this)));
+        } while (!rdcssRoot(localRoot, localRoot.gcasRead(this), localRoot.copyToGen(this, new Gen())));
 
         return localRoot;
     }
@@ -153,7 +153,7 @@ public ImmutableTrieMap<K, V> immutableSnapshot() {
 
     @Override
     public MutableTrieMap<K, V> mutableSnapshot() {
-        return new MutableTrieMap<>(snapshot().copyToGen(new Gen(), this));
+        return new MutableTrieMap<>(snapshot().copyToGen(this, new Gen()));
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -128,20 +128,20 @@ private TryGcas(final Gcas<K, V> prev) {`
`128`	`128`	`}`
`129`	`129`
`130`	`130`	`MainNode<K, V> gcasRead(final TrieMap<?, ?> ct) {`
`131`		`- return gcasComplete((MainNode<K, V>) MAIN_VH.getVolatile(this), ct);`
	`131`	`+ return gcasComplete(ct, (MainNode<K, V>) MAIN_VH.getVolatile(this));`
`132`	`132`	`}`
`133`	`133`
`134`		`- boolean gcasWrite(final MainNode<K, V> next, final TrieMap<?, ?> ct) {`
	`134`	`+ boolean gcasWrite(final TrieMap<?, ?> ct, final MainNode<K, V> next) {`
`135`	`135`	`// note: plain read of 'next', i.e. take a look at what we are about to CAS-out`
`136`	`136`	`if (MAIN_VH.compareAndSet(this, VerifyException.throwIfNull(PREV_VH.get(next)), next)) {`
`137`	`137`	`// established as write, now try to complete it and report if we have succeeded`
`138`		`- gcasComplete(next, ct);`
	`138`	`+ gcasComplete(ct, next);`
`139`	`139`	`return PREV_VH.getVolatile(next) == null;`
`140`	`140`	`}`
`141`	`141`	`return false;`
`142`	`142`	`}`
`143`	`143`
`144`		`- private MainNode<K, V> gcasComplete(final MainNode<K, V> firstMain, final TrieMap<?, ?> ct) {`
	`144`	`+ private MainNode<K, V> gcasComplete(final TrieMap<?, ?> ct, final MainNode<K, V> firstMain) {`
`145`	`145`	`// complete the GCAS starting at firstMain`
`146`	`146`	`var currentMain = firstMain;`
`147`	`147`
`@@ -211,7 +211,7 @@ private MainNode<K, V> gcasComplete(final MainNode<K, V> firstMain, final TrieMa`
`211`	`211`	`}`
`212`	`212`	`}`
`213`	`213`
`214`		`- INode<K, V> copyToGen(final Gen ngen, final TrieMap<K, V> ct) {`
	`214`	`+ INode<K, V> copyToGen(final TrieMap<K, V> ct, final Gen ngen) {`
`215`	`215`	`return new INode<>(ngen, gcasRead(ct));`
`216`	`216`	`}`
`217`	`217`
`@@ -329,10 +329,16 @@ boolean insert(final MutableTrieMap<K, V> ct, final Gen startGen, final int hc,`
`329`	`329`
`330`	`330`	`private void clean(final TrieMap<K, V> ct, final INode<K, V> parent, final int lev) {`
`331`	`331`	`if (parent.gcasRead(ct) instanceof CNode<K, V> cn) {`
`332`		`- parent.gcasWrite(cn.toCompressed(ct, gen, lev - LEVEL_BITS), ct);`
	`332`	`+ parent.gcasWrite(ct, cn.toCompressed(ct, gen, lev - LEVEL_BITS));`
`333`	`333`	`}`
`334`	`334`	`}`
`335`	`335`
	`336`	`+ // gcasRead() and if it is a TNode convert it to an SNode instead. Called indirectly via cn.toCompressed() from`
	`337`	`+ // clean() just above`
	`338`	`+ Branch<K, V> resurrect(final TrieMap<K, V> ct) {`
	`339`	`+ return gcasReadNonNull(ct) instanceof TNode<K, V> tn ? new SNode<>(tn) : this;`
	`340`	`+ }`
	`341`	`+`
`336`	`342`	`private void cleanParent(final MutableTrieMap<K, V> ct, final Gen startGen, final int hc, final TNode<K, V> tn,`
`337`	`343`	`final INode<K, V> parent, final int lev) {`
`338`	`344`	`while (true) {`
`@@ -357,7 +363,7 @@ private void cleanParent(final MutableTrieMap<K, V> ct, final Gen startGen, fina`
`357`	`363`	`}`
`358`	`364`
`359`	`365`	`// retry while we make progress and the tree is does not move to next generation`
`360`		`- if (parent.gcasWrite(cn.toContracted(gen, pos, tn, lev - LEVEL_BITS), ct)`
	`366`	`+ if (parent.gcasWrite(ct, cn.toContracted(gen, pos, tn, lev - LEVEL_BITS))`
`361`	`367`	`\|\| ct.readRoot().gen != startGen) {`
`362`	`368`	`return;`
`363`	`369`	`}`
Original file line number	Diff line number	Diff line change
`@@ -109,7 +109,7 @@ public int size() {`
`109`	`109`
`110`	`110`	`@Override`
`111`	`111`	`public MutableTrieMap<K, V> mutableSnapshot() {`
`112`		`- return new MutableTrieMap<>(root.copyToGen(new Gen(), this));`
	`112`	`+ return new MutableTrieMap<>(root.copyToGen(this, new Gen()));`
`113`	`113`	`}`
`114`	`114`
`115`	`115`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -86,19 +86,19 @@ static <K,V> LNodeEntries<K, V> of(final K k1, final V v1, final K k2, final V v`
`86`	`86`	`final boolean insert(final MutableTrieMap<K, V> ct, final INode<K, V> in, final LNode<K, V> ln, final K key,`
`87`	`87`	`final V val) {`
`88`	`88`	`final var entry = findEntry(key);`
`89`		`- return in.gcasWrite(entry == null ? toInserted(ln, key, val) : toReplaced(ln, entry, val), ct);`
	`89`	`+ return in.gcasWrite(ct, entry == null ? toInserted(ln, key, val) : toReplaced(ln, entry, val));`
`90`	`90`	`}`
`91`	`91`
`92`	`92`	`@Nullable Result<V> insertIf(final MutableTrieMap<K, V> ct, final INode<K, V> in, final LNode<K, V> ln, final K key,`
`93`	`93`	`final V val, final Object cond) {`
`94`	`94`	`final var entry = findEntry(key);`
`95`	`95`	`if (entry == null) {`
`96`		`- return cond != null && cond != ABSENT \|\| in.gcasWrite(toInserted(ln, key, val), ct) ? Result.empty() : null;`
	`96`	`+ return cond != null && cond != ABSENT \|\| in.gcasWrite(ct, toInserted(ln, key, val)) ? Result.empty() : null;`
`97`	`97`	`}`
`98`	`98`	`if (cond == ABSENT) {`
`99`	`99`	`return entry.toResult();`
`100`	`100`	`} else if (cond == null \|\| cond == PRESENT \|\| cond.equals(entry.value())) {`
`101`		`- return in.gcasWrite(toReplaced(ln, entry, val), ct) ? entry.toResult() : null;`
	`101`	`+ return in.gcasWrite(ct, toReplaced(ln, entry, val)) ? entry.toResult() : null;`
`102`	`102`	`}`
`103`	`103`	`return Result.empty();`
`104`	`104`	`}`
`@@ -124,7 +124,7 @@ final boolean insert(final MutableTrieMap<K, V> ct, final INode<K, V> in, final`
`124`	`124`	`final var size = ln.size;`
`125`	`125`	`final var next = size == 2 ? new TNode<>(ln, map.key(), map.value(), hc) : new LNode<>(ln, map, size - 1);`
`126`	`126`
`127`		`- return in.gcasWrite(next, ct) ? entry.toResult() : null;`
	`127`	`+ return in.gcasWrite(ct, next) ? entry.toResult() : null;`
`128`	`128`	`}`
`129`	`129`
`130`	`130`	`private LNode<K, V> toInserted(final LNode<K, V> ln, final K key, final V val) {`
Original file line number	Diff line number	Diff line change
`@@ -141,7 +141,7 @@ private INode<K, V> snapshot() {`
`141`	`141`	`INode<K, V> localRoot;`
`142`	`142`	`do {`
`143`	`143`	`localRoot = readRoot();`
`144`		`- } while (!rdcssRoot(localRoot, localRoot.gcasRead(this), localRoot.copyToGen(new Gen(), this)));`
	`144`	`+ } while (!rdcssRoot(localRoot, localRoot.gcasRead(this), localRoot.copyToGen(this, new Gen())));`
`145`	`145`
`146`	`146`	`return localRoot;`
`147`	`147`	`}`
`@@ -153,7 +153,7 @@ public ImmutableTrieMap<K, V> immutableSnapshot() {`
`153`	`153`
`154`	`154`	`@Override`
`155`	`155`	`public MutableTrieMap<K, V> mutableSnapshot() {`
`156`		`- return new MutableTrieMap<>(snapshot().copyToGen(new Gen(), this));`
	`156`	`+ return new MutableTrieMap<>(snapshot().copyToGen(this, new Gen()));`
`157`	`157`	`}`
`158`	`158`
`159`	`159`	`@Override`