Stop applying flatten attribute

This blows up the code size too much. I wonder if having all those
distinct possible program counters helps libfuzzer /shrug

Bench.cpp

@@ -332,18 +332,24 @@ void benchWorstCaseForRadixRangeRead() {
   auto end = std::vector<uint8_t>(kKeyLenForWorstCase - 1, 255);
   end.push_back(254);
 
-  weaselab::ConflictSet::Result result;
+  weaselab::ConflictSet::ReadRange r[] = {
-  weaselab::ConflictSet::ReadRange r{
+      {{begin.data(), int(begin.size())}, {end.data(), int(end.size())}, 0},
-      {begin.data(), int(begin.size())}, {end.data(), int(end.size())}, 0};
+  };
+  weaselab::ConflictSet::Result results[sizeof(r) / sizeof(r[0])];
+  for (auto &result : results) {
+    result = weaselab::ConflictSet::TooOld;
+  }
+  bench.batch(sizeof(r) / sizeof(r[0]));
 
   bench.run("worst case for radix tree", [&]() {
     for (int i = 0; i < 256; ++i) {
-      result = weaselab::ConflictSet::TooOld;
+      cs[i]->check(r, results, sizeof(r) / sizeof(r[0]));
-      cs[i]->check(&r, &result, 1);
+      for (auto result : results) {
         if (result != weaselab::ConflictSet::Commit) {
           abort();
         }
       }
+    }
   });
 
   // for (int i = 0; i < 256; ++i) {

ConflictSet.cpp

@@ -2489,91 +2489,6 @@ downLeftSpine:
 }
 
 namespace {
-// Return true if the max version among all keys that start with key[:prefixLen]
-// that are >= key is <= readVersion
-bool checkRangeLeftSide(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) {
-    if (remaining.size() == 0) {
-      assert(searchPathLen >= prefixLen);
-      return maxVersion(n) <= readVersion;
-    }
-
-    if (searchPathLen >= prefixLen) {
-      if (!checkMaxBetweenExclusive(n, remaining[0], 256, readVersion, tls)) {
-        return false;
-      }
-    }
-
-    auto [c, maxV] = getChildAndMaxVersion(n, remaining[0]);
-    Node *child = c;
-    if (child == nullptr) {
-      auto c = getChildGeq(n, remaining[0]);
-      if (c != nullptr) {
-        if (searchPathLen < prefixLen) {
-          n = c;
-          goto downLeftSpine;
-        }
-        n = c;
-        return maxVersion(n) <= readVersion;
-      } else {
-        n = nextSibling(n);
-        if (n == nullptr) {
-          return true;
-        }
-        goto downLeftSpine;
-      }
-    }
-
-    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) {
-        auto c = n->partialKey()[i] <=> remaining[i];
-        if (c > 0) {
-          if (searchPathLen < prefixLen) {
-            goto downLeftSpine;
-          }
-          if (n->entryPresent && n->entry.rangeVersion > readVersion) {
-            return false;
-          }
-          return maxVersion(n) <= readVersion;
-        } else {
-          n = nextSibling(n);
-          if (n == nullptr) {
-            return true;
-          }
-          goto downLeftSpine;
-        }
-      }
-      if (commonLen == n->partialKeyLen) {
-        // partial key matches
-        remaining = remaining.subspan(commonLen, remaining.size() - commonLen);
-      } else if (n->partialKeyLen > int(remaining.size())) {
-        assert(searchPathLen >= prefixLen);
-        if (n->entryPresent && n->entry.rangeVersion > readVersion) {
-          return false;
-        }
-        return maxVersion(n) <= readVersion;
-      }
-    }
-    if (maxV <= readVersion) {
-      return true;
-    }
-  }
-downLeftSpine:
-  for (; !n->entryPresent; n = getFirstChildExists(n)) {
-  }
-  return n->entry.rangeVersion <= readVersion;
-}
-
 // 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,
@@ -2669,69 +2584,6 @@ downLeftSpine:
 }
 } // namespace
 
-bool checkRangeRead(int lcp, Node *n, std::span<const uint8_t> begin,
-                    std::span<const uint8_t> end, InternalVersionT readVersion,
-                    ReadContext *tls) {
-  ++tls->range_read_accum;
-
-  auto remaining = begin.subspan(0, lcp);
-  Arena arena;
-
-  // Advance down common prefix, but stay on a physical path in the tree
-  for (;; ++tls->range_read_iterations_accum) {
-    assert(getSearchPath(arena, n) <=>
-               begin.subspan(0, lcp - remaining.size()) ==
-           0);
-    if (remaining.size() == 0) {
-      break;
-    }
-    auto [c, v] = getChildAndMaxVersion(n, remaining[0]);
-    Node *child = c;
-    if (child == nullptr) {
-      break;
-    }
-
-    if (child->partialKeyLen > 0) {
-      int cl = std::min<int>(child->partialKeyLen, remaining.size() - 1);
-      int i =
-          longestCommonPrefix(child->partialKey(), remaining.data() + 1, cl);
-      if (i != child->partialKeyLen) {
-        break;
-      }
-    }
-    if (v <= readVersion) {
-      ++tls->range_read_short_circuit_accum;
-      return true;
-    }
-    n = child;
-    remaining =
-        remaining.subspan(1 + child->partialKeyLen,
-                          remaining.size() - (1 + child->partialKeyLen));
-  }
-  assert(getSearchPath(arena, n) <=> begin.subspan(0, lcp - remaining.size()) ==
-         0);
-
-  const int consumed = lcp - remaining.size();
-  assume(consumed >= 0);
-
-  begin = begin.subspan(consumed, int(begin.size()) - consumed);
-  end = end.subspan(consumed, int(end.size()) - consumed);
-  lcp -= consumed;
-
-  if (lcp == int(begin.size())) {
-    return checkRangeRightSide(n, end, lcp, readVersion, tls);
-  }
-
-  // This makes it safe to check maxVersion within checkRangeLeftSide. If this
-  // were false, then we would have returned above since lcp == begin.size().
-  assert(!(n->parent == nullptr && begin.size() == 0));
-
-  return checkRangeStartsWith(n, begin.subspan(0, lcp), begin[lcp], end[lcp],
-                              readVersion, tls) &&
-         checkRangeLeftSide(n, begin, lcp + 1, readVersion, tls) &&
-         checkRangeRightSide(n, end, lcp + 1, readVersion, tls);
-}
-
 #ifdef __x86_64__
 // Explicitly instantiate with target avx512f attribute so the compiler can
 // inline compare16_32bit_avx512, and generally use avx512f within more
@@ -3067,12 +2919,6 @@ Node *firstGeqPhysical(Node *n, const std::span<const uint8_t> key) {
 #define PRESERVE_NONE
 #endif
 
-#if __has_attribute(flatten)
-#define FLATTEN __attribute__((flatten))
-#else
-#define FLATTEN
-#endif
-
 typedef PRESERVE_NONE void (*Continuation)(struct CheckJob *,
                                            struct CheckContext *);
 
@@ -3088,6 +2934,12 @@ struct CheckJob {
   Node *n;
   std::span<const uint8_t> begin;
   std::span<const uint8_t> end;       // range read only
+  std::span<const uint8_t> remaining; // range read only
+  Node *child;                        // range read only
+  int lcp;                            // range read only
+  int prefixLen;                      // range read only
+  int searchPathLen;                  // range read only
+  Node *commonPrefixNode;             // range read only
   InternalVersionT readVersion;
   ConflictSet::Result *result;
   Continuation continuation;
@@ -3110,7 +2962,7 @@ struct CheckContext {
 #endif
 };
 
-FLATTEN PRESERVE_NONE void keepGoing(CheckJob *job, CheckContext *context) {
+PRESERVE_NONE void keepGoing(CheckJob *job, CheckContext *context) {
 #if __has_attribute(musttail)
   job = job->next;
   MUSTTAIL return job->continuation(job, context);
@@ -3120,7 +2972,7 @@ FLATTEN PRESERVE_NONE void keepGoing(CheckJob *job, CheckContext *context) {
 #endif
 }
 
-FLATTEN PRESERVE_NONE void complete(CheckJob *job, CheckContext *context) {
+PRESERVE_NONE void complete(CheckJob *job, CheckContext *context) {
   if (context->started == context->count) {
     if (job->prev == job) {
 #if !__has_attribute(musttail)
@@ -3141,12 +2993,11 @@ FLATTEN PRESERVE_NONE void complete(CheckJob *job, CheckContext *context) {
 
 namespace check_point_read_state_machine {
 
-FLATTEN PRESERVE_NONE void begin(CheckJob *, CheckContext *);
+PRESERVE_NONE void begin(CheckJob *, CheckContext *);
 
-template <class NodeT>
+template <class NodeT> PRESERVE_NONE void iter(CheckJob *, CheckContext *);
-FLATTEN PRESERVE_NONE void iter(CheckJob *, CheckContext *);
 
-FLATTEN PRESERVE_NONE void down_left_spine(CheckJob *, CheckContext *);
+PRESERVE_NONE void down_left_spine(CheckJob *, CheckContext *);
 
 static Continuation iterTable[] = {iter<Node0>, iter<Node3>, iter<Node16>,
                                    iter<Node48>, iter<Node256>};
@@ -3274,12 +3125,11 @@ void down_left_spine(CheckJob *job, CheckContext *context) {
 
 namespace check_prefix_read_state_machine {
 
-FLATTEN PRESERVE_NONE void begin(CheckJob *, CheckContext *);
+PRESERVE_NONE void begin(CheckJob *, CheckContext *);
 
-template <class NodeT>
+template <class NodeT> PRESERVE_NONE void iter(CheckJob *, CheckContext *);
-FLATTEN PRESERVE_NONE void iter(CheckJob *, CheckContext *);
 
-FLATTEN PRESERVE_NONE void down_left_spine(CheckJob *, CheckContext *);
+PRESERVE_NONE void down_left_spine(CheckJob *, CheckContext *);
 
 static Continuation iterTable[] = {iter<Node0>, iter<Node3>, iter<Node16>,
                                    iter<Node48>, iter<Node256>};
@@ -3401,18 +3251,39 @@ void down_left_spine(CheckJob *job, CheckContext *context) {
 } // namespace check_prefix_read_state_machine
 
 namespace check_range_read_state_machine {
-FLATTEN PRESERVE_NONE void begin(CheckJob *, CheckContext *);
+PRESERVE_NONE void begin(CheckJob *, CheckContext *);
 
-FLATTEN PRESERVE_NONE void begin(CheckJob *job, CheckContext *context) {
+template <class NodeT>
-  int lcp = longestCommonPrefix(job->begin.data(), job->end.data(),
+PRESERVE_NONE void common_prefix_iter(CheckJob *, CheckContext *);
+
+PRESERVE_NONE void done_common_prefix_iter(CheckJob *, CheckContext *);
+
+static Continuation commonPrefixIterTable[] = {
+    common_prefix_iter<Node0>, common_prefix_iter<Node3>,
+    common_prefix_iter<Node16>, common_prefix_iter<Node48>,
+    common_prefix_iter<Node256>};
+
+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[] = {
+    left_side_iter<Node0>, left_side_iter<Node3>, left_side_iter<Node16>,
+    left_side_iter<Node48>, left_side_iter<Node256>};
+
+PRESERVE_NONE void begin(CheckJob *job, CheckContext *context) {
+  job->lcp = longestCommonPrefix(job->begin.data(), job->end.data(),
                                  std::min(job->begin.size(), job->end.size()));
-  if (lcp == int(job->begin.size()) &&
+  if (job->lcp == int(job->begin.size()) &&
       job->end.size() == job->begin.size() + 1 && job->end.back() == 0) {
     job->continuation = check_point_read_state_machine::begin;
     // Call directly since we have nothing to prefetch
     MUSTTAIL return job->continuation(job, context);
   }
-  if (lcp == int(job->begin.size() - 1) &&
+  if (job->lcp == int(job->begin.size() - 1) &&
       job->end.size() == job->begin.size() &&
       int(job->begin.back()) + 1 == int(job->end.back())) {
     job->continuation = check_prefix_read_state_machine::begin;
@@ -3420,11 +3291,295 @@ FLATTEN PRESERVE_NONE void begin(CheckJob *job, CheckContext *context) {
     MUSTTAIL return job->continuation(job, context);
   }
 
-  *job->result = checkRangeRead(lcp, job->n, job->begin, job->end,
+  ++context->tls->range_read_accum;
+  job->remaining = job->begin.subspan(0, job->lcp);
+
+  if (job->remaining.size() == 0) {
+    MUSTTAIL return done_common_prefix_iter(job, context);
+  }
+
+  auto c = getChild(job->n, job->remaining[0]);
+  job->child = c;
+  if (job->child == nullptr) {
+    MUSTTAIL return done_common_prefix_iter(job, context);
+  }
+
+  job->continuation = commonPrefixIterTable[c.getType()];
+  __builtin_prefetch(job->child);
+  MUSTTAIL return keepGoing(job, context);
+}
+
+// Advance down common prefix, but stay on a physical path in the tree
+template <class NodeT>
+void common_prefix_iter(CheckJob *job, CheckContext *context) {
+
+  assert(NodeT::kType == job->child->getType());
+  NodeT *child = static_cast<NodeT *>(job->child);
+
+  if (child->partialKeyLen > 0) {
+    int cl = std::min<int>(child->partialKeyLen, job->remaining.size() - 1);
+    int i =
+        longestCommonPrefix(child->partialKey(), job->remaining.data() + 1, cl);
+    if (i != child->partialKeyLen) {
+      MUSTTAIL return done_common_prefix_iter(job, context);
+    }
+  }
+  job->n = child;
+  job->remaining = job->remaining.subspan(1 + child->partialKeyLen,
+                                          job->remaining.size() -
+                                              (1 + child->partialKeyLen));
+
+  ++context->tls->range_read_iterations_accum;
+
+  if (job->remaining.size() == 0) {
+    MUSTTAIL return done_common_prefix_iter(job, context);
+  }
+
+  auto c = getChild(child, job->remaining[0]);
+  job->child = c;
+  if (job->child == nullptr) {
+    MUSTTAIL return done_common_prefix_iter(job, context);
+  }
+
+  job->continuation = commonPrefixIterTable[c.getType()];
+  __builtin_prefetch(job->child);
+  MUSTTAIL return keepGoing(job, context);
+}
+
+PRESERVE_NONE void done_common_prefix_iter(CheckJob *job,
+                                           CheckContext *context) {
+
+  {
+    Arena arena;
+    assert(getSearchPath(arena, job->n) <=>
+               job->begin.subspan(0, job->lcp - job->remaining.size()) ==
+           0);
+  }
+
+  const int consumed = job->lcp - job->remaining.size();
+  assume(consumed >= 0);
+
+  job->begin = job->begin.subspan(consumed, int(job->begin.size()) - consumed);
+  job->end = job->end.subspan(consumed, int(job->end.size()) - consumed);
+  job->lcp -= consumed;
+  job->commonPrefixNode = job->n;
+
+  if (job->lcp == int(job->begin.size())) {
+    *job->result = checkRangeRightSide(job->n, job->end, job->lcp,
                                        job->readVersion, context->tls)
                        ? ConflictSet::Commit
                        : ConflictSet::Conflict;
-  return complete(job, context);
+    MUSTTAIL return complete(job, context);
+  }
+
+  // This makes it safe to check maxVersion within checkRangeLeftSide. If this
+  // were false, then we would have returned above since lcp == begin.size().
+  assert(!(job->n->parent == nullptr && job->begin.size() == 0));
+
+  if (!checkRangeStartsWith(job->n, job->begin.subspan(0, job->lcp),
+                            job->begin[job->lcp], job->end[job->lcp],
+                            job->readVersion, context->tls)) {
+    *job->result = ConflictSet::Conflict;
+    MUSTTAIL return complete(job, context);
+  }
+
+  job->remaining = job->begin;
+  job->searchPathLen = 0;
+  job->prefixLen = job->lcp + 1;
+
+  if (job->remaining.size() == 0) {
+    assert(job->searchPathLen >= job->prefixLen);
+    if (maxVersion(job->n) > job->readVersion) {
+      job->setResult(false);
+      MUSTTAIL return complete(job, context);
+    } else {
+      job->continuation = done_left_side_iter;
+      MUSTTAIL return job->continuation(job, context);
+    }
+  }
+
+  if (job->searchPathLen >= job->prefixLen) {
+    if (!checkMaxBetweenExclusive(job->n, job->remaining[0], 256,
+                                  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(job->n, job->remaining[0]);
+    if (c != nullptr) {
+      if (job->searchPathLen < job->prefixLen) {
+        job->n = c;
+        job->continuation = left_side_down_left_spine;
+        __builtin_prefetch(job->n);
+        MUSTTAIL return keepGoing(job, context);
+      }
+      job->n = c;
+      if (maxVersion(job->n) > job->readVersion) {
+        job->setResult(false);
+        MUSTTAIL return complete(job, context);
+      }
+      job->continuation = done_left_side_iter;
+      MUSTTAIL return job->continuation(job, context);
+    } else {
+      job->n = nextSibling(job->n);
+      if (job->n == nullptr) {
+        job->continuation = done_left_side_iter;
+        MUSTTAIL return job->continuation(job, context);
+      }
+      job->continuation = left_side_down_left_spine;
+      __builtin_prefetch(job->n);
+      MUSTTAIL return keepGoing(job, context);
+    }
+  }
+
+  job->n = child;
+  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>
+PRESERVE_NONE void left_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);
+  ++job->searchPathLen;
+
+  if (n->partialKeyLen > 0) {
+    int commonLen = std::min<int>(n->partialKeyLen, job->remaining.size());
+    int i =
+        longestCommonPrefix(n->partialKey(), job->remaining.data(), commonLen);
+    job->searchPathLen += i;
+    if (i < commonLen) {
+      auto c = n->partialKey()[i] <=> job->remaining[i];
+      if (c > 0) {
+        if (job->searchPathLen < job->prefixLen) {
+          job->continuation = left_side_down_left_spine;
+          MUSTTAIL return job->continuation(job, context);
+        }
+        if (n->entryPresent && n->entry.rangeVersion > job->readVersion) {
+          job->setResult(false);
+          MUSTTAIL return complete(job, context);
+        }
+        if (maxVersion(n) > job->readVersion) {
+          job->setResult(false);
+          MUSTTAIL return complete(job, context);
+        }
+        job->continuation = done_left_side_iter;
+        MUSTTAIL return job->continuation(job, context);
+      } else {
+        job->n = nextSibling(n);
+        if (job->n == nullptr) {
+          job->continuation = done_left_side_iter;
+          MUSTTAIL return job->continuation(job, context);
+        }
+        job->continuation = left_side_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())) {
+      assert(job->searchPathLen >= job->prefixLen);
+      if (n->entryPresent && n->entry.rangeVersion > job->readVersion) {
+        job->setResult(false);
+        MUSTTAIL return complete(job, context);
+      }
+      if (maxVersion(n) > job->readVersion) {
+        job->setResult(false);
+        MUSTTAIL return complete(job, context);
+      }
+      job->continuation = done_left_side_iter;
+      MUSTTAIL return job->continuation(job, context);
+    }
+  }
+
+  ++context->tls->range_read_iterations_accum;
+
+  if (job->remaining.size() == 0) {
+    assert(job->searchPathLen >= job->prefixLen);
+    if (maxVersion(n) > job->readVersion) {
+      *job->result = ConflictSet::Conflict;
+      MUSTTAIL return complete(job, context);
+    } else {
+      job->continuation = done_left_side_iter;
+      MUSTTAIL return job->continuation(job, context);
+    }
+  }
+
+  if (job->searchPathLen >= job->prefixLen) {
+    if (!checkMaxBetweenExclusive(n, job->remaining[0], 256, job->readVersion,
+                                  context->tls)) {
+      *job->result = ConflictSet::Conflict;
+      MUSTTAIL return complete(job, context);
+    }
+  }
+
+  auto c = getChild(n, job->remaining[0]);
+  Node *child = c;
+  if (child == nullptr) {
+    auto c = getChildGeq(n, job->remaining[0]);
+    if (c != nullptr) {
+      if (job->searchPathLen < job->prefixLen) {
+        job->n = c;
+        job->continuation = left_side_down_left_spine;
+        __builtin_prefetch(job->n);
+        MUSTTAIL return keepGoing(job, context);
+      }
+      job->n = c;
+      if (maxVersion(job->n) > job->readVersion) {
+        *job->result = ConflictSet::Conflict;
+        MUSTTAIL return complete(job, context);
+      }
+      job->continuation = done_left_side_iter;
+      MUSTTAIL return job->continuation(job, context);
+    } else {
+      job->n = nextSibling(job->n);
+      if (job->n == nullptr) {
+        job->continuation = done_left_side_iter;
+        MUSTTAIL return job->continuation(job, context);
+      }
+      job->continuation = left_side_down_left_spine;
+      __builtin_prefetch(job->n);
+      MUSTTAIL return keepGoing(job, context);
+    }
+  }
+
+  job->n = child;
+  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));
+  MUSTTAIL return complete(job, context);
+}
+
+void left_side_down_left_spine(CheckJob *job, CheckContext *context) {
+  if (job->n->entryPresent) {
+    if (job->n->entry.rangeVersion > job->readVersion) {
+      job->setResult(false);
+      MUSTTAIL return complete(job, context);
+    }
+    job->continuation = done_left_side_iter;
+    MUSTTAIL return job->continuation(job, context);
+  }
+  job->n = getFirstChildExists(job->n);
+  __builtin_prefetch(job->n);
+  MUSTTAIL return keepGoing(job, context);
 }
 
 } // namespace check_range_read_state_machine