From 529935deb40d11c7b3c8c49536c63b68c7933c7d Mon Sep 17 00:00:00 2001 From: retoor Date: Mon, 14 Sep 2026 10:13:57 +0000 Subject: [PATCH] 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 Claude-Session: https://claude.ai/code/session_01UqJpkdJ6Njnt1pw3CbghzB --- BENCH.md | 93 ++++++++++++++++++++++++++++++++++--------------------- CLAUDE.md | 2 +- 2 files changed, 59 insertions(+), 36 deletions(-) diff --git a/BENCH.md b/BENCH.md index acf35d3..69ecbdb 100644 --- a/BENCH.md +++ b/BENCH.md @@ -88,21 +88,37 @@ rotation, which a treap avoids); Liljenzin, "Confluently Persistent Sets and Maps" (arXiv:1301.3388), for persistent treaps specifically as an MVCC snapshot mechanism, which is this exact use case. -**Headline result, measured, same methodology as the original finding:** +**Headline result, measured, same methodology as the original finding.** +The "After" figures below are drawn from the "After: complete results" +table further down this document (the current, canonical post-fix run), +not from an intermediate run kept only in this section — every number here +can be found again in that table: | Category | Before | After | Speedup | |---|---|---|---| -| create 20,000 files (mem) | 9.5565s / 2,093 ops/s | 0.0372s / 537,103 ops/s | **257x** | -| unlink 20,000 files (mem) | 10.5575s / 1,894 ops/s | 0.0320s / 625,437 ops/s | **330x** | -| mkdir 4,000 dirs (mem) | 0.3363s / 11,896 ops/s | 0.0052s / 771,085 ops/s | **65x** | -| rmdir 4,000 dirs (mem) | 0.3395s / 11,782 ops/s | 0.0045s / 898,169 ops/s | **75x** | -| create 20,000 files (dir) | 12.0367s / 1,662 ops/s | 1.1531s / 17,345 ops/s | **10x** | -| unlink 20,000 files (dir) | 10.1851s / 1,964 ops/s | 0.5146s / 38,864 ops/s | **20x** | -| concurrent create+read+unlink (mem) | 36.7261s / 2,614 ops/s | 0.2807s / 341,972 ops/s | **131x** | +| create 20,000 files (mem) | 9.5565s / 2,093 ops/s | 0.0400s / 499,809 ops/s | **239x** | +| unlink 20,000 files (mem) | 10.5575s / 1,894 ops/s | 0.0179s / 1,117,742 ops/s | **590x** | +| mkdir 4,000 dirs (mem) | 0.3363s / 11,896 ops/s | 0.0056s / 719,696 ops/s | **60x** | +| rmdir 4,000 dirs (mem) | 0.3395s / 11,782 ops/s | 0.0040s / 993,724 ops/s | **85x** | +| create 20,000 files (dir) | 12.0367s / 1,662 ops/s | 1.1418s / 17,517 ops/s | **11x** | +| unlink 20,000 files (dir) | 10.1851s / 1,964 ops/s | 0.4911s / 40,724 ops/s | **21x** | +| concurrent create+read+unlink (mem) | 36.7261s / 2,614 ops/s | 0.2750s / 349,127 ops/s | **134x** | + +(An earlier revision of this table cited a different post-fix run — same +fixed code, an earlier `make bench` invocation — whose numbers were 65–330x +rather than 60–590x; the two runs agree on every qualitative conclusion +below, but `unlink 20,000 files (mem)` in particular differs by nearly 2x +between them, 0.0320s vs 0.0179s, both measuring an operation fast enough +(sub-20ms for 20,000 entries) that ordinary system jitter in this +containerized environment has real proportional effect. This is exactly +the kind of run-to-run variance this document's stated methodology warns +about — "illustrative of shape... more than precise absolute figures" — +and the reason the table now points at one single canonical run rather +than mixing figures from two.) `mem` now *beats* raw fs (not merely "no longer loses to it") at every one -of these: create is 27x faster than raw fs (was 9.5x slower), unlink is 15x -faster (was 22x slower), and the concurrent mixed workload is 22x faster +of these: create is 25x faster than raw fs (was 9.5x slower), unlink is 27x +faster (was 22x slower), and the concurrent mixed workload is 21x faster (was 6x slower) — the single-writer design's "optimizes for read-heavy concurrent workloads" caveat from the original analysis (below) no longer needs to exclude bulk-write workloads, because the write side is no longer @@ -112,13 +128,17 @@ the bottleneck it was. original O(n²) finding was** — not just "it got faster," which an unrelated constant-factor optimization could also produce: `mkdir` at N=4,000 vs. `create` at N=20,000 (same structural-write mechanism, 5x the N) now shows a -**7.15x** slowdown (0.0372s / 0.0052s). O(n) predicts 5.0x; O(n log n) +**7.14x** slowdown (0.0400s / 0.0056s). O(n) predicts 5.0x; O(n log n) predicts 5.97x; the old O(n²) regime predicted, and measured, 25–28.4x. -7.15x lands close to the O(n log n) prediction (some excess is expected — +7.14x lands close to the O(n log n) prediction (some excess is expected — `create` writes bytes into a fresh `MutCell` and `mkdir` does not, a constant-factor difference the pure index-complexity comparison doesn't -account for) and nowhere near quadratic. The index's asymptotic behavior -changed, not merely its constant factor. +account for) and nowhere near quadratic — and is itself nearly identical to +the 7.15x this same check gave against the earlier run referenced above, +which is the reassurance that matters here: the *ratio* this check depends +on is stable across runs even where the fastest individual measurements +are not. The index's asymptotic behavior changed, not merely its constant +factor. Full output of both runs is reproduced verbatim in the appendices below and is reproducible again with `make bench`; the correctness of the new index @@ -394,37 +414,39 @@ make for them; they are current-state measurements only, discussed in ## Analysis -The bullets below are preserved from the original run for the categories -the fix did not change (the "wins" and the non-structural-write "losses"), -with the historical O(n²) explanation kept as the record of what was found, -now marked where "Resolution" above supersedes it. +The bullets below are checked against the current "After" table (the +54-measurement run) rather than preserved unchanged from an earlier draft — +an earlier revision of this section cited exact figures from the prior +45-measurement after-run, which drifted out of sync with the "After" table +once that table was replaced with the newer run; the numbers below were +recomputed from the table actually in this document, not carried over. ### Where PackFS wins clearly -- **Reads of small files: 20–23x faster than both raw fs and the `dir` - backend** (2.4M ops/s vs ~125K ops/s). No syscall per read — `mem` +- **Reads of small files: 15–16x faster than both raw fs and the `dir` + backend** (2.0M ops/s vs ~125–131K ops/s). No syscall per read — `mem` reads are an expected-O(log n) treap lookup plus a `memcpy` out of an already-resident buffer (Section 5.3, 9.1). -- **`stat`: ~10x faster than raw fs.** Same reason — no syscall, and the - index is sorted for expected-O(log n) lookup rather than requiring a - directory entry scan. +- **`stat`: ~9x faster than raw fs** (2.6M ops/s vs 286K ops/s). Same + reason — no syscall, and the index is sorted for expected-O(log n) + lookup rather than requiring a directory entry scan. - **Random-access reads against a compacted pack: ~20x faster than raw fs** - (2.4M ops/s vs 121K ops/s), because the pack is one `mmap`'d file with a + (2.4M ops/s vs 123K ops/s), because the pack is one `mmap`'d file with a binary-searchable index (Section 9.1), against 20,000 individual `open`/`read`/`close` syscall triples on the raw-fs side. This is the single result that most directly validates the architecture's stated purpose — Section 3.2's claim that "random access is a flat table lookup, not a linear scan or a directory-parse-then-seek" — under an actual measured workload, not just by construction. -- **Compaction throughput**: ~100 MB/s / ~815K entries/s to serialize the +- **Compaction throughput**: ~91 MB/s / ~748K entries/s to serialize the live tree into a fresh pack (Section 4.1 step 5) — not directly comparable to any raw-fs operation, but fast enough that compacting a - 20,000-file, 2.5 MB tree is not a practically-felt pause (25ms). -- **Bulk create/unlink/mkdir/rmdir: RESOLVED, now 10–330x faster than the - pre-fix `mem`/`dir` numbers, and (for `mem`) 15–27x faster than raw fs** — + 20,000-file, 2.5 MB tree is not a practically-felt pause (27ms). +- **Bulk create/unlink/mkdir/rmdir: RESOLVED, now 11–590x faster than the + pre-fix `mem`/`dir` numbers, and (for `mem`) 21–27x faster than raw fs** — see "Resolution" above; this used to be PackFS's clearest loss and is now among its clearest wins. -- **Concurrent mixed create+read+unlink: RESOLVED, now 22x faster than raw +- **Concurrent mixed create+read+unlink: RESOLVED, now 21x faster than raw fs** (was 6x *slower* before the fix) — see "Resolution." - **`pack_write` compaction dedup on distinct-content files: RESOLVED, 1.75– 39.6x faster depending on N, plus a latent hash-collision correctness bug @@ -435,8 +457,8 @@ now marked where "Resolution" above supersedes it. ### Where raw fs still wins, and why that's expected, not a bug -- **`dir` backend `stat` is ~2.2x *slower* than raw fs `stat`** (133K ops/s - vs 287K ops/s) — because `pfs_dir_statat` (Section 6.2's containment) +- **`dir` backend `stat` is ~2.0x *slower* than raw fs `stat`** (145K ops/s + vs 286K ops/s) — because `pfs_dir_statat` (Section 6.2's containment) resolves and opens the path via `openat2`/`O_NOFOLLOW`, then `fstat`s the resulting fd, where a raw `stat()` call is a single syscall. This is the direct, measured cost of path containment on the metadata path, @@ -444,8 +466,9 @@ now marked where "Resolution" above supersedes it. itself (which raw fs pays an equivalent single-syscall cost for anyway). Unaffected by the index rewrite. - **`raw+fsync` create is catastrophically slow on this environment** - (172 ops/s — 116 seconds for 20,000 files, ~5.8ms per `fsync`, identical - in both runs to three decimal places), because this container's overlay + (172 ops/s — 116 seconds for 20,000 files, ~5.8ms per `fsync`, within + measurement noise across the "Before" and "After" runs — 116.1030s vs + 116.2147s, a 0.1% difference), because this container's overlay filesystem has poor per-call `fsync` latency. This is a property of the container, not of PackFS or of a bare disk — flagged in the environment section above precisely so it isn't misread as "PackFS's journal must be @@ -454,8 +477,8 @@ now marked where "Resolution" above supersedes it. which is a real, inherited cost on this kind of storage — not a PackFS-specific one. Unaffected by the index rewrite (the journal's cost is per-write `fsync` latency, not index maintenance). -- **Large writes above ~16 MB: raw fs is faster than `mem`** (3,478– - 3,496 MB/s vs 1,087–1,483 MB/s). Section 5.7's buffer-growth discipline +- **Large writes above ~16 MB: raw fs is faster than `mem`** (3,450– + 3,656 MB/s vs 1,091–1,458 MB/s). Section 5.7's buffer-growth discipline (allocate new, copy old + new, publish, retire old — never realloc in place) means every capacity doubling re-copies everything written so far; a plain unsynced `write()` to a real file only ever appends new diff --git a/CLAUDE.md b/CLAUDE.md index 2640eac..5e13b6e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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`).