BENCH.md: fix numbers left stale by the After-table replacement

Doing another literal, no-caveats audit pass (per the standing request to
verify literally everything, not just reassert it) found real, concrete
inconsistencies introduced by the previous commit: it replaced BENCH.md's
"After: complete results" table and Appendix B with a newer, more complete
bench/bench.c run (adding the mount-table-scaling category), but several
other places in the same document still quoted exact figures from the
older run those tables used to hold -- a reader cross-checking the
"Resolution" section's headline table, or the "Analysis" section's bullet
points, against the "After" table below them would have found contradictory
numbers for the same measurement.

Verified programmatically, not just by re-reading: every row's time,
throughput, and MB/s in the Before (45 rows) and After (54 rows) tables now
matches its corresponding appendix line exactly (0 mismatches out of 99
rows, checked with a script, not by eye).

Fixed:
- The "Resolution" section's headline before/after table used the older
  run's mem create/unlink/mkdir/rmdir numbers (0.0372s/0.0320s/0.0052s/
  0.0045s), which don't match the current After table (0.0400s/0.0179s/
  0.0056s/0.0040s) -- for unlink specifically, a genuine near-2x difference
  between two runs of identical, already-fixed code, not just noise. Now
  points at the single current run, with speedups recomputed (239x/590x/
  60x/85x/11x/21x/134x, mem now 25-27x faster than raw fs, complexity-class
  check now 7.14x vs the old 7.15x -- nearly identical, which is the
  reassurance that actually matters), plus an explicit note on the observed
  run-to-run variance rather than silently picking one number.
- The "Analysis" section's read/stat/random-read/compaction/dir-stat/
  large-write bullets all cited the older run's exact ops/s and MB/s
  figures (2.4M small-file reads, ~815K compaction entries/s, 133K dir
  stat, etc.); recomputed against the current After table (now 15-16x for
  small reads, ~9x for stat, ~91MB/s / ~748K entries/s for compaction, 145K
  vs 286K for dir stat, corrected large-write MB/s ranges).
- The same stale 65-330x / 15-27x / 22x range in CLAUDE.md's "Known
  performance characteristics" bulk-write bullet, updated to match and to
  explain the same run-to-run variance rather than presenting one run's
  numbers as if they were exact.

No code changes; make test still passes (all 6 binaries, unaffected by a
documentation-only change).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UqJpkdJ6Njnt1pw3CbghzB
This commit is contained in:
2026-09-14 10:13:57 +00:00
co-authored by Claude Sonnet 5
parent 684c6d95de
commit 529935deb4
2 changed files with 59 additions and 36 deletions
+1 -1
View File
@@ -89,7 +89,7 @@ These are hard requirements stated in the spec, not stylistic suggestions — an
`bench/bench.c` (`make bench`) measures PackFS against the host filesystem; full results and analysis are in `BENCH.md`. The one finding here that future work on this codebase needs to know without re-running the benchmark:
- **RESOLVED: bulk sequential `create`/`unlink`/`mkdir` on `mem`/`dir` was O(n²) in the number of entries — confirmed empirically (`BENCH.md`), then fixed, not just worked around.** The index (`UpperSnapshot`) was a flat sorted array; every structural write copied it in full before publishing the next snapshot (Section 5.3's single-writer model), and at 20,000 sequential creates this measured ~9–22x slower than the raw host filesystem. `concept.md` Section 5.3 names the exact trigger for reconsidering that design — "a persistent (structurally shared) tree structure is not required until this assumption is empirically violated" — and that trigger was hit. The index is now a **persistent treap** (`src/upper.c`, `struct TreapNode` and the functions around it — see the citations in that comment, Seidel & Aragon 1996 and Liljenzin arXiv:1301.3388, for why a treap and not a persistent AVL/red-black/weight-balanced tree): a structural write now copies only the O(log n) nodes on the path to the change. Post-fix, the same benchmark shows `mem` create/unlink/mkdir 65–330x faster than before, and — the more important comparison — 15–27x *faster* than raw fs where it used to be 9–22x *slower*; the concurrent mixed workload went from 6x slower than raw fs to 22x faster. `BENCH.md`'s "Resolution" section has the full before/after table and the complexity-class re-confirmation (the same `mkdir`-at-N=4,000-vs-`create`-at-N=20,000 methodology that found O(n²) now shows a ratio consistent with O(n log n), not O(n²)). A new test, `tests/test_index_stress.c`, cross-checks the treap's correctness under thousands of randomized (non-sequential) creates/deletes/renames against an independent reference model — read it, not just the benchmark, before touching `snapshot_upsert`/`snapshot_remove`/the treap functions again.
- **RESOLVED: bulk sequential `create`/`unlink`/`mkdir` on `mem`/`dir` was O(n²) in the number of entries — confirmed empirically (`BENCH.md`), then fixed, not just worked around.** The index (`UpperSnapshot`) was a flat sorted array; every structural write copied it in full before publishing the next snapshot (Section 5.3's single-writer model), and at 20,000 sequential creates this measured ~9–22x slower than the raw host filesystem. `concept.md` Section 5.3 names the exact trigger for reconsidering that design — "a persistent (structurally shared) tree structure is not required until this assumption is empirically violated" — and that trigger was hit. The index is now a **persistent treap** (`src/upper.c`, `struct TreapNode` and the functions around it — see the citations in that comment, Seidel & Aragon 1996 and Liljenzin arXiv:1301.3388, for why a treap and not a persistent AVL/red-black/weight-balanced tree): a structural write now copies only the O(log n) nodes on the path to the change. Post-fix, the same benchmark shows `mem` create/unlink/mkdir/rmdir 11–590x faster than before (the range is wide because at these post-fix speeds — single-digit milliseconds for 20,000 entries — ordinary run-to-run system jitter has real proportional effect; see `BENCH.md`'s "Resolution" for the two-run comparison that makes this explicit rather than hiding it behind one cherry-picked number), and — the more important, and more stable, comparison — `mem` create and unlink are now 25–27x *faster* than raw fs where they used to be 9.5–22x *slower*; the concurrent mixed workload separately went from 6x slower than raw fs to 21x faster. `BENCH.md`'s "Resolution" section has the full before/after table and the complexity-class re-confirmation (the same `mkdir`-at-N=4,000-vs-`create`-at-N=20,000 methodology that found O(n²) now shows a ratio consistent with O(n log n), not O(n²)). A new test, `tests/test_index_stress.c`, cross-checks the treap's correctness under thousands of randomized (non-sequential) creates/deletes/renames against an independent reference model — read it, not just the benchmark, before touching `snapshot_upsert`/`snapshot_remove`/the treap functions again.
- **`dir`-backend `stat` costs roughly 2x a raw `stat()` call**, because `pfs_dir_statat` (containment, Section 6.2) resolves and opens the path, then `fstat`s the fd, where raw `stat()` is one syscall. Expected, and the same containment cost `open` already pays — recorded so it isn't mistaken for a regression if someone benchmarks it again later.
- **`mem`-backed large sequential writes lose to a plain unsynced host `write()` above roughly 16 MB**, because Section 5.7's buffer-growth discipline (allocate new, copy everything, publish, retire old) re-copies previously-written bytes on every capacity doubling, where the host page cache only ever appends new pages. This is the measured cost of the safety property Section 5.7 requires (no reader ever sees a freed buffer), not an accidental inefficiency.
- **RESOLVED: `pack_write`'s compaction-time exact-duplicate elimination (Section 9.2) was O(n²) in entry count, plus a latent correctness bug, both fixed together.** The dedup check was a linear scan of every previously-seen `(hash, size)` pair per entry, comparing only the hash and size — never the actual bytes — before trusting a match, so two different-content entries that happened to collide on `(hash, size)` under FNV-1a64 (explicitly not collision-resistant, `src/hash.c`) would have been silently merged, corrupting one of them; this was never observed in practice but was true of the code as written. `bench/bench.c`'s own compaction benchmark never surfaced either problem because its test files are byte-identical, which made every scan match on the first comparison (the scan's best case, not its worst case). Measuring with unique content instead (`pack_write` called directly, bypassing the journal/fsync path) showed 80,000 entries taking 1.74s, with a 20,000→80,000 (4x N) step showing 15.6x — matching O(n²)'s 16x prediction. Fixed with an open-addressing hash table (`DedupSlot`, load factor 1/2, linear probing) plus a `memcmp` verification before ever reusing a `data_off`, closing both the complexity issue and the correctness gap at once; post-fix, the same 80,000-entry case takes 0.044s (39.6x faster) and the 20,000→80,000 ratio drops to 3.35x, consistent with O(n). `BENCH.md`'s "Resolution #2" has the full before/after numbers. Correctness is covered permanently by `tests/test_pack_overlay.c` (asserts duplicate-content entries share one `data_off` and distinct-content entries do not); the performance fix is covered permanently by `tests/test_pack_write_perf.c` (a regression tripwire against 10,000 unique entries, run as part of `make test`).