From ae5bd27d18afec4d788af2a09d7fec3e10b4228c Mon Sep 17 00:00:00 2001 From: Weaselbot Date: Tue, 10 Nov 2026 15:20:45 -0500 Subject: [PATCH] Compute the TooOld window in integer arithmetic `newestVersion - 2e9` is evaluated in `double`, whose 53 bit mantissa makes the `newestVersion - 2000000000` boundary off by up to 512 versions once `newestVersion >= 2^53`. skip_list therefore reported `Commit` for reads one version below the window, and `TooOld` for reads exactly at it. `ReferenceImpl` in Internal.h had the same expression, so the differential tests could not catch it. Share a single integer `kNominalVersionWindow` via Internal.h, use it in both places, and drop the duplicate (already integer) definition from ConflictSet.cpp. Adds a regression test that bisimulates skip_list against the radix tree above 2^53. Closes #99 --- ConflictSet.cpp | 1 - Internal.h | 11 ++++++++++- SkipList.cpp | 2 +- test_conflict_set.py | 16 ++++++++++++++++ 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/ConflictSet.cpp b/ConflictSet.cpp index 3273dc2..6241983 100644 --- a/ConflictSet.cpp +++ b/ConflictSet.cpp @@ -96,7 +96,6 @@ int64_t getSearchPathLength(struct Node *); // ==================== BEGIN IMPLEMENTATION ==================== -constexpr int64_t kNominalVersionWindow = 2e9; constexpr int64_t kMaxCorrectVersionWindow = std::numeric_limits::max(); static_assert(kNominalVersionWindow <= kMaxCorrectVersionWindow); diff --git a/Internal.h b/Internal.h index f2b2ea8..baf7ed6 100644 --- a/Internal.h +++ b/Internal.h @@ -27,6 +27,15 @@ using namespace weaselab; #define SHOW_MEMORY 0 #endif +// The window of versions, ending at `newestVersion`, that a subsequent `check` +// is guaranteed to answer correctly. Reads older than +// `newestVersion - kNominalVersionWindow` are reported as `TooOld`. +// +// This must be an integer constant: `newestVersion - 2e9` is computed in +// `double`, whose 53 bit mantissa makes the boundary off by up to 512 versions +// once `newestVersion >= 2^53`. +constexpr int64_t kNominalVersionWindow = 2000000000; + // std::span is not trivially constructible. We want a span that leaves its // members uninitialized for performance reasons. struct TrivialSpan { @@ -513,7 +522,7 @@ struct ReferenceImpl { int count) const { for (int i = 0; i < count; ++i) { if (reads[i].readVersion < oldestVersion || - reads[i].readVersion < newestVersion - 2e9) { + reads[i].readVersion < newestVersion - kNominalVersionWindow) { results[i] = ConflictSet::TooOld; continue; } diff --git a/SkipList.cpp b/SkipList.cpp index d64d550..ac81b04 100644 --- a/SkipList.cpp +++ b/SkipList.cpp @@ -741,7 +741,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl { int iters = skipList.detectConflicts(ranges, count, results); for (int i = 0; i < count; ++i) { if (reads[i].readVersion < oldestVersion || - reads[i].readVersion < newestVersion - 2e9) { + reads[i].readVersion < newestVersion - kNominalVersionWindow) { results[i] = TooOld; } tls.commits_accum += results[i] == Commit; diff --git a/test_conflict_set.py b/test_conflict_set.py index 924e89e..802958c 100644 --- a/test_conflict_set.py +++ b/test_conflict_set.py @@ -209,6 +209,22 @@ def test_two_billion_versions(): cs.check(read(1, b"\x00", b"\xff")) +def test_too_old_window_precision(): + # Regression test for issue #99: the TooOld window boundary + # (`newestVersion - 2e9`) must be computed with integer arithmetic. It used + # to be computed in `double`, whose 53 bit mantissa makes the boundary off + # by up to ~512 versions once newestVersion >= 2^53. + for newest in (1 << 53, (1 << 53) + 1, (1 << 53) + 7, 2**63 - 1): + with DebugConflictSet() as cs: + cs.addWrites(newest) # No writes; advances newestVersion + boundary = newest - 2 * 10**9 + # One below the window is TooOld, even though `double` rounding + # used to put it inside the window. + assert cs.check(read(boundary - 1, b"k")) == [Result.TOO_OLD] + # Exactly at the boundary is not TooOld. + assert cs.check(read(boundary, b"k")) == [Result.COMMIT] + + def test_positive_bytes(): with DebugConflictSet() as cs: cs.addWrites(1, write(b"hello")) -- 2.43.0