commit 65178051fd1bbb812c06eba5a065631e2769d3d8
parent d918c915f381f21176c929da719576a4724df86f
Author: Jack Mordaunt <jackmordaunt.dev@gmail.com>
Date: Fri, 18 Sep 2026 13:04:46 -0400
scan: give the snapshot retry count a reason
Sixteen attempts was a number picked rather than derived, and it did less than
it appeared to. Only one failure benefits from retrying: a write still in
progress, which copies a few hundred bytes and so finishes in tens of
nanoseconds against under ten for an attempt. Four covers that. If the builder
has been descheduled partway through instead, sixteen attempts fail as surely
as four and a caller keeping last frame's answer has lost nothing either way.
The two sequence reads being sequentially consistent is what makes this a
seqlock rather than a race that happens to work, so that now says so where
someone might otherwise reorder it.
Diffstat:
2 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/scan/snapshot.odin b/scan/snapshot.odin
@@ -24,11 +24,9 @@ Snapshot :: struct {
/*
Hands a snapshot from the thread building it to the thread drawing it.
-A sequence number is raised before a write and again after, so it is odd only while
-the data is in flux. A reader takes a copy and checks the number is unchanged and
-even; if not it takes another. That is what keeps a frame from ever waiting on a
-build step, however long the step takes: the reader either gets the new answer or
-keeps the one it already had.
+A sequence number is odd only while a write is in flux. A reader copies, then checks
+the number is unchanged and even. A frame therefore never waits on a build: it gets
+the new answer, or keeps the one it had.
*/
Publisher :: struct {
seq: u32,
@@ -41,14 +39,28 @@ publish :: proc(p: ^Publisher, s: Snapshot) {
sync.atomic_add(&p.seq, 1)
}
-// The latest complete snapshot. Fails only if the builder wrote several times during
-// the attempt, in which case the caller draws what it drew last frame.
+/*
+Attempts before giving up.
+
+Enough to cover one write, which copies a few hundred bytes and so takes tens of
+nanoseconds against under ten for an attempt. Beyond that the builder has been
+descheduled partway through and no amount of spinning will help, while a caller that
+keeps last frame's answer has lost nothing.
+*/
+@(private)
+ATTEMPTS :: 4
+
+// The latest complete snapshot. Fails while a write is in flux, which is the
+// caller's cue to keep the frame it already has.
current :: proc(p: ^Publisher) -> (s: Snapshot, ok: bool) {
- for _ in 0 ..< 16 {
+ for _ in 0 ..< ATTEMPTS {
before := sync.atomic_load(&p.seq)
if before & 1 != 0 {
continue // a write is in progress
}
+ // Both loads are sequentially consistent, which is what stops the compiler
+ // hoisting this copy above the first or sinking it below the second. Moving it
+ // either way turns a correct seqlock into one that returns torn data silently.
s = p.data
if sync.atomic_load(&p.seq) == before {
return s, true
diff --git a/scan/snapshot_test.odin b/scan/snapshot_test.odin
@@ -0,0 +1,37 @@
+package scan
+
+import "core:testing"
+
+@(test)
+test_snapshot_round_trips :: proc(t: ^testing.T) {
+ p: Publisher
+ _, ok := current(&p)
+ testing.expect(t, ok, "an untouched publisher should still hand out its zero value")
+
+ s := Snapshot{count = 2, nodes = 99, complete = true}
+ s.rows[0] = {node = 7, bytes = 4096}
+ publish(&p, s)
+
+ got, got_ok := current(&p)
+ testing.expect(t, got_ok)
+ testing.expect_value(t, got.count, 2)
+ testing.expect_value(t, got.nodes, u64(99))
+ testing.expect_value(t, got.rows[0].bytes, u64(4096))
+ testing.expect(t, got.complete)
+}
+
+@(test)
+test_snapshot_refuses_a_write_in_flux :: proc(t: ^testing.T) {
+ // An odd sequence is what a half-written snapshot looks like. The reader must
+ // decline rather than hand back something torn.
+ p: Publisher
+ publish(&p, Snapshot{count = 1})
+ p.seq += 1 // as though a write had begun
+
+ _, ok := current(&p)
+ testing.expect(t, !ok, "a snapshot mid-write was handed out")
+
+ p.seq += 1 // and finished
+ _, ok2 := current(&p)
+ testing.expect(t, ok2, "a finished write was not readable")
+}
