Implement firstGeq directly

ConflictSet.cpp

@@ -602,6 +602,20 @@ Node *prevPhysical(Node *node) {
   }
 }
 
+Node *nextSibling(Node *node) {
+  for (;;) {
+    if (node->parent == nullptr) {
+      return nullptr;
+    }
+    auto next = getChildGeq(node->parent, node->parentsIndex + 1);
+    if (next < 0) {
+      node = node->parent;
+    } else {
+      return getChildExists(node->parent, next);
+    }
+  }
+}
+
 Node *prevLogical(Node *node) {
   for (node = prevPhysical(node); node != nullptr && !node->entryPresent;
        node = prevPhysical(node))
@@ -614,7 +628,12 @@ struct Iterator {
   int cmp;
 };
 
-std::string_view getSearchPath(Arena &arena, Node *n) {
+namespace {
+std::string getSearchPath(Node *n) {
+  Arena arena;
+  if (n == nullptr) {
+    return "<end>";
+  }
   auto result = vector<char>(arena);
   for (;;) {
     for (int i = n->partialKeyLen - 1; i >= 0; --i) {
@@ -628,124 +647,76 @@ std::string_view getSearchPath(Arena &arena, Node *n) {
   }
   std::reverse(result.begin(), result.end());
   if (result.size() > 0) {
-    return std::string_view((const char *)&result[0],
+    return printable(std::string_view((const char *)&result[0],
-                            result.size()); // NOLINT
+                                      result.size())); // NOLINT
   } else {
-    return std::string_view();
+    return std::string();
   }
 }
-
+} // namespace
-struct StepwiseLastLeq {
-  StepwiseLastLeq() {}
-
-  Node *n;
-  std::span<const uint8_t> remaining;
-  StepwiseLastLeq(Node *n, const std::span<const uint8_t> key)
-      : n(n), remaining(key) {}
-
-  int cmp;
-
-  enum Phase { Search, ScanBackward, DownRightSpine };
-  Phase phase = Search;
-
-  bool step() {
-    switch (phase) {
-    case Search:
-      if (n->partialKeyLen > 0) {
-        int commonLen = std::min<int>(n->partialKeyLen, remaining.size());
-        for (int i = 0; i < commonLen; ++i) {
-          auto c = n->partialKey[i] <=> remaining[i];
-          if (c == 0) {
-            continue;
-          }
-          if (c > 0) {
-            n = prevPhysical(n);
-            phase = ScanBackward;
-            return false;
-          } else {
-            phase = DownRightSpine;
-            return false;
-          }
-        }
-        if (commonLen == n->partialKeyLen) {
-          // partial key matches
-          remaining =
-              remaining.subspan(commonLen, remaining.size() - commonLen);
-        } else if (n->partialKeyLen > int(remaining.size())) {
-          // n is the first physical node greater than remaining, and there's no
-          // eq node
-          n = prevPhysical(n);
-          phase = ScanBackward;
-          return false;
-        }
-      }
-      if (remaining.size() == 0) {
-        // We've found the physical node corresponding to search path `key`
-        if (n->entryPresent) {
-          cmp = 0;
-          return true;
-        } else {
-          phase = ScanBackward;
-          return false;
-        }
-      } else {
-        int c = getChildLeq(n, remaining[0]);
-        if (c == remaining[0]) {
-          n = getChildExists(n, c);
-          remaining = remaining.subspan(1, remaining.size() - 1);
-        } else {
-          if (c >= 0) {
-            n = getChildExists(n, c);
-            phase = DownRightSpine;
-            return false;
-          } else {
-            phase = ScanBackward;
-            return false;
-          }
-        }
-      }
-      return false;
-    case DownRightSpine:
-      // The physical node corresponding to search path `key` does not
-      // exist. Let's find the physical node corresponding to the highest
-      // search key (not necessarily present) less than key.
-      // Move down the right spine
-      {
-        int c = getChildLeq(n, 255);
-        if (c >= 0) {
-          n = getChildExists(n, c);
-        } else {
-          phase = ScanBackward;
-        }
-        return false;
-      }
-    case ScanBackward:
-      // Iterate backwards along existing physical nodes until we find a present
-      // entry
-      if (!n->entryPresent) {
-        n = prevPhysical(n);
-        return false;
-      }
-      cmp = -1;
-      return true;
-    }
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
-  }
-};
-
-Iterator lastLeq(Node *n, const std::span<const uint8_t> key) {
-  StepwiseLastLeq l{n, key};
-  while (!l.step())
-    ;
-  return {l.n, l.cmp};
-}
 
 Iterator firstGeq(Node *n, const std::span<const uint8_t> key) {
-  auto result = lastLeq(n, key);
+  auto remaining = key;
-  if (result.cmp == 0) {
+  for (;;) {
-    return result;
+    if (n->partialKeyLen > 0) {
+      int commonLen = std::min<int>(n->partialKeyLen, remaining.size());
+      for (int i = 0; i < commonLen; ++i) {
+        auto c = n->partialKey[i] <=> remaining[i];
+        if (c == 0) {
+          continue;
+        }
+        if (c > 0) {
+          goto downLeftSpine;
+        } else {
+          n = nextSibling(n);
+          goto downLeftSpine;
+        }
+      }
+      if (commonLen == n->partialKeyLen) {
+        // partial key matches
+        remaining = remaining.subspan(commonLen, remaining.size() - commonLen);
+      } else if (n->partialKeyLen > int(remaining.size())) {
+        // n is the first physical node greater than remaining, and there's no
+        // eq node
+        goto downLeftSpine;
+      }
+    }
+    if (remaining.size() == 0) {
+      if (n->entryPresent) {
+        return {n, 0};
+      }
+      int c = getChildGeq(n, 0);
+      assert(c >= 0);
+      n = getChildExists(n, c);
+      goto downLeftSpine;
+    } else {
+      int c = getChildGeq(n, remaining[0]);
+      if (c == remaining[0]) {
+        n = getChildExists(n, c);
+        remaining = remaining.subspan(1, remaining.size() - 1);
+      } else {
+        if (c >= 0) {
+          n = getChildExists(n, c);
+          goto downLeftSpine;
+        } else {
+          n = nextSibling(n);
+          goto downLeftSpine;
+        }
+      }
+    }
+  }
+downLeftSpine:
+  if (n == nullptr) {
+    return {nullptr, 1};
+  }
+  for (;;) {
+    if (n->entryPresent) {
+      return {n, 1};
+    }
+    int c = getChildGeq(n, 0);
+    assert(c >= 0);
+    n = getChildExists(n, c);
   }
-  return {nextLogical(result.n), 1};
 }
 
 // Returns a pointer to the newly inserted node.  caller is reponsible for
@@ -837,6 +808,14 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
                                        reads[i].begin.p, reads[i].begin.len));
         auto right = firstGeq(
             root, std::span<const uint8_t>(reads[i].end.p, reads[i].end.len));
+#if DEBUG_VERBOSE && !defined(NDEBUG)
+        fprintf(stderr, "firstGeq for `%s' got `%s'\n",
+                printable(reads[i].begin).c_str(),
+                getSearchPath(left.n).c_str());
+        fprintf(stderr, "firstGeq for `%s' got `%s'\n",
+                printable(reads[i].end).c_str(),
+                getSearchPath(right.n).c_str());
+#endif
         if (left.n != nullptr && left.cmp != 0 &&
             left.n->entry.rangeVersion > reads[i].readVersion) {
           result[i] = Conflict;
@@ -1069,10 +1048,9 @@ void checkParentPointers(Node *node, bool &success) {
   for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
     auto *child = getChildExists(node, i);
     if (child->parent != node) {
-      Arena arena;
       fprintf(stderr, "%s child %d has parent pointer %p. Expected %p\n",
-              printable(getSearchPath(arena, node)).c_str(), i,
+              getSearchPath(node).c_str(), i, (void *)child->parent,
-              (void *)child->parent, (void *)node);
+              (void *)node);
       success = false;
     }
     checkParentPointers(child, success);
@@ -1089,10 +1067,8 @@ void checkParentPointers(Node *node, bool &success) {
     expected = std::max(expected, checkMaxVersion(child, success));
   }
   if (node->maxVersion != expected) {
-    Arena arena;
     fprintf(stderr, "%s has max version %" PRId64 " . Expected %" PRId64 "\n",
-            printable(getSearchPath(arena, node)).c_str(), node->maxVersion,
+            getSearchPath(node).c_str(), node->maxVersion, expected);
-            expected);
     success = false;
   }
   return expected;
@@ -1107,7 +1083,7 @@ void checkParentPointers(Node *node, bool &success) {
     if (e == 0) {
       Arena arena;
       fprintf(stderr, "%s has child %02x with no reachable entries\n",
-              printable(getSearchPath(arena, node)).c_str(), i);
+              getSearchPath(node).c_str(), i);
       success = false;
     }
   }