| Comment: | logstore: bound the priority window structurally, not with an assert
gatherCandidates capped the entries ADDED from the caller's priority slice, not the elements read: a duplicate hits ridListed and continues without advancing n, so the fill check never fires and the walk runs the caller's whole array with a linear dedup scan at each step. The only thing holding that back was an assert that the slice fits the window -- and std.debug.assert compiles to nothing in ReleaseFast and ReleaseSmall, so there was no guard in exactly the build where an unbounded scan would matter. Slicing the prefix holds in every mode, and turns an over-length slice from a precondition violation into defined behavior: accepted, tail ignored. Both callers today are inside the window either way. Two tests, both mutation-checked against the old shape. What is NOT pinned is the duplicate-heavy case: dedup leaves the surplus slots to the chronological walk, so the candidate set is identical either way and only the count of elements read differs. That is a cost property, not a behavioral one, and no test through this API can catch it -- the header comment says so rather than letting the pair read as covering it. |
|---|---|
| Timelines: | family | ancestors | descendants | both | trunk |
| Files: | files | file ages | folders |
| SHA3-256: |
bfec65b1a17b88103bd815494475f0a9 |
| User & Date: | dmestas on 2026-07-30 20:21:39.598 |
| Other Links: | manifest | tags |
|
2026-07-30
| ||
| 21:20 |
build: --release means ReleaseSafe, and pin it
src/ carries 1086 std.debug.assert calls. They are how a detected inconsistency becomes a crash instead of a durable write -- TigerStyle's 'downgrade catastrophic correctness bugs into liveness bugs' -- and that trade exists only while the assertion does. ReleaseFast and ReleaseSmall compile every one to nothing, so a binary built that way converts each guarded corruption into an unguarded one, in a store where a bad frame is permanent and a crash is not. Measured, not assumed. Read-side verbs are indistinguishable between the two modes: status, log and stat all sit inside noise on a 5 MB store. zit pack is not: 599ms fast against 15.2s safe, about 25x, because the delta encoder's best-of-N search is a tight per-byte loop where a bounds check is a large share of the work. That argues for setRuntimeSafety(false) on that measured loop -- local, reviewable, over code the fuzz and DST lanes already cover -- rather than stripping assertions from every other path in the binary. Not taken here. Note the option surface changes: stating a preference replaces the -Doptimize enum with a -Drelease bool, so the deliberate escape hatch is the top-level --release=fast. -Doptimize=ReleaseFast no longer parses. ci_test pins the preference, and says that if the project ever decides to ship unchecked, the pin should be deleted in that same commit so the decision is visible in history rather than arriving as drift. check-in: 4379187a10 user: dmestas tags: trunk | |
| 20:21 |
logstore: bound the priority window structurally, not with an assert
gatherCandidates capped the entries ADDED from the caller's priority slice, not the elements read: a duplicate hits ridListed and continues without advancing n, so the fill check never fires and the walk runs the caller's whole array with a linear dedup scan at each step. The only thing holding that back was an assert that the slice fits the window -- and std.debug.assert compiles to nothing in ReleaseFast and ReleaseSmall, so there was no guard in exactly the build where an unbounded scan would matter. Slicing the prefix holds in every mode, and turns an over-length slice from a precondition violation into defined behavior: accepted, tail ignored. Both callers today are inside the window either way. Two tests, both mutation-checked against the old shape. What is NOT pinned is the duplicate-heavy case: dedup leaves the surplus slots to the chronological walk, so the candidate set is identical either way and only the count of elements read differs. That is a cost property, not a behavioral one, and no test through this API can catch it -- the header comment says so rather than letting the pair read as covering it. check-in: bfec65b1a1 user: dmestas tags: trunk | |
| 18:47 |
merge shun-implementation: shunning and space reclamation
A repository-local shun table (sync SS3.3), the zit shun verb over it, and the four suppression directions the drafts require: never requested, never announced or served, an arriving copy discarded, and the bytes reclaimed on demand through the existing atomic rewrite. Built as eight slices, each through design review, TDD, spec audit, three review lenses, mutation proof and a full gate. Reading the eight as one change afterwards found what none of them could alone: a peer-reachable assert that crashed the client on a malformed DELTASRC, and a pre-rename guard that re-stat'ed a stale handle so two concurrent rewrites could un-shun each other. Two defects predating the feature are fixed on the way. FileStorage.sizeImpl answers from a cached watermark, so pack's tail-revalidate compared a key against the number it was derived from and could never abort -- the guard had never worked. And a malformed file card could trap the client through PendingDeltaQueue. Format version stays 1: an older binary meets an unknown frame kind and rejects cleanly, which is what it should do with a tombstone it cannot read. No interchange surface changed -- smoke covers ten fossil 2.28 scenarios including a fossil clone of a defer-shunned store and a real fossil push of a shunned artifact. Known and recorded, not blocking: a running zit serve keeps serving the old inode until restarted after a --force reclaim swaps generations. check-in: e57bcceadf user: dmestas tags: trunk | |