Disable skip list tests by default

Would be great to find a way to make this all more readable

CMakeLists.txt

@@ -182,10 +182,13 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
   target_compile_options(driver_skip_list PRIVATE ${TEST_FLAGS})
   target_link_libraries(driver_skip_list PRIVATE skip_list)
 
+  # enable to test skip list
+  if(0)
     foreach(TEST ${CORPUS_TESTS})
       get_filename_component(hash ${TEST} NAME)
       add_test(NAME skip_list_${hash} COMMAND driver_skip_list ${TEST})
     endforeach()
+  endif()
 
   # ad hoc testing
   add_executable(conflict_set_main ConflictSet.cpp)

ConflictSet.cpp

@@ -2488,102 +2488,6 @@ downLeftSpine:
   return n->entry.rangeVersion <= readVersion;
 }
 
-namespace {
-// Return true if the max version among all keys that start with key[:prefixLen]
-// that are < key is <= readVersion
-bool checkRangeRightSide(Node *n, std::span<const uint8_t> key, int prefixLen,
-                         InternalVersionT readVersion, ReadContext *tls) {
-  auto remaining = key;
-  int searchPathLen = 0;
-
-  for (;; ++tls->range_read_iterations_accum) {
-    assert(searchPathLen <= int(key.size()));
-    if (remaining.size() == 0) {
-      goto downLeftSpine;
-    }
-
-    if (searchPathLen >= prefixLen) {
-      if (n->entryPresent && n->entry.pointVersion > readVersion) {
-        return false;
-      }
-
-      if (!checkMaxBetweenExclusive(n, -1, remaining[0], readVersion, tls)) {
-        return false;
-      }
-    }
-
-    if (searchPathLen > prefixLen && n->entryPresent &&
-        n->entry.rangeVersion > readVersion) {
-      return false;
-    }
-
-    Node *child = getChild(n, remaining[0]);
-    if (child == nullptr) {
-      auto c = getChildGeq(n, remaining[0]);
-      if (c != nullptr) {
-        n = c;
-        goto downLeftSpine;
-      } else {
-        goto backtrack;
-      }
-    }
-
-    n = child;
-    remaining = remaining.subspan(1, remaining.size() - 1);
-    ++searchPathLen;
-
-    if (n->partialKeyLen > 0) {
-      int commonLen = std::min<int>(n->partialKeyLen, remaining.size());
-      int i = longestCommonPrefix(n->partialKey(), remaining.data(), commonLen);
-      searchPathLen += i;
-      if (i < commonLen) {
-        ++searchPathLen;
-        auto c = n->partialKey()[i] <=> remaining[i];
-        if (c > 0) {
-          goto downLeftSpine;
-        } else {
-          if (searchPathLen > prefixLen && n->entryPresent &&
-              n->entry.rangeVersion > readVersion) {
-            return false;
-          }
-          goto backtrack;
-        }
-      }
-      if (commonLen == n->partialKeyLen) {
-        // partial key matches
-        remaining = remaining.subspan(commonLen, remaining.size() - commonLen);
-      } else if (n->partialKeyLen > int(remaining.size())) {
-        goto downLeftSpine;
-      }
-    }
-  }
-backtrack:
-  for (;;) {
-    // searchPathLen > prefixLen implies n is not the root
-    if (searchPathLen > prefixLen && maxVersion(n) > readVersion) {
-      return false;
-    }
-    if (n->parent == nullptr) {
-      return true;
-    }
-    auto next = getChildGeq(n->parent, n->parentsIndex + 1);
-    if (next == nullptr) {
-      searchPathLen -= 1 + n->partialKeyLen;
-      n = n->parent;
-    } else {
-      searchPathLen -= n->partialKeyLen;
-      n = next;
-      searchPathLen += n->partialKeyLen;
-      goto downLeftSpine;
-    }
-  }
-downLeftSpine:
-  for (; !n->entryPresent; n = getFirstChildExists(n)) {
-  }
-  return n->entry.rangeVersion <= readVersion;
-}
-} // namespace
-
 #ifdef __x86_64__
 // Explicitly instantiate with target avx512f attribute so the compiler can
 // inline compare16_32bit_avx512, and generally use avx512f within more
@@ -2990,14 +2894,22 @@ PRESERVE_NONE void complete(CheckJob *job, CheckContext *context) {
   MUSTTAIL return keepGoing(job, context);
 }
 
+PRESERVE_NONE void down_left_spine(CheckJob *job, CheckContext *context) {
+  if (job->n->entryPresent) {
+    job->setResult(job->n->entry.rangeVersion <= job->readVersion);
+    MUSTTAIL return complete(job, context);
+  }
+  job->n = getFirstChildExists(job->n);
+  __builtin_prefetch(job->n);
+  MUSTTAIL return keepGoing(job, context);
+}
+
 namespace check_point_read_state_machine {
 
 PRESERVE_NONE void begin(CheckJob *, CheckContext *);
 
 template <class NodeT> PRESERVE_NONE void iter(CheckJob *, CheckContext *);
 
-PRESERVE_NONE void down_left_spine(CheckJob *, CheckContext *);
-
 static Continuation iterTable[] = {iter<Node0>, iter<Node3>, iter<Node16>,
                                    iter<Node48>, iter<Node256>};
 
@@ -3118,16 +3030,6 @@ template <class NodeT> void iter(CheckJob *job, CheckContext *context) {
   MUSTTAIL return keepGoing(job, context);
 }
 
-void down_left_spine(CheckJob *job, CheckContext *context) {
-  if (job->n->entryPresent) {
-    job->setResult(job->n->entry.rangeVersion <= job->readVersion);
-    MUSTTAIL return complete(job, context);
-  }
-  job->n = getFirstChildExists(job->n);
-  __builtin_prefetch(job->n);
-  MUSTTAIL return keepGoing(job, context);
-}
-
 } // namespace check_point_read_state_machine
 
 namespace check_prefix_read_state_machine {
@@ -3136,8 +3038,6 @@ PRESERVE_NONE void begin(CheckJob *, CheckContext *);
 
 template <class NodeT> PRESERVE_NONE void iter(CheckJob *, CheckContext *);
 
-PRESERVE_NONE void down_left_spine(CheckJob *, CheckContext *);
-
 static Continuation iterTable[] = {iter<Node0>, iter<Node3>, iter<Node16>,
                                    iter<Node48>, iter<Node256>};
 
@@ -3253,16 +3153,6 @@ template <class NodeT> void iter(CheckJob *job, CheckContext *context) {
   MUSTTAIL return keepGoing(job, context);
 }
 
-void down_left_spine(CheckJob *job, CheckContext *context) {
-  if (job->n->entryPresent) {
-    job->setResult(job->n->entry.rangeVersion <= job->readVersion);
-    MUSTTAIL return complete(job, context);
-  }
-  job->n = getFirstChildExists(job->n);
-  __builtin_prefetch(job->n);
-  MUSTTAIL return keepGoing(job, context);
-}
-
 } // namespace check_prefix_read_state_machine
 
 namespace check_range_read_state_machine {
@@ -3278,21 +3168,23 @@ static Continuation commonPrefixIterTable[] = {
     common_prefix_iter<Node16>, common_prefix_iter<Node48>,
     common_prefix_iter<Node256>};
 
-template <class NodeT, bool kFirst>
+template <class NodeT>
 PRESERVE_NONE void left_side_iter(CheckJob *, CheckContext *);
 
 PRESERVE_NONE void left_side_down_left_spine(CheckJob *, CheckContext *);
 
 PRESERVE_NONE void done_left_side_iter(CheckJob *, CheckContext *);
 
-static Continuation leftSideIterTable[2][5] = {
-    {left_side_iter<Node0, false>, left_side_iter<Node3, false>,
-     left_side_iter<Node16, false>, left_side_iter<Node48, false>,
-     left_side_iter<Node256, false>},
-    {left_side_iter<Node0, true>, left_side_iter<Node3, true>,
-     left_side_iter<Node16, true>, left_side_iter<Node48, true>,
-     left_side_iter<Node256, true>},
-};
+static Continuation leftSideIterTable[] = {
+    left_side_iter<Node0>, left_side_iter<Node3>, left_side_iter<Node16>,
+    left_side_iter<Node48>, left_side_iter<Node256>};
+
+template <class NodeT>
+PRESERVE_NONE void right_side_iter(CheckJob *, CheckContext *);
+
+static Continuation rightSideIterTable[] = {
+    right_side_iter<Node0>, right_side_iter<Node3>, right_side_iter<Node16>,
+    right_side_iter<Node48>, right_side_iter<Node256>};
 
 PRESERVE_NONE void begin(CheckJob *job, CheckContext *context) {
   job->lcp = longestCommonPrefix(job->begin.data(), job->end.data(),
@@ -3393,11 +3285,49 @@ PRESERVE_NONE void done_common_prefix_iter(CheckJob *job,
   job->commonPrefixNode = job->n;
 
   if (job->lcp == int(job->begin.size())) {
-    job->setResult(checkRangeRightSide(job->n, job->end, job->lcp,
-                                       job->readVersion, context->tls));
+    job->remaining = job->end;
+    if (job->lcp == 0) {
+      if (job->n->entryPresent &&
+          job->n->entry.pointVersion > job->readVersion) {
+        job->setResult(false);
         MUSTTAIL return complete(job, context);
       }
 
+      if (!checkMaxBetweenExclusive(job->n, -1, job->remaining[0],
+                                    job->readVersion, context->tls)) {
+        job->setResult(false);
+        MUSTTAIL return complete(job, context);
+      }
+    }
+
+    // This is a hack
+    --job->lcp;
+
+    auto c = getChild(job->n, job->remaining[0]);
+    Node *child = c;
+    if (child == nullptr) {
+      auto c = getChildGeq(job->n, job->remaining[0]);
+      if (c != nullptr) {
+        job->n = c;
+        job->continuation = down_left_spine;
+        MUSTTAIL return job->continuation(job, context);
+      } else {
+        job->n = nextSibling(job->n);
+        if (job->n == nullptr) {
+          job->setResult(true);
+          MUSTTAIL return complete(job, context);
+        }
+        job->continuation = down_left_spine;
+        MUSTTAIL return job->continuation(job, context);
+      }
+    }
+
+    job->n = child;
+    job->continuation = rightSideIterTable[c.getType()];
+    __builtin_prefetch(job->n);
+    MUSTTAIL return keepGoing(job, context);
+  }
+
   // If this were not true we would have returned above
   assert(job->begin.size() > 0);
 
@@ -3433,14 +3363,14 @@ PRESERVE_NONE void done_common_prefix_iter(CheckJob *job,
   }
 
   job->n = child;
-  job->continuation = leftSideIterTable[true][c.getType()];
+  job->continuation = leftSideIterTable[c.getType()];
   __builtin_prefetch(job->n);
   MUSTTAIL return keepGoing(job, context);
 }
 
 // Return true if the max version among all keys that start with key[:prefixLen]
 // that are >= key is <= readVersion
-template <class NodeT, bool kFirst>
+template <class NodeT>
 PRESERVE_NONE void left_side_iter(CheckJob *job, CheckContext *context) {
   assert(NodeT::kType == job->n->getType());
   NodeT *n = static_cast<NodeT *>(job->n);
@@ -3454,7 +3384,7 @@ PRESERVE_NONE void left_side_iter(CheckJob *job, CheckContext *context) {
     if (i < commonLen) {
       auto c = n->partialKey()[i] <=> job->remaining[i];
       if (c > 0) {
-        if constexpr (kFirst) {
+        if (n->parent == job->commonPrefixNode) {
           if (i < job->lcp) {
             job->continuation = left_side_down_left_spine;
             MUSTTAIL return job->continuation(job, context);
@@ -3539,18 +3469,40 @@ PRESERVE_NONE void left_side_iter(CheckJob *job, CheckContext *context) {
   }
 
   job->n = child;
-  job->continuation = leftSideIterTable[false][c.getType()];
+  job->continuation = leftSideIterTable[c.getType()];
   __builtin_prefetch(job->n);
   MUSTTAIL return keepGoing(job, context);
 }
 
 PRESERVE_NONE void done_left_side_iter(CheckJob *job, CheckContext *context) {
 
-  job->setResult(checkRangeRightSide(job->commonPrefixNode, job->end,
-                                     job->lcp + 1, job->readVersion,
-                                     context->tls));
+  job->n = job->commonPrefixNode;
+  job->remaining = job->end;
+
+  auto c = getChild(job->n, job->remaining[0]);
+  Node *child = c;
+  if (child == nullptr) {
+    auto c = getChildGeq(job->n, job->remaining[0]);
+    if (c != nullptr) {
+      job->n = c;
+      job->continuation = down_left_spine;
+      MUSTTAIL return job->continuation(job, context);
+    } else {
+      job->n = nextSibling(job->n);
+      if (job->n == nullptr) {
+        job->setResult(true);
         MUSTTAIL return complete(job, context);
       }
+      job->continuation = down_left_spine;
+      MUSTTAIL return job->continuation(job, context);
+    }
+  }
+
+  job->n = child;
+  job->continuation = rightSideIterTable[c.getType()];
+  __builtin_prefetch(job->n);
+  MUSTTAIL return keepGoing(job, context);
+}
 
 void left_side_down_left_spine(CheckJob *job, CheckContext *context) {
   if (job->n->entryPresent) {
@@ -3566,6 +3518,102 @@ void left_side_down_left_spine(CheckJob *job, CheckContext *context) {
   MUSTTAIL return keepGoing(job, context);
 }
 
+// Return true if the max version among all keys that start with key[:prefixLen]
+// that are < key is <= readVersion
+template <class NodeT>
+PRESERVE_NONE void right_side_iter(CheckJob *job, CheckContext *context) {
+  assert(NodeT::kType == job->n->getType());
+  NodeT *n = static_cast<NodeT *>(job->n);
+
+  job->remaining = job->remaining.subspan(1, job->remaining.size() - 1);
+
+  if (n->partialKeyLen > 0) {
+    int commonLen = std::min<int>(n->partialKeyLen, job->remaining.size());
+    int i =
+        longestCommonPrefix(n->partialKey(), job->remaining.data(), commonLen);
+    if (i < commonLen) {
+      auto c = n->partialKey()[i] <=> job->remaining[i];
+      if (c > 0) {
+        job->continuation = down_left_spine;
+        MUSTTAIL return job->continuation(job, context);
+      } else {
+        if ((n->parent != job->commonPrefixNode || i >= job->lcp) &&
+            n->entryPresent && n->entry.rangeVersion > job->readVersion) {
+          job->setResult(false);
+          MUSTTAIL return complete(job, context);
+        }
+        if ((n->parent != job->commonPrefixNode || i >= job->lcp) &&
+            maxVersion(n) > job->readVersion) {
+          job->setResult(false);
+          MUSTTAIL return complete(job, context);
+        }
+        job->n = nextSibling(job->n);
+        if (job->n == nullptr) {
+          job->setResult(true);
+          MUSTTAIL return complete(job, context);
+        }
+        job->continuation = down_left_spine;
+        MUSTTAIL return job->continuation(job, context);
+      }
+    }
+    if (commonLen == n->partialKeyLen) {
+      // partial key matches
+      job->remaining =
+          job->remaining.subspan(commonLen, job->remaining.size() - commonLen);
+    } else if (n->partialKeyLen > int(job->remaining.size())) {
+      job->continuation = down_left_spine;
+      MUSTTAIL return job->continuation(job, context);
+    }
+  }
+
+  ++context->tls->range_read_iterations_accum;
+
+  if (job->remaining.size() == 0) {
+    job->continuation = down_left_spine;
+    MUSTTAIL return job->continuation(job, context);
+  }
+
+  if (n->entryPresent && (n->entry.pointVersion > job->readVersion ||
+                          n->entry.rangeVersion > job->readVersion)) {
+    job->setResult(false);
+    MUSTTAIL return complete(job, context);
+  }
+
+  if (!checkMaxBetweenExclusive(n, -1, job->remaining[0], job->readVersion,
+                                context->tls)) {
+    job->setResult(false);
+    MUSTTAIL return complete(job, context);
+  }
+
+  auto c = getChild(job->n, job->remaining[0]);
+  Node *child = c;
+  if (child == nullptr) {
+    auto c = getChildGeq(n, job->remaining[0]);
+    if (c != nullptr) {
+      job->n = c;
+      job->continuation = down_left_spine;
+      MUSTTAIL return job->continuation(job, context);
+    } else {
+      if (n != job->commonPrefixNode && maxVersion(n) > job->readVersion) {
+        job->setResult(false);
+        MUSTTAIL return complete(job, context);
+      }
+      job->n = nextSibling(job->n);
+      if (job->n == nullptr) {
+        job->setResult(true);
+        MUSTTAIL return complete(job, context);
+      }
+      job->continuation = down_left_spine;
+      MUSTTAIL return job->continuation(job, context);
+    }
+  }
+
+  job->n = child;
+  job->continuation = rightSideIterTable[c.getType()];
+  __builtin_prefetch(job->n);
+  MUSTTAIL return keepGoing(job, context);
+}
+
 } // namespace check_range_read_state_machine
 
 void CheckJob::init(const ConflictSet::ReadRange *read,