CNDB-15669: implement mayHaveContent metadata flag by lesnik2u · Pull Request #4 · blambov/cassandra

lesnik2u · 2026-02-18T15:30:43Z

Introduces a metadata flag in encoded cursor positions to indicate whether a node potentially carries content. This allows the trie iteration infrastructure to skip expensive content resolution calls for nodes that are guaranteed to be structural.

Those are the results of 50 benchmarks averaged:

Benchmark average before:

Benchmark                                (BATCH)  (count)  (deletionPattern)  (deletionSpec)  (deletionsRatio)  (flush)     (memtableClass)  (partitions)  (threadCount)  (useNet)  Mode  Cnt  Score   Error  Units
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM        TrieMemtable          1000              1     false  avgt   10  8.139 ± 0.230  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM        TrieMemtable             4              1     false  avgt   10  8.158 ± 0.399  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM  TrieMemtableStage2          1000              1     false  avgt   10  8.113 ± 0.832  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM  TrieMemtableStage2             4              1     false  avgt   10  7.804 ± 1.454  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM  TrieMemtableStage1          1000              1     false  avgt   10  7.568 ± 1.111  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM  TrieMemtableStage1             4              1     false  avgt   10  7.154 ± 0.371  ms/op

Benchmark average after:

ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM        TrieMemtable          1000              1     false  avgt   10  8.003 ± 0.313  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM        TrieMemtable             4              1     false  avgt   10  7.908 ± 0.643  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM  TrieMemtableStage2          1000              1     false  avgt   10  8.061 ± 1.140  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM  TrieMemtableStage2             4              1     false  avgt   10  7.601 ± 0.424  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM  TrieMemtableStage1          1000              1     false  avgt   10  7.458 ± 0.435  ms/op
ReadTestWidePartitions.readGreaterMatch     1000  1000000             RANDOM           EQUAL                 0    INMEM  TrieMemtableStage1             4              1     false  avgt   10  7.547 ± 0.373  ms/op

Flamegraphs for TrieMemtable seem to backup those results eg.
Before:

After:

lesnik2u · 2026-02-18T15:32:55Z

This file was CLRF for some reason.. Lmk if I should change it back to CLRF
src/java/org/apache/cassandra/db/tries/DepthAdjustedCursor.java: ASCII text, with CRLF line terminators

blambov

Nice work.

blambov · 2026-02-19T08:17:20Z

+                // the head alone would not reflect.
+                currentPosition = pos;
+                applyToSelectedElementsInHeap((self, cursor, index) -> {
+                    currentPosition |= cursor.encodedPosition();


Try using self.currentPosition to avoid capturing this in the closure.

blambov · 2026-02-19T08:18:34Z

+                applyToSelectedElementsInHeap((self, cursor, index) -> {
+                    currentPosition |= cursor.encodedPosition();
+                }, 0);
+                currentPosition = Cursor.unionFlags(pos, currentPosition, Cursor.FLAGS_MASK);


This shouldn't be necessary as the position bits of all the selected cursors must match. Maybe replace with a comment?

done, added.

blambov · 2026-02-19T08:20:17Z

+    /// Flag indicating whether this position may have content.
+    long MAY_HAVE_CONTENT_BIT = 1L;
+
    /// reverse direction.


The beginning of this doc is missing.

added it back, removed by mistake when rebasing

blambov · 2026-02-19T08:22:10Z

                             depth(encodedPosition),
                             incomingTransition(encodedPosition),
                             isOnReturnPath(encodedPosition) ? "↑" : " ",
+                             (encodedPosition & MAY_HAVE_CONTENT_BIT) != 0 ? "*" : " ",


Make this "C" for content.

blambov · 2026-02-19T08:25:28Z

        state = c2 != null ? State.AT_BOTH : State.C1_ONLY;
        currentPosition = c1.encodedPosition();
+        if (c2 != null)
+            currentPosition = Cursor.unionFlags(currentPosition, c2.encodedPosition(), Cursor.FLAGS_MASK);


Maybe add a version of unionFlags to be used when it is known that the position bits match, using a simple or.

Actually I don't think we should call this any other way -- so perhaps do something like

if (DEBUG) assertEquals(0, compare(pos1, pos2)); return pos1 | pos2;

in the method?

If you have the time to experiment, it would also be a good idea to check how costly each version is (with if (DEBUG), with an assertion without the if and assertions enabled (which is how Cassandra is normally run in production), and without the assertion altogether, or the version that masks the flags).

Wrote a simple benchmark here are the results

[junit-timeout] ================================================================================ [junit-timeout] UnionFlags Performance Benchmark Suite [junit-timeout] ================================================================================ [junit-timeout] Assertions enabled: true [junit-timeout] DEBUG flag: false [junit-timeout] Operations per iteration: 1000000 [junit-timeout] Warmup iterations: 5 [junit-timeout] Measurement iterations: 20 [junit-timeout] ================================================================================ [junit-timeout] [junit-timeout] Warming up... [junit-timeout] Warmup complete. [junit-timeout] [junit-timeout] Running benchmarks... [junit-timeout] [junit-timeout] [junit-timeout] ================================================================================ [junit-timeout] RESULTS [junit-timeout] ================================================================================ [junit-timeout] Current Masking Approach : 9,154,872 ns total, 0.46 ns/op, 2,184,629,124 ops/sec [junit-timeout] Simple OR with DEBUG Assertion : 6,673,580 ns total, 0.33 ns/op, 2,996,892,223 ops/sec [junit-timeout] Simple OR with Assertion : 9,689,000 ns total, 0.48 ns/op, 2,064,196,512 ops/sec [junit-timeout] Simple OR (No Assertion) : 6,795,041 ns total, 0.34 ns/op, 2,943,322,932 ops/sec

blambov · 2026-02-19T10:44:41Z

        if (!Cursor.isExhausted(nextPosition))
            prepareNextPosition(currentPosition);
-        return currentPosition;
+        return encodedPosition();


Could we apply the adjustment to currentPosition once here and return it unchanged in encodedPosition?

Could we use the same adjustNextPosition mechanism as SingletonOrderedCursor, which will also simplify the latter?

blambov · 2026-02-19T10:45:32Z

        currentPosition = Cursor.positionForDescentWithByte(pos, current);
        nextPosition = Cursor.exhaustedPosition(currentPosition);
-        return currentPosition;
+        return encodedPosition();


This should always result in reaching the end state, i.e. we can directly add the flag to currentPosition above.

blambov · 2026-02-19T10:51:01Z

                case ROOT:
                case NONE:
                default:
+                    if (!Cursor.isExhausted(encodedPosition))


In the NONE case negation will not change the boundary status of the state (negating a boundary still results in a boundary).
In the ROOT case we have to flip the flag (if a boundary existed we drop it, it one didn't we add it).

blambov · 2026-02-19T10:51:16Z

    class Negated implements TrieSetCursor
    {
        final TrieSetCursor source;
+        final Direction direction;


Is this needed?

no, removed

blambov · 2026-02-19T10:53:35Z

+                    break;
                default:
-                    return checkOverride(source.advance());
+                    checkOverride(source.advance());


checkOverride already calls encodedPosition() when needed.

blambov · 2026-02-20T12:47:54Z

+        // the head alone would not reflect.
+
+        // Optimization: if the head already has all flags set, no need to walk the heap
+        if ((pos & Cursor.FLAGS_MASK) == Cursor.FLAGS_MASK)


For this to be helpful we need to use a mask of only the flags that we can actually set.

blambov · 2026-02-20T12:48:37Z


+    /// Collects and caches the current position by unioning flags from all cursors at the same position.
+    /// This is called after advancing to ensure the position is always up-to-date.
+    private long collectAndCachePosition()


Nit: I would call this collectAndCachePositionFlags().

blambov · 2026-02-20T12:53:42Z

        {
            currentPosition = nextPosition;
-            if (nodeContent != null || isLeaf(fullNode))
+            if (nodeContent != null)


AFAIU the only reason to have this now is the constructor. Let's do this there instead of checking for every advance.

(I was tempted to assert that the flag is as expected, but we already do it in DEBUG mode thus even that is not needed.)

blambov · 2026-02-20T12:57:16Z

                    return Cursor.exhaustedPosition(encodedPosition);
                case ROOT:
+                    // In ROOT case, set flag if negated state is a boundary
+                    if (!Cursor.isExhausted(encodedPosition) && state().isBoundary())


Just do encodedPosition ^= MAY_HAVE_CONTENT_BIT.

The position cannot be exhausted when overriding is ROOT (run the tries package with an assertion if you want to be certain).

!isExhausted() check was redundant but we can't do just encodedPosition ^= MAY_HAVE_CONTENT_BIT
It assumes that the content status of the negated cursor is always the exact opposite of the original cursor's
IIUC then this would be a counter example

Scenario Source Set Source isBoundary()? (content bit) Source succeedingIncluded? Negated Set Needs Boundary? (correct bit) Does source_bit ^ 1 work?

1 ("a", "b") No (0) No Yes (1) Yes (0 ^ 1 = 1)

2 (null, "b") No (0) Yes No (0) No (0 ^ 1 = 1)

Isn't isBoundary() true for the source root in the second example?

blambov · 2026-02-20T12:58:56Z

        @Override
        public RangeState state()
        {
+            Direction dir = direction();


Almost all calls of this method will go through the NONE path. I'd rather we didn't call direction() for them.

Move this to the ROOT case.

blambov · 2026-03-16T13:39:00Z

                    // In ROOT case, set flag if negated state is a boundary
-                    if (!Cursor.isExhausted(encodedPosition) && state().isBoundary())
-                        encodedPosition |= MAY_HAVE_CONTENT_BIT;
+                    if (!Cursor.isExhausted(pos) && state().isBoundary())


Is it really possible that we are in the root state and the source is exhausted?
(By contract a cursor cannot start in an exhausted state.)

Both the source and negated cursors can have boundaries at the ROOT position, then XOR would be incorrect or am I mistaken here? eg.

Negated boundaries: [null, 616263, 616667, 616761, 616a62, null] Negated set Forward: -> START 61 -> CONTAINED 62 -> CONTAINED 63 -> END 66 -> NOT_CONTAINED 67↑ -> START 67 -> CONTAINED 61 -> END 6a -> NOT_CONTAINED 62↑ -> START ↑ -> END Reverse: -> END 61 -> CONTAINED 6a -> CONTAINED 62 -> START 67 -> NOT_CONTAINED 61↑ -> END 66 -> CONTAINED 67 -> START 62 -> NOT_CONTAINED 63↑ -> END ↑ -> START Tail for FORWARD tail bounds [null, 616263, 616667, 616761, 616a62] Forward: java.lang.AssertionError: Non-null content for position without MAY_HAVE_CONTENT_BIT: depth 0 incomingTransition 00 FORWARD TrieSetCursor$Negated pos depth 0 incomingTransition 00 FORWARD at state START

blambov

I think we also want to do the position check whenever we get mutationCursor.content() in all InMemoryTrie mutators.

blambov · 2026-03-18T15:30:44Z

+
                // Always check if we are seeing new content; if we do, that's an easy state update.
-                S content = content();
+                S content = (position & MAY_HAVE_CONTENT_BIT) != 0 ? content() : null;


I don't think we need to do this as content() is just this.content -- maybe inline it here instead?

…lag in trie cursors. This metadata flag allows skipping content lookups for structural nodes and is correctly propagated and unioned across all cursor implementations.

…ursor

blambov · 2026-06-09T11:18:46Z

            }
+
+            // MAY_HAVE_CONTENT_BIT optimization: only call content() if flag indicates potential content
+            if ((currentPosition & MAY_HAVE_CONTENT_BIT) != 0)


I don't think it's a good idea for the deletion branch to be presented before content that covers it. Dumps, for one, look weird with this order.

blambov · 2026-06-09T11:27:07Z

        assertFresh();
-        T content = content();   // handle content on the root node
+        long currentPosition = encodedPosition();
+        T content = (currentPosition & MAY_HAVE_CONTENT_BIT) != 0 ? content() : null;


We should be able to put this in a static Cursor.content(cursor, cursorPosition) method without changing the performance.

We can also add a line in the content() javadoc to say it's preferable to get this via the static. If you prefer, call it Cursor.checkFlagAndGetContent or something similar.

blambov · 2026-06-09T11:33:42Z

    {
        assertFresh();
-        T content = content();   // handle content on the root node
+        long currentPosition = encodedPosition();


How about making assertFresh return the position (which it has to fetch anyway) and rename it to something like getPositionAndAssertFresh?

blambov · 2026-06-09T11:48:54Z

        {
            if (!Cursor.isExhausted(position))
            {
+                currentPosition = position;


AFAICS callers already update currentPosition, this should not be necessary.

blambov · 2026-06-09T11:50:27Z

            if (rootAscentContent != null)
                addBacktrack(NONE, 0, -1);
-            updateActiveAndReturn(encodedPosition());
+            setNodeState(currentPosition, rootDescentContent, currentFullNode, currentNode);


How about

assert currentFullNode == root && currentNode == root; long rootPos = encodedPosition(); if (rootDescentContent != null) setNodeState(rootPos | MAY_HAVE_CONTENT_BIT, rootDescentContent, root, root); updateActiveAndReturn(rootPos);

so that we don't need to change the visibility of currentPosition?

I have messed up this construction a bit, making it do unnecessary work and call updateActiveAndReturn twice; if you prefer, add a separate constructor that doesn't do any of the usual preparation so that we can set the values we need directly.

blambov · 2026-06-09T12:24:11Z

        }

+        @Override
+        public long encodedPosition()


Doesn't the base class already deal with the content flag? What does the deletion branch have to do with it?

blambov · 2026-06-09T12:26:29Z

        {
-            return Cursor.isRootPosition(encodedPosition()) && deletionBranch != null
+            long pos = encodedPosition();
+            return (Cursor.isRootPosition(pos) || Cursor.compare(pos, matchingPositionAtRoot) == 0) && deletionBranch != null


Why are we presenting the deletion branch again at after the prefix?

blambov · 2026-06-09T12:46:46Z

        {
-            if (Cursor.isRootPosition(encodedPosition()))
+            long pos = encodedPosition();
+            if (Cursor.isRootPosition(pos) || Cursor.compare(pos, matchingPositionAtRoot) == 0)


I don't understand this change either. If we are at the root, we should present the deletion branch. If not, we should present the live path only.

There is a problem with the existing code, though, when the input cursor could return deletion branches (e.g. for prefixedBySeparately(prefix, false)) which I'm fixing in the base PR.

blambov · 2026-06-09T13:04:12Z

        if (!Cursor.isExhausted(nextPosition))
            prepareNextPosition(currentPosition);
-        return currentPosition;
+        return encodedPosition();


Could we use the same adjustNextPosition mechanism as SingletonOrderedCursor, which will also simplify the latter?

blambov · 2026-06-09T13:07:34Z

-            }
            else
-                return checkOverride(source.skipTo(encodedSkipPosition));
+                checkOverride(source.skipTo(encodedSkipPosition));


This should return because checkOverride already calls updateCurrentPosition().

lesnik2u commented Feb 18, 2026

View reviewed changes

blambov force-pushed the CNDB-15669 branch from d5dcb5f to e79469c Compare February 18, 2026 16:18

blambov reviewed Feb 19, 2026

View reviewed changes

blambov force-pushed the CNDB-15669 branch 2 times, most recently from ea33ff1 to 290c584 Compare February 20, 2026 10:35

lesnik2u force-pushed the CNDB-15669-mayHaveContent branch 2 times, most recently from 6f452e7 to 3df0ae5 Compare February 20, 2026 12:04

lesnik2u requested a review from blambov February 20, 2026 12:06

blambov reviewed Feb 20, 2026

View reviewed changes

lesnik2u force-pushed the CNDB-15669-mayHaveContent branch from ce51443 to 3d92f1f Compare March 13, 2026 16:01

lesnik2u requested a review from blambov March 13, 2026 16:03

blambov reviewed Mar 16, 2026

View reviewed changes

blambov reviewed Mar 18, 2026

View reviewed changes

blambov force-pushed the CNDB-15669 branch 2 times, most recently from 970213d to 66c45f1 Compare March 23, 2026 12:21

lesnik2u force-pushed the CNDB-15669-mayHaveContent branch from af326a8 to 4df869e Compare March 23, 2026 13:54

lesnik2u requested a review from blambov March 24, 2026 12:05

blambov force-pushed the CNDB-15669 branch 2 times, most recently from a4cd17f to f2e5ca8 Compare March 24, 2026 16:56

lesnik2u force-pushed the CNDB-15669-mayHaveContent branch 3 times, most recently from ac98486 to 0734395 Compare March 27, 2026 12:22

blambov force-pushed the CNDB-15669 branch 3 times, most recently from 952e794 to cfce0f0 Compare April 3, 2026 13:50

blambov force-pushed the CNDB-15669 branch from cfce0f0 to 7901ada Compare April 6, 2026 13:04

blambov force-pushed the CNDB-15669 branch from 7901ada to 5b58b4d Compare April 28, 2026 15:06

lesnik2u added 3 commits May 26, 2026 14:08

CNDB-15669: Implementation of the MAY_HAVE_CONTENT_BIT optimization f…

d381439

…lag in trie cursors. This metadata flag allows skipping content lookups for structural nodes and is correctly propagated and unioned across all cursor implementations.

CNDB-15669: Add MetadataFlagsTest

b04f905

CNDB-15669: fix small nit

13c9f55

lesnik2u added 6 commits May 26, 2026 14:09

CNDB-15669: fix

5e3fdb7

CNDB-15669: Address review feedback

fc08e14

CNDB-15669: adressing review feedback and small optimizations

f0e7739

CNDB-15669: small nit fix

d546b17

CNDB-15669: Added more content guards and fixed xor login in TrieSetC…

7949190

…ursor

CNDB-15669: Fix MetadataFlagsTest after rebasing

2c64c6a

lesnik2u force-pushed the CNDB-15669-mayHaveContent branch from 0734395 to 2c64c6a Compare May 26, 2026 12:23

lesnik2u mentioned this pull request Jun 1, 2026

CNDB-15669: Implement mayHaveDeletionBranch #6

Open

blambov reviewed Jun 9, 2026

View reviewed changes

Scenario	Source Set	Source isBoundary()? (content bit)	Source succeedingIncluded?	Negated Set Needs Boundary? (correct bit)	Does source_bit ^ 1 work?
1	("a", "b")	No (0)	No	Yes (1)	Yes (0 ^ 1 = 1)
2	(null, "b")	No (0)	Yes	No (0)	No (0 ^ 1 = 1)

Conversation

lesnik2u commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blambov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lesnik2u Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blambov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lesnik2u commented Feb 18, 2026 •

edited

Loading

lesnik2u Feb 25, 2026 •

edited

Loading