retoor/packfs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix the O(n^2) bulk create/unlink: flat array -> persistent treap

Browse Source 
BENCH.md's earlier finding was real: every structural write (create/
unlink/mkdir) copied the entire sorted UpperSnapshot entry array before
publishing the next snapshot, making bulk sequential creation O(n^2).
concept.md Section 5.3 names the exact condition for reconsidering this
("a persistent structurally-shared tree structure is not required
until this assumption is empirically violated") -- that condition was
measured, not hypothesized, so this closes it rather than leaving it
as a documented-but-open limitation.

The index is now a persistent treap (src/upper.c): a structural write
copies only the O(log n) nodes on the path to the change, sharing
every other node (its own refcount, cascading like MutCell's and
UpperSnapshot's) with whichever snapshot(s) it was built from. Chosen
over a persistent AVL/red-black/weight-balanced tree because deletion
in those can need O(log n) rebalancing rotations -- each a real
allocation in a persistent setting -- where a treap needs only O(1)
amortized rotations for insert and delete (Seidel & Aragon 1996), with
expected O(log n) height regardless of insertion order, including the
sorted-by-creation-order pattern that made the flat array quadratic in
the first place. Liljenzin's "Confluently Persistent Sets and Maps"
(arXiv:1301.3388) documents persistent treaps giving O(1) snapshots
for MVCC specifically, which is this exact use case. Full reasoning
and the ownership convention (functions consume one ref of their tree
arguments, return one owned ref) are in upper.c's comment above
struct TreapNode.

Blast radius kept deliberately small: snapshot_upsert/snapshot_remove
keep their exact original signatures, so upper_create/upper_mkdir/
upper_remove/upper_rename/upper_copy_up needed zero changes.
upper_lookup/upper_has_children keep their exact contracts. Only
upstd_readdir and overlay_readdir's manual array scans became calls to
a new upper_visit_range (O(log n + r) range query, replacing an O(n)
scan in both, a bonus fix beyond what was strictly necessary) since
there's no flat array left to scan.

Added tests/test_index_stress.c: thousands of randomized (not
sequential) creates/deletes/renames across nested directories,
cross-checked against an independent reference model after every
round, not just "did it not crash" -- exercises exactly the code path
the O(n^2) bug and this fix live in, at a scale the other tests don't
reach. Verified under -fsanitize=undefined (150+ runs across this
change's lifetime, 0 failures) and -fsanitize=address (100+ runs, 0
real findings; known sandbox ASan-startup flakes excluded, see prior
commits) per CLAUDE.md's sanitizer rule for upper.c/overlay.c changes.

Measured result (make bench, same environment as the original
finding): mem create 257x faster, unlink 330x faster, mkdir 65x
faster, concurrent mixed workload 131x faster -- and, the comparison
that matters, mem now beats raw fs at every one of these (was losing
by 6-22x before). The complexity-class change is confirmed the same
way the O(n^2) was found: mkdir at N=4,000 vs create at N=20,000 now
shows a 7.15x slowdown for a 5x increase in N, matching the O(n log n)
prediction (5.97x) rather than the old O(n^2) one (25x). Full
before/after tables in BENCH.md's new "Resolution" section, which
keeps the original run as the historical record rather than
overwriting it, per this project's own documentation standard.

Recorded the fix in CLAUDE.md's "Known performance characteristics"
(marked RESOLVED, not silently removed) and its architecture map.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UqJpkdJ6Njnt1pw3CbghzB
This commit is contained in:
retoor
2026-09-14 08:09:49 +00:00
co-authored by Claude Sonnet 5
parent 0b3b207bc0
commit 64283d587b
8 changed files with 753 additions and 257 deletions
Download Patch File Download Diff File Expand all files Collapse all files
BENCH.md
+157 -75
View File
@@ -1,14 +1,23 @@
# PackFS vs. the host filesystem: benchmark results
This document reports the results of `bench/bench.c` (`make bench`), run to
completion once on the environment described below. It is a single run, not
a statistically-averaged series — treat the numbers as illustrative of
*shape* (which operations are faster, by roughly what factor, and why) more
than as precise absolute figures for a different machine. The methodology,
including exactly what each backend label measures, is documented in
`bench/bench.c`'s file header; read it before interpreting the numbers
below, since "raw fs," "raw+fsync," and "dir" are not interchangeable
baselines.
**Status: the O(n²) bulk create/unlink finding this document originally
reported has been fixed.** The index (`UpperSnapshot`) is now a persistent
treap instead of a flat sorted array (`src/upper.c`; see CLAUDE.md's "Known
performance characteristics"). This document keeps the original run's
numbers as the historical record of the problem — per this project's own
documentation standard, a correction extends the record rather than erasing
it — and adds the post-fix run's numbers alongside them. **Read "Resolution"
below before drawing conclusions from the "Original results" section**,
which describes a version of the index this codebase no longer has.
This document reports the results of `bench/bench.c` (`make bench`), each
run once on the environment described below. These are single runs, not a
statistically-averaged series — treat the numbers as illustrative of *shape*
(which operations are faster, by roughly what factor, and why) more than as
precise absolute figures for a different machine. The methodology, including
exactly what each backend label measures, is documented in `bench/bench.c`'s
file header; read it before interpreting the numbers below, since "raw fs,"
"raw+fsync," and "dir" are not interchangeable baselines.
## Environment
@@ -20,10 +29,72 @@ baselines.
- `N_SMALL` = 20,000 files × 128 bytes for the metadata-heavy suite;
`N_DIRS` = 4,000; concurrency = 8 threads × 4,000 ops; large files up to
64 MB. Full parameters are in `bench/bench.c`.
- Wall-clock time for the full run: 5m49s, almost entirely spent in the
single `raw+fsync` 20,000-file create test (116s) — see below.
- Both runs took several minutes of wall-clock time, almost entirely spent
in the single `raw+fsync` 20,000-file create test (~116s in both runs —
this cost is on the host's storage, not PackFS, and is identical before
and after the fix, as expected).
## Results
## Resolution
The original run below found bulk sequential `create`/`unlink`/`mkdir` on
`mem`/`dir` to be O(n²) in file count, because every structural write copied
the entire sorted entry array before publishing the next snapshot
(Section 5.3). `concept.md` itself names the trigger condition for
reconsidering that design — "a persistent (structurally shared) tree
structure is not required until this assumption is empirically violated" —
and the original run was exactly that violation, measured rather than
hypothesized.
The index was rewritten as a **persistent treap** (`src/upper.c`): a
structural write now builds only the O(log n) nodes on the path to the
change, sharing every other node (refcounted) with the snapshot it was
built from, instead of copying all n entries. The choice of treap over a
persistent AVL/red-black/weight-balanced tree, and the reasoning behind it,
is documented in `src/upper.c`'s comment above `struct TreapNode` and cited
there: Seidel & Aragon, "Randomized Search Trees" (1996), for the O(1)
amortized rotation count on both insert and delete (persistent balanced
trees that need O(log n) rotations pay a real, repeated allocation cost per
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.
**Result, measured, same methodology as the original finding:**
| Category | Before | After | Speedup |
|---|---|---|---|
| create 20,000 files (mem) | 9.56s / 2,093 ops/s | 0.037s / 537,103 ops/s | **257x** |
| unlink 20,000 files (mem) | 10.56s / 1,894 ops/s | 0.032s / 625,437 ops/s | **330x** |
| mkdir 4,000 dirs (mem) | 0.336s / 11,896 ops/s | 0.0052s / 771,085 ops/s | **65x** |
| create 20,000 files (dir) | 12.04s / 1,662 ops/s | 1.15s / 17,345 ops/s | **10x** |
| concurrent create+read+unlink (mem) | 36.73s / 2,614 ops/s | 0.28s / 341,972 ops/s | **131x** |
`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
(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
the bottleneck it was.
**The complexity-class change is confirmed quantitatively, the same way the
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. 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 — content writes'
buffer-allocation and per-file host effects differ between `create`, which
writes bytes, and `mkdir`, which does not) and nowhere near quadratic. The
index's asymptotic behavior changed, not merely its constant factor.
Full output of the post-fix run is reproducible with `make bench`; the
correctness of the new index under heavy randomized structural churn
(non-sequential creates/deletes/renames, nested directories, thousands of
entries, cross-checked against an independent reference model — not just
"the benchmark ran fast without crashing") is `tests/test_index_stress.c`,
part of `make test`.
## Original results (pre-fix; kept as the historical record — see "Resolution" above)
| Category | Backend | Time | Throughput | MB/s |
|---|---|---|---|---|
@@ -55,69 +126,89 @@ baselines.
| concurrent create+read+unlink (8×4,000×3) | mem | 36.73s | 2,614 ops/s | — |
| concurrent create+read+unlink (8×4,000×3) | raw fs | 6.12s | 15,686 ops/s | — |
Full per-run output, including the read-throughput MB/s columns omitted
above for brevity, is reproducible with `make bench`.
## Post-fix results
| Category | Backend | Time | Throughput | MB/s |
|---|---|---|---|---|
| create 20,000 files | mem | 0.037s | 537,103 ops/s | 65.6 |
| create 20,000 files | dir | 1.153s | 17,345 ops/s | 2.1 |
| create 20,000 files | raw fs | 1.010s | 19,805 ops/s | 2.4 |
| create 20,000 files | raw+fsync | 116.098s | 172 ops/s | 0.0 |
| read 20,000 files | mem | 0.0085s | 2,364,410 ops/s | 288.6 |
| read 20,000 files | dir | 0.160s | 124,933 ops/s | 15.3 |
| read 20,000 files | raw fs | 0.159s | 125,459 ops/s | 15.3 |
| stat 20,000 files | mem | 0.0073s | 2,723,001 ops/s | — |
| stat 20,000 files | dir | 0.151s | 132,517 ops/s | — |
| stat 20,000 files | raw fs | 0.070s | 287,063 ops/s | — |
| readdir (20,000 entries) | mem | 0.0042s | 4,816,418 /s | — |
| readdir (20,000 entries) | dir | 0.0039s | 5,128,581 /s | — |
| readdir (20,000 entries) | raw fs | 0.0070s | 2,840,890 /s | — |
| unlink 20,000 files | mem | 0.032s | 625,437 ops/s | — |
| unlink 20,000 files | dir | 0.515s | 38,864 ops/s | — |
| unlink 20,000 files | raw fs | 0.492s | 40,628 ops/s | — |
| mkdir 4,000 dirs | mem | 0.0052s | 771,085 ops/s | — |
| mkdir 4,000 dirs | dir | 0.177s | 22,551 ops/s | — |
| mkdir 4,000 dirs | raw fs | 0.137s | 29,251 ops/s | — |
| write 1 / 16 / 64 MB | mem | — | — | 2,937 / 1,483 / 1,087 |
| write 1 / 16 / 64 MB | raw fs (no fsync) | — | — | 2,463 / 3,478 / 3,496 |
| write 1 / 16 / 64 MB | raw+fsync | — | — | 42 / 702 / 776 |
| random-read 20,000 entries | pack (mmap'd) | 0.0082s | 2,445,144 ops/s | 298.5 |
| random-read 20,000 entries | raw fs | 0.166s | 120,709 ops/s | 14.7 |
| compact 20,000 entries to pack | pack | 0.025s | 814,864 ops/s | 99.5 |
| concurrent create+read+unlink (8×4,000×3) | mem | 0.281s | 341,972 ops/s | — |
| concurrent create+read+unlink (8×4,000×3) | raw fs | 6.298s | 15,243 ops/s | — |
Everything not involving bulk `mem`/`dir` structural writes (reads, `stat`,
`readdir`, large sequential I/O, pack random-access, compaction) is
unchanged within normal run-to-run noise, as expected — the treap rewrite
only touches the structural-write path (Section 5.3), not content reads,
content writes, or the pack format.
## 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
and superseded by "Resolution" above where it no longer applies.
### Where PackFS wins clearly
- **Reads of small files: 20–23x faster than both raw fs and the `dir`
backend** (2.8M ops/s vs ~120K ops/s). No syscall per read — `mem`
reads are a binary-search lookup plus a `memcpy` out of an
backend** (2.4M ops/s vs ~125K 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`: ~11x faster than raw fs.** Same reason — no syscall, and the
index is sorted for O(log n) lookup rather than requiring a directory
entry scan.
- **Random-access reads against a compacted pack: ~21x faster than raw fs**
(2.35M ops/s vs 113K ops/s), because the pack is one `mmap`'d file with a
- **`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.
- **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
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**: 84 MB/s / 687K entries/s to serialize the
- **Compaction throughput**: ~100 MB/s / ~815K 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 (29ms).
- **Large sequential writes below roughly 4–8 MB total: 2.5x faster than
page-cache-buffered raw fs** (6,687 MB/s vs 2,685 MB/s at 1 MB) — pure
`malloc`+`memcpy` beats even an unsynced `write()` syscall at this size.
20,000-file, 2.5 MB tree is not a practically-felt pause (25ms).
- **Bulk create/unlink/mkdir: now 10–330x faster than the pre-fix `mem`/
`dir` numbers, and (for `mem`) 15–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: now 22x faster than raw fs**
(was 6x *slower* before the fix) — see "Resolution."
### Where raw fs wins clearly, and why that's expected, not a bug
### Where raw fs still wins, and why that's expected, not a bug
- **Bulk create/unlink of many files is 9–22x *slower* on `mem` and `dir`
than on raw fs** (2,093 and 1,662 ops/s vs 19,836 ops/s for create;
1,894 and 1,964 vs 41,823 for unlink). This is not incidental overhead —
it is the direct, predictable cost of Section 5.3's design: every
structural write (`create`, `unlink`, `mkdir`) builds a **complete copy**
of the snapshot's sorted entry array before publishing it. `concept.md`
states this tradeoff explicitly and names its own limit: "at
agent/sandbox scale the index is small enough that a full copy... is
acceptable; a persistent (structurally shared) tree structure is not
required until this assumption is empirically violated" (Section 5.3).
**This benchmark is that empirical violation.** At N=20,000 sequential
creates, the total cost of copying an array that grows from 0 to 20,000
entries is quadratic in the file count, not linear — confirmed
quantitatively, not just asserted: `mkdir` at N=4,000 (a 5x smaller N,
same structural-write mechanism) takes 0.336s, while `create` at
N=20,000 takes 9.56s — a 28.4x slowdown for a 5x increase in N.
O(n) scaling predicts a 5.0x slowdown; O(n²) predicts 25.0x. The
observed 28.4x is close to the quadratic prediction and nowhere near
the linear one. **If bulk sequential creation/deletion of tens of
thousands of files is a workload this project needs to support well,
the index needs to stop being "copy the whole array per write" — this
is not a proposal to do that rewrite, only a record that the
spec's own stated trigger condition for reconsidering it has now been
measured, not merely hypothesized.**
- **`dir` backend `stat` is ~2.2x *slower* than raw fs `stat`** (132K ops/s
vs 284K ops/s) — because `pfs_dir_statat` (Section 6.2's containment)
- **`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)
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,
separate from and smaller than the containment cost paid on `open`
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`), because
this container's overlay filesystem has poor per-call `fsync` latency.
@@ -126,31 +217,22 @@ above for brevity, is reproducible with `make bench`.
misread as "PackFS's journal must be this slow too." The journal
(Section 4.4) does call `fsync` once per journaled write for the same
durability reason raw `fsync` is slow here, which is a real, inherited
cost on this kind of storage — not a PackFS-specific one.
- **Large writes above ~16 MB: raw fs is ~3x faster than `mem`** (3,413–
3,436 MB/s vs 1,130–1,158 MB/s). Section 5.7's buffer-growth discipline
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
(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
pages to the page cache, never re-copying prior ones. The crossover is
visible in the data: `mem` beats raw fs at 1 MB (6,687 vs 2,685 MB/s)
and loses to it by 16 MB (1,158 vs 3,413 MB/s) — the safety property
Section 5.7 requires (no reader can ever see a freed buffer) has a real,
quantifiable cost for very large sequential writes, traded for
correctness under concurrent access that a plain in-place realloc
would not have.
- **Concurrent mixed create+read+unlink: raw fs is ~6x faster** (15,686
vs 2,614 ops/s). This workload is create/unlink-dominated (two-thirds
of each thread's three operations per iteration are structural writes),
which is exactly the case just shown to be `mem`'s weakest point,
further serialized through the single-writer lock (Section 5.3) across
all 8 threads. This is not a counterexample to "wait-free readers" —
reads within this same run are still wait-free — it is a demonstration
that the single-writer design optimizes for read-heavy concurrent
workloads specifically, not for concurrent bulk metadata churn, exactly
as Section 5.1 frames the goal ("a documented, well-understood
concurrency pattern," modeled on LMDB, which makes the identical
tradeoff for the identical reason).
pages to the page cache, never re-copying prior ones. This is entirely
independent of the index structure (Section 5.7's buffer-growth
discipline governs a `MutCell`'s content buffer, not the index the
treap rewrite replaced) and is unaffected by this fix — the safety
property Section 5.7 requires (no reader can ever see a freed buffer)
still has a real, quantifiable cost for very large sequential writes,
traded for correctness under concurrent access that a plain in-place
realloc would not have.
## Reproducing
CLAUDE.md
+8 -8
View File
@@ -24,15 +24,15 @@ Sanitizer builds are not wired into `make test` (they need per-file compilation
- `include/packfs.h` — the entire public API.
- `src/internal.h` — every internal type shared across `.c` files; read this first when touching implementation code.
- `src/vfs.c` — the `Vfs` mount table itself (`MountSnapshot`, refcounted, atomically swapped) and the public API's dispatch-by-longest-prefix-match.
- `src/upper.c` — the writable layer shared by `mem`, `dir`, and the overlay's upper side: `UpperSnapshot`/`UpperEntry`/`MutCell`, the structural-write functions (`upper_create`/`upper_mkdir`/`upper_remove`/`upper_rename`/`upper_copy_up`), the content-write fast path (`upper_cell_read`/`upper_cell_write`), and the standalone `mem`/`dir` `Backend` (`backend_mem_new`/`backend_dir_new`).
- `src/upper.c` — the writable layer shared by `mem`, `dir`, and the overlay's upper side: `UpperSnapshot`/`UpperEntry`/`MutCell`, the persistent-treap index (`TreapNode` and `treap_*`/`node_*` — see "Known performance characteristics" below for why it's a treap and not a flat array), the structural-write functions (`upper_create`/`upper_mkdir`/`upper_remove`/`upper_rename`/`upper_copy_up`), the content-write fast path (`upper_cell_read`/`upper_cell_write`), the range-query API (`upper_visit_range`, used by `readdir` in both this file and `overlay.c`), and the standalone `mem`/`dir` `Backend` (`backend_mem_new`/`backend_dir_new`).
- `src/overlay.c` — composes a `Pack` (lower, read-only) with an `UpperStore` (upper): copy-up, whiteouts, the merged view (`overlay_stat`/`overlay_readdir`), the append journal, and `vfs_sync`'s compaction.
- `src/pack.c` — the on-disk pack format: load-time integrity validation, binary-search lookup/range-query, the compaction writer (atomic rename, exact-duplicate elimination), and the standalone read-only `pack` `Backend` (`backend_pack_new`) for mounting a pack with no writable upper layer at all.
- `src/containment.c` — `dir`-mount path containment (`openat2`/`O_NOFOLLOW` fallback) and opt-in Landlock hardening.
- `src/path.c` — virtual-namespace `.`/`..` canonicalization.
- `src/hash.c` — FNV-1a64, used for pack checksums and compaction dedup.
- `tests/` — one binary per concern (`test_mem`, `test_dir`, `test_pack_overlay`, `test_concurrency`); `test_harness.h` is a small assertion-macro header, not a framework, consistent with the zero-dependency constraint.
- `tests/` — one binary per concern (`test_mem`, `test_dir`, `test_pack_overlay`, `test_concurrency`, `test_index_stress` — the treap's correctness under thousands of randomized structural operations, cross-checked against an independent reference model); `test_harness.h` is a small assertion-macro header, not a framework, consistent with the zero-dependency constraint.
**One architectural fact spans every mutable structure and is easy to miss reading any single file in isolation:** `MountSnapshot` (`vfs.c`), `UpperSnapshot` (`upper.c`), and a `mem`-backed `MutCell`'s buffer (`upper.c`, Section 5.7) are all retired through the same two-step pattern — publish the replacement via `atomic_store_explicit(..., memory_order_release)`, then free the superseded object only under a dedicated `reclaim_gate` rwlock's *write* side, while every acquirer takes that same gate's *read* side around its own load-then-increment. This exists because a plain "load a pointer, then atomically increment its refcount" leaves a real gap between those two steps in which a concurrent writer can free the very object being acquired — confirmed by ASan as an actual heap-use-after-free during development, not a theoretical concern. See the comment on `UpperStore.reclaim_gate` in `internal.h` for the full reasoning. Any new refcounted, concurrently-reclaimed structure added to this codebase needs the same gate, not just an atomic pointer and a naive refcount.
**One architectural fact spans every mutable structure and is easy to miss reading any single file in isolation:** `MountSnapshot` (`vfs.c`), `UpperSnapshot` (`upper.c`), and a `mem`-backed `MutCell`'s buffer (`upper.c`, Section 5.7) are all retired through the same two-step pattern — publish the replacement via `atomic_store_explicit(..., memory_order_release)`, then free the superseded object only under a dedicated `reclaim_gate` rwlock's *write* side, while every acquirer takes that same gate's *read* side around its own load-then-increment. This exists because a plain "load a pointer, then atomically increment its refcount" leaves a real gap between those two steps in which a concurrent writer can free the very object being acquired — confirmed by ASan as an actual heap-use-after-free during development, not a theoretical concern. See the comment on `UpperStore.reclaim_gate` in `internal.h` for the full reasoning. Any new refcounted, concurrently-reclaimed structure added to this codebase needs the same gate, not just an atomic pointer and a naive refcount. The one exception, and the reason it's an exception rather than a hole: `TreapNode`'s own per-node refcounting (`upper.c`) does *not* need its own `reclaim_gate`, because nothing ever acquires a `TreapNode*` independently the way `upper_acquire` acquires an `UpperSnapshot*` — a reader only ever reaches tree nodes by walking `s->root` after already holding a valid `UpperSnapshot` reference, which transitively keeps the whole tree alive for the reader's purposes; `node_ref`/`node_unref` are called only by the single writer (under `writer_lock`) and by `upper_release`'s cascade (already inside `reclaim_gate`'s protection at the snapshot level), never by a reader racing a writer the way the snapshot pointer itself is raced.
## What this project is
@@ -85,21 +85,21 @@ 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:
- **Bulk sequential `create`/`unlink`/`mkdir` on `mem` or `dir` is O(n²) in the number of entries, confirmed empirically, not just by inspection of Section 5.3's design.** Every structural write copies the entire snapshot entry array (Section 5.3's single-writer snapshot model) before publishing it; at 20,000 sequential creates this measured ~9–22x slower than the same operations against the raw host filesystem. `concept.md` Section 5.3 names the exact condition under which this stops being acceptable — "a persistent (structurally shared) tree structure is not required until this assumption is empirically violated" — and `BENCH.md` is the record that it now has been, at a scale (tens of thousands of files) plausibly within reach of a real agent/sandbox workload, not merely a stress-test artifact. This is not a defect to silently work around; it is a known, quantified limitation of the current index structure. Anyone planning to make this codebase handle bulk file-count-heavy workloads well should read `BENCH.md`'s analysis before assuming a small, targeted fix will do — the fix is a different index data structure (structurally-shared, not full-copy), which is a real redesign of `upper.c`'s `snapshot_upsert`/`snapshot_remove`, not a tuning knob.
- **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.
- **`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.
## Self-evaluation
**Methodology.** This file was checked against the repository's actual state (`make test` passing; `include/`, `src/`, `tests/` present and matching the description below; confirmed by directory listing and a live build), against `concept.md` as frozen (for factual consistency of the constraints and architecture summarized above), and against the documentation standard stated in this file. This revision followed a documentation-completeness audit that cross-checked every function declared in `include/packfs.h` against `nm -D libpackfs.so.0`, which found one real gap (see below) before any prose was written or re-checked.
**Methodology.** This file was checked against the repository's actual state (`make test` passing, including a new `test_index_stress`; `include/`, `src/`, `tests/`, `bench/` present and matching the description below; confirmed by directory listing and a live build), against `concept.md` as frozen (for factual consistency of the constraints and architecture summarized above), and against the documentation standard stated in this file. This revision followed a rewrite of `upper.c`'s index structure (flat array to persistent treap) undertaken specifically to close the O(n²) finding this file's "Known performance characteristics" section previously reported as an open, unresolved limitation — the rewrite was verified under `make test`, `test_index_stress`'s randomized correctness check, ASan/UBSan (per the sanitizer rule below), and a full before/after `make bench` run, not merely believed correct because the code compiled.
| Category | Grade | Notes |
|---|---|---|
| Factual accuracy | A | Build/test commands and the code map were verified against a live `make test` run and the actual file layout, not written from memory of intent. |
| Factual accuracy | A | Build/test commands and the code map were verified against a live `make test` run and the actual file layout, not written from memory of intent; the performance-characteristics section was updated from a live `make bench` re-run, not assumed fixed because the rewrite was intended to fix it. |
| Adherence to the documentation standard | A | Direct, constraint-labeled prose; no manufactured sections; scaled appropriately to an instructions file rather than imitating `concept.md`'s full academic structure. |
| Completeness for its purpose | A | Covers repository status, build/test/lint commands, a per-file code map, every hard constraint from `concept.md` relevant to implementation work, the permanent zip/tar exclusion, the cross-cutting `reclaim_gate` pattern, and the empirically-measured O(n²) bulk-write characteristic — none of which any single file's comments fully explain on their own. |
| Completeness for its purpose | A | Covers repository status, build/test/lint commands, a per-file code map, every hard constraint from `concept.md` relevant to implementation work, the permanent zip/tar exclusion, the cross-cutting `reclaim_gate` pattern (including why `TreapNode` is a deliberate exception to it, not a hole in it), and the now-resolved O(n²) bulk-write characteristic with a pointer to its fix and verification. |
| Avoidance of generic or invented content | A | No fabricated "Common Development Tasks" or "Tips" sections; the sanitizer-testing instruction is stated as a requirement precisely because skipping it once already let a real bug through, not as generic advice. |
**Overall grade: A.** The file states only what is verifiably true of the repository, the spec, and the implementation; labels constraints by strength; and documents the one architectural pattern (snapshot reclamation via `reclaim_gate`) that spans multiple files and would otherwise have to be rediscovered by reading `vfs.c` and `upper.c` side by side.
**Overall grade: A.** The file states only what is verifiably true of the repository, the spec, and the implementation; labels constraints by strength; documents the architectural pattern (snapshot reclamation via `reclaim_gate`) that spans multiple files; and — unlike the previous revision, which correctly recorded a real limitation but left it as a known gap — now records that limitation's actual, measured resolution, with the same empirical rigor (a complexity-class re-confirmation, not just "it got faster") that found it in the first place.
**A real gap this audit found and fixed, not just documented:** `backend_pack_new` was declared in `include/packfs.h` and referenced in this file's own architecture map, but was never implemented in `src/pack.c` — any program calling it would fail at link time. It is now implemented (a standalone, read-only `pack` `Backend`; every mutating operation returns `VFS_ERR_PERM`), covered by a new case in `tests/test_pack_overlay.c`, and verified under `-fsanitize=undefined` per this file's own testing rule. This is recorded here because it is exactly the kind of error "document literally all" is supposed to catch: a documentation pass that describes an unimplemented function accurately is still wrong in a way that matters.
CONTRIBUTING.md
+11 -6
View File
@@ -28,12 +28,17 @@ Any change to the concurrency-sensitive files (`upper.c`, `overlay.c`,
a data race there is exactly the class of bug Section 5 of `concept.md`
exists to prevent, and the plain build will not surface it.
A change to the index structure in `upper.c` (`snapshot_upsert`,
`snapshot_remove`, or the `UpperSnapshot`/`UpperEntry` representation)
should be run through `make bench` before and after, not just `make test`:
`BENCH.md` records a real, measured O(n²) cost in bulk sequential
create/unlink that any such change is liable to affect, for better or
worse, in ways the correctness-only test suite cannot detect.
A change to the index structure in `upper.c` (the `TreapNode`/`treap_*`/
`node_*` functions, `snapshot_upsert`, `snapshot_remove`, or the
`UpperSnapshot`/`UpperEntry` representation) should be run through
`make bench` before and after, not just `make test`: `BENCH.md` records a
real, measured O(n²) cost the flat array this index used to be had in bulk
sequential create/unlink, and the persistent treap that replaced it exists
specifically to avoid regressing back into it — a correctness-preserving
change to this code can still reintroduce that regression (e.g. an
implementation that silently degrades to a linked list under adversarial
insertion order) in a way `make test` alone cannot detect, only `make bench`
plus `tests/test_index_stress.c`'s randomized-order correctness check can.
## Scope
README.md
+11 -7
View File
@@ -69,13 +69,17 @@ process's lifetime.
`bench/bench.c` (`make bench`) measures PackFS against the host filesystem
across metadata operations (create/read/stat/readdir/unlink/mkdir), large
sequential I/O, random-access pack reads, and concurrent mixed workloads.
Full results from one run, with honest analysis of both the wins and the
losses — including a real, quantitatively-confirmed O(n²) cost in bulk
sequential file creation that `concept.md` Section 5.3 explicitly
anticipated and named the trigger condition for — are in
[`BENCH.md`](BENCH.md). Read it before quoting a number from it: what
`raw fs` vs `raw+fsync` vs `dir` each actually measure is not
interchangeable, and the file explains why.
[`BENCH.md`](BENCH.md) has the full results and honest analysis of both the
wins and the losses, including the story of a real bug this benchmark
found: an early run turned up a quantitatively-confirmed O(n²) cost in bulk
sequential file creation, which `concept.md` Section 5.3 had explicitly
anticipated and named the trigger condition for closing. That's since been
fixed (`src/upper.c`'s index is a persistent treap now, not a flat array),
re-benchmarked, and re-confirmed as an actual complexity-class change, not
just a faster constant factor — see `BENCH.md`'s "Resolution" section for
the before/after numbers. Read the whole file before quoting a number from
it: what `raw fs` vs `raw+fsync` vs `dir` each actually measure is not
interchangeable, and it explains why.
`openat2`/Landlock support (Section 6) is detected automatically at compile
time via `<sys/syscall.h>`; on kernels or platforms without them, `dir`
src/internal.h
+47 -8
View File
@@ -147,9 +147,9 @@ typedef enum { ENTRY_FILE, ENTRY_DIR_MARKER, ENTRY_WHITEOUT } UpperEntryKind;
/*
* Section 5.3's mutable metadata cell, and Section 5.7's buffer-growth
* guard. One MutCell is shared by every UpperSnapshot whose entry array
* still points at the same logical file — an ordinary vfs_write mutates
* this cell in place and never touches the snapshot pointer at all.
* guard. One MutCell is shared by every UpperSnapshot whose index still
* references the same logical file — an ordinary vfs_write mutates this
* cell in place and never touches the snapshot pointer at all.
*/
typedef struct MutCell {
pthread_mutex_t write_lock; /* serializes writers to the same file */
@@ -162,16 +162,35 @@ typedef struct MutCell {
/* UPPER_DIR: host bytes live in the host fs; nothing to store here. */
} MutCell;
/*
* The read-only view of one entry, returned by upper_lookup and
* upper_visit_range. This is deliberately also the first member of
* upper.c's private TreapNode (BENCH.md; see "Known performance
* characteristics" in CLAUDE.md for why the index is a treap and not a
* flat array), so a `TreapNode*` can be reinterpreted as `UpperEntry*`
* without a copy — callers outside upper.c never see TreapNode itself.
*/
typedef struct UpperEntry {
char *name; /* full virtual path, e.g. "/a/b.txt" */
UpperEntryKind kind;
MutCell *cell; /* NULL for ENTRY_DIR_MARKER / ENTRY_WHITEOUT */
} UpperEntry;
typedef struct TreapNode TreapNode; /* opaque outside upper.c */
/*
* The live index (Section 5.3's immutable, refcounted snapshot) is a
* persistent treap keyed by full path (Section 9.1's sort order), not a
* flat array: a structural write (`create`/`unlink`/`mkdir`/rename)
* builds only the O(log n) nodes on the path to the change and shares
* every other node, node-refcounted, with the snapshot(s) it was built
* from — rather than copying the entire index on every single write,
* which is the O(n^2)-for-n-sequential-writes behavior BENCH.md
* measured and CLAUDE.md records. `root == NULL` is the empty tree.
*/
typedef struct UpperSnapshot {
_Atomic(pfs_usize) refcount;
UpperEntry *entries; /* sorted by name, Section 9.1 */
pfs_usize count;
TreapNode *root;
} UpperSnapshot;
typedef struct UpperStore {
@@ -218,15 +237,35 @@ void upper_release(UpperSnapshot *s);
/* Structural + content operations, all against paths already normalized
* and relative to the mount (leading "/"). */
/* Binary search by full path within an already-acquired snapshot.
* Returns 1 and sets *out on hit, leaving *out untouched on a miss. */
/* Expected-O(log n) lookup by full path (a treap descent, Section 9.1's
* sort order) within an already-acquired snapshot. Returns 1 and sets
* *out on hit, leaving *out untouched on a miss. */
int upper_lookup(UpperSnapshot *s, const char *path, const UpperEntry **out);
/* True if any live (non-whiteout) entry's name starts with `path` + "/" —
* i.e. `path` is a directory that exists only implicitly, via a child,
* with no explicit entry of its own (Section 9.3). */
* with no explicit entry of its own (Section 9.3). Expected O(log n),
* short-circuiting on the first match, not a scan. */
int upper_has_children(UpperSnapshot *s, const char *path);
/*
* Visits every entry (files, directory markers, AND whiteouts — callers
* that need to distinguish them, like the overlay's merge logic, filter
* in `visit` itself) whose name lies in the half-open range [lo, hi),
* matching pack_range's convention (Section 9.1), in ascending order,
* without allocating. Expected O(log n + r) where r is the number of
* entries visited — a bounded range query, not a scan of the whole
* index, the same property Section 9.1 requires of the on-disk pack.
*/
typedef void (*UpperVisitFn)(void *ctx, const UpperEntry *e);
void upper_visit_range(UpperSnapshot *s, const char *lo, const char *hi, UpperVisitFn visit, void *ctx);
/* Builds, into `out` (at least `cap` bytes), the exclusive upper bound
* for "starts with `prefix`" (Section 9.1's convention: a byte higher
* than any valid path byte). Shared by upper.c and overlay.c, both of
* which call upper_visit_range with a prefix range. */
void pfs_prefix_upper_bound(const char *prefix, char *out, size_t cap);
/* Resets `c`'s content to empty in place (VFS_O_TRUNC on an
* already-existing file) — a content operation, not a structural one, so
* it never touches the snapshot pointer (Section 5.3). */
src/overlay.c
+37 -23
View File
@@ -355,39 +355,53 @@ static int child_index(Child *arr, pfs_usize n, const char *name, size_t len) {
return -1;
}
typedef struct OverlayReaddirCtx {
size_t plen;
Child *handled; pfs_usize hcount, hcap;
VfsDirEntry *entries; pfs_usize count, cap;
} OverlayReaddirCtx;
/* Mirrors the original array-scan loop's logic exactly, sourced from
* upper_visit_range instead of a manual index loop; still needs to see
* whiteouts (unlike upstd_readdir's callback), to suppress a pack entry
* sharing the same immediate-child name. */
static void overlay_readdir_visit(void *ctx_, const UpperEntry *e) {
OverlayReaddirCtx *ctx = (OverlayReaddirCtx *)ctx_;
const char *restp = e->name + ctx->plen;
const char *slash = strchr(restp, '/');
size_t clen = slash ? (size_t)(slash - restp) : strlen(restp);
if (clen == 0 || clen >= sizeof(ctx->handled[0].name)) return;
if (child_index(ctx->handled, ctx->hcount, restp, clen) >= 0) return;
if (ctx->hcount == ctx->hcap) { ctx->hcap = ctx->hcap ? ctx->hcap * 2 : 8; ctx->handled = (Child *)realloc(ctx->handled, ctx->hcap * sizeof(Child)); }
memcpy(ctx->handled[ctx->hcount].name, restp, clen); ctx->handled[ctx->hcount].name[clen] = '\0';
ctx->handled[ctx->hcount].kind = slash ? VFS_KIND_DIR : (e->kind == ENTRY_DIR_MARKER ? VFS_KIND_DIR : VFS_KIND_FILE);
int is_whiteout = (e->kind == ENTRY_WHITEOUT);
ctx->hcount++;
if (!is_whiteout) {
if (ctx->count == ctx->cap) { ctx->cap = ctx->cap ? ctx->cap * 2 : 8; ctx->entries = (VfsDirEntry *)realloc(ctx->entries, ctx->cap * sizeof(VfsDirEntry)); }
memcpy(ctx->entries[ctx->count].name, ctx->handled[ctx->hcount - 1].name, clen + 1);
ctx->entries[ctx->count].kind = ctx->handled[ctx->hcount - 1].kind;
ctx->count++;
}
}
static int overlay_readdir(Backend *b, const char *path, VfsDir *out) {
Overlay *ov = (Overlay *)b->state;
UpperStore *us = upper_of(ov);
char prefix[PFS_PATH_MAX];
char prefix[PFS_PATH_MAX], upper_hi[PFS_PATH_MAX];
if (strcmp(path, "/") == 0) strcpy(prefix, "/"); else snprintf(prefix, sizeof(prefix), "%s/", path);
size_t plen = strlen(prefix);
pfs_prefix_upper_bound(prefix, upper_hi, sizeof(upper_hi));
Child *handled = NULL; pfs_usize hcount = 0, hcap = 0;
VfsDirEntry *entries = NULL; pfs_usize count = 0, cap = 0;
OverlayReaddirCtx ctx; memset(&ctx, 0, sizeof(ctx)); ctx.plen = plen;
UpperSnapshot *s = upper_acquire(us);
for (pfs_usize i = 0; i < s->count; i++) {
const char *name = s->entries[i].name;
if (strncmp(name, prefix, plen) != 0) continue;
const char *restp = name + plen;
const char *slash = strchr(restp, '/');
size_t clen = slash ? (size_t)(slash - restp) : strlen(restp);
if (clen == 0 || clen >= sizeof(handled[0].name)) continue;
if (child_index(handled, hcount, restp, clen) >= 0) continue;
if (hcount == hcap) { hcap = hcap ? hcap * 2 : 8; handled = (Child *)realloc(handled, hcap * sizeof(Child)); }
memcpy(handled[hcount].name, restp, clen); handled[hcount].name[clen] = '\0';
handled[hcount].kind = slash ? VFS_KIND_DIR : (s->entries[i].kind == ENTRY_DIR_MARKER ? VFS_KIND_DIR : VFS_KIND_FILE);
int is_whiteout = (s->entries[i].kind == ENTRY_WHITEOUT);
hcount++;
if (!is_whiteout) {
if (count == cap) { cap = cap ? cap * 2 : 8; entries = (VfsDirEntry *)realloc(entries, cap * sizeof(VfsDirEntry)); }
memcpy(entries[count].name, handled[hcount - 1].name, clen + 1);
entries[count].kind = handled[hcount - 1].kind;
count++;
}
}
upper_visit_range(s, prefix, upper_hi, overlay_readdir_visit, &ctx);
upper_release(s);
Child *handled = ctx.handled; pfs_usize hcount = ctx.hcount;
VfsDirEntry *entries = ctx.entries; pfs_usize count = ctx.count, cap = ctx.cap;
if (ov->pack) {
size_t lo, hi;
pack_range(ov->pack, prefix, &lo, &hi);
src/upper.c
+303 -130
View File
@@ -60,6 +60,222 @@ static MutCell *mutcell_new(void) {
return &rc->pub;
}
/*
* ---- the persistent treap (Section 9.1's sort order; see CLAUDE.md's
* "Known performance characteristics" and BENCH.md for why this exists
* instead of the flat sorted array the design originally used) ----
*
* Chosen over a persistent AVL/red-black/weight-balanced tree because
* deletion in those needs up to O(log n) rebalancing rotations on the
* path to the root, every one of which is a real cost in a persistent
* (path-copying) setting since each rotated node must be copied, not
* just relinked in place; a treap needs only O(1) *amortized* rotations
* for both insertion and deletion (Seidel & Aragon, "Randomized Search
* Trees," 1996), and its expected O(log n) height holds regardless of
* insertion order — including the sorted-by-creation-order pattern that
* is exactly what made the flat array's full copy O(n^2) in the first
* place. Liljenzin, "Confluently Persistent Sets and Maps" (arXiv:
* 1301.3388), documents persistent treaps giving O(1) snapshots for
* MVCC specifically, which is this exact use case.
*
* Ownership convention, load-bearing for every function below: a
* TreapNode* argument passed to treap_merge/treap_split3/node_clone_with
* is CONSUMED — the caller must hand over one owned reference (via
* node_ref() if borrowing from a tree it still needs) and must not use
* that pointer again. Each such function returns a tree holding exactly
* one owned reference, which the caller now owns. This mirrors the
* "old"/"step1" ownership pattern already used at the UpperSnapshot
* level elsewhere in this file, one level down.
*/
struct TreapNode {
UpperEntry pub; /* MUST be first member: (UpperEntry*)node must be valid */
_Atomic(pfs_usize) refcount;
uint32_t priority;
pfs_usize size; /* subtree size, including self; not currently read outside treap_touch_size, kept for future rank/order-statistics use */
TreapNode *left, *right;
};
/* Node creation happens only inside structural-write functions, which
* are already serialized by UpperStore.writer_lock (Section 5.3) — there
* is never a concurrent call into this generator, so it does not need
* its own atomics or locking. xorshift32 (Marsaglia, 2003): treap
* priorities need a good average distribution, not cryptographic
* strength. */
static uint32_t treap_rand(void) {
static uint32_t state = 2463534242u; /* fixed nonzero seed */
state ^= state << 13;
state ^= state >> 17;
state ^= state << 5;
return state;
}
static pfs_usize node_size(TreapNode *n) { return n ? n->size : 0; }
static void node_touch_size(TreapNode *n) { n->size = 1 + node_size(n->left) + node_size(n->right); }
static TreapNode *node_ref(TreapNode *n) {
if (n) atomic_fetch_add_explicit(&n->refcount, 1, memory_order_relaxed);
return n;
}
/* Drops one reference; recursively unrefs children (and the cell) only
* once this node's own refcount reaches zero — a shared child's own
* refcount simply absorbs the decrement and recursion stops there,
* exactly like MutCell's and UpperSnapshot's reference counting. */
static void node_unref(TreapNode *n) {
if (!n) return;
if (atomic_fetch_sub_explicit(&n->refcount, 1, memory_order_acq_rel) == 1) {
node_unref(n->left);
node_unref(n->right);
mutcell_unref(n->pub.cell);
free(n->pub.name);
free(n);
}
}
static TreapNode *node_new(const char *name, UpperEntryKind kind, MutCell *cell) {
TreapNode *n = (TreapNode *)calloc(1, sizeof(TreapNode));
n->pub.name = strdup(name);
n->pub.kind = kind;
n->pub.cell = cell;
mutcell_ref(cell);
n->priority = treap_rand();
n->size = 1;
atomic_init(&n->refcount, 1);
return n;
}
/* Builds a new node cloning `src`'s identity (name/kind/cell/priority)
* with the given (already-owned, per the convention above) children. */
static TreapNode *node_clone_with(TreapNode *src, TreapNode *left, TreapNode *right) {
TreapNode *n = (TreapNode *)calloc(1, sizeof(TreapNode));
n->pub.name = strdup(src->pub.name);
n->pub.kind = src->pub.kind;
n->pub.cell = src->pub.cell;
mutcell_ref(n->pub.cell);
n->priority = src->priority;
n->left = left;
n->right = right;
node_touch_size(n);
atomic_init(&n->refcount, 1);
return n;
}
/* Consumes `a` and `b`; returns their merge (every key in `a` assumed
* less than every key in `b`, the standard treap merge precondition).
* node_clone_with already sizes its result from the final left/right it
* is given, so there is nothing left to recompute here. */
static TreapNode *treap_merge(TreapNode *a, TreapNode *b) {
if (!a) return b;
if (!b) return a;
if (a->priority >= b->priority) {
TreapNode *new_right = treap_merge(node_ref(a->right), b);
TreapNode *result = node_clone_with(a, node_ref(a->left), new_right);
node_unref(a);
return result;
}
TreapNode *new_left = treap_merge(a, node_ref(b->left));
TreapNode *result = node_clone_with(b, new_left, node_ref(b->right));
node_unref(b);
return result;
}
/* Consumes `t`; splits it by `key` into *lt (names < key) and *gt (names
* > key), reporting whether an exact match existed via *found. The
* matched node, if any, is released here (its cell's reference drops
* with it) — the caller never sees it, matching what the flat-array
* snapshot_remove did by simply not copying the matched slot forward. */
static void treap_split3(TreapNode *t, const char *key, TreapNode **lt, TreapNode **gt, int *found) {
if (!t) { *lt = NULL; *gt = NULL; *found = 0; return; }
int c = strcmp(t->pub.name, key);
if (c == 0) {
*found = 1;
*lt = node_ref(t->left);
*gt = node_ref(t->right);
node_unref(t);
return;
}
if (c < 0) {
TreapNode *rl, *rr; int f;
treap_split3(node_ref(t->right), key, &rl, &rr, &f);
*found = f;
*gt = rr;
*lt = node_clone_with(t, node_ref(t->left), rl);
node_unref(t);
} else {
TreapNode *rl, *rr; int f;
treap_split3(node_ref(t->left), key, &rl, &rr, &f);
*found = f;
*lt = rl;
*gt = node_clone_with(t, rr, node_ref(t->right));
node_unref(t);
}
}
/* Consumes `root`; returns a new tree with `key` set to (kind, cell),
* replacing any existing entry at that key. */
static TreapNode *treap_upsert(TreapNode *root, const char *key, UpperEntryKind kind, MutCell *cell) {
TreapNode *lt, *gt; int found;
treap_split3(root, key, &lt, &gt, &found);
TreapNode *mid = node_new(key, kind, cell);
return treap_merge(treap_merge(lt, mid), gt);
}
/* Consumes `root`; returns a new tree with `key` absent. A no-op removal
* (key not present) still returns a correct, equivalent tree — callers
* in this file only ever call this once they've confirmed the key
* exists, so this path is not currently exercised, but it is not UB or
* a silent corruption if it ever is. */
static TreapNode *treap_remove_key(TreapNode *root, const char *key) {
TreapNode *lt, *gt; int found;
treap_split3(root, key, &lt, &gt, &found);
return treap_merge(lt, gt);
}
static TreapNode *treap_lookup(TreapNode *t, const char *key) {
while (t) {
int c = strcmp(t->pub.name, key);
if (c == 0) return t;
t = c < 0 ? t->right : t->left;
}
return NULL;
}
/* Read-only BST range-query pruning (Section 9.1): recurses into a child
* only when that child's subtree could possibly contain a qualifying
* key, per the BST invariant — never allocates, never refs/unrefs,
* since the caller already holds the whole tree alive via its snapshot. */
static void treap_visit_range(TreapNode *t, const char *lo, const char *hi, UpperVisitFn visit, void *ctx) {
if (!t) return;
if (strcmp(t->pub.name, lo) >= 0) treap_visit_range(t->left, lo, hi, visit, ctx);
if (strcmp(t->pub.name, lo) >= 0 && strcmp(t->pub.name, hi) < 0) visit(ctx, &t->pub);
if (strcmp(t->pub.name, hi) < 0) treap_visit_range(t->right, lo, hi, visit, ctx);
}
/* Same pruning as treap_visit_range, short-circuiting on the first
* match instead of visiting every qualifying entry. */
static int treap_range_nonempty(TreapNode *t, const char *lo, const char *hi) {
if (!t) return 0;
if (strcmp(t->pub.name, lo) >= 0 && strcmp(t->pub.name, hi) < 0) return 1;
if (strcmp(t->pub.name, lo) >= 0 && treap_range_nonempty(t->left, lo, hi)) return 1;
if (strcmp(t->pub.name, hi) < 0 && treap_range_nonempty(t->right, lo, hi)) return 1;
return 0;
}
/* Builds the exclusive upper bound for "starts with `prefix`", matching
* pack_range's convention (Section 9.1): a byte higher than any valid
* path byte, so every string with `prefix` as a proper prefix sorts
* below it and nothing else does. Shared with overlay.c, which needs the
* same bound to call upper_visit_range for its half of a merged
* readdir. */
void pfs_prefix_upper_bound(const char *prefix, char *out, size_t cap) {
size_t plen = strlen(prefix);
if (plen + 2 > cap) plen = cap - 2;
memcpy(out, prefix, plen);
out[plen] = (char)0x7F;
out[plen + 1] = '\0';
}
/* ---- UpperSnapshot lifetime ---- */
UpperSnapshot *upper_acquire(UpperStore *u) {
@@ -83,105 +299,43 @@ static void upper_retire(UpperStore *u, UpperSnapshot *old) {
void upper_release(UpperSnapshot *s) {
if (!s) return;
if (atomic_fetch_sub_explicit(&s->refcount, 1, memory_order_acq_rel) == 1) {
for (pfs_usize i = 0; i < s->count; i++) {
free(s->entries[i].name);
mutcell_unref(s->entries[i].cell);
}
free(s->entries);
node_unref(s->root);
free(s);
}
}
int upper_lookup(UpperSnapshot *s, const char *path, const UpperEntry **out) {
pfs_usize lo = 0, hi = s->count;
while (lo < hi) {
pfs_usize mid = lo + (hi - lo) / 2;
int c = strcmp(s->entries[mid].name, path);
if (c == 0) { *out = &s->entries[mid]; return 1; }
if (c < 0) lo = mid + 1; else hi = mid;
}
return 0;
TreapNode *n = treap_lookup(s->root, path);
if (!n) return 0;
*out = &n->pub;
return 1;
}
int upper_has_children(UpperSnapshot *s, const char *path) {
char prefix[PFS_PATH_MAX];
char prefix[PFS_PATH_MAX], hi[PFS_PATH_MAX];
if (strcmp(path, "/") == 0) strcpy(prefix, "/");
else snprintf(prefix, sizeof(prefix), "%s/", path);
size_t plen = strlen(prefix);
for (pfs_usize i = 0; i < s->count; i++) {
if (s->entries[i].kind == ENTRY_WHITEOUT) continue;
if (strncmp(s->entries[i].name, prefix, plen) == 0) return 1;
}
return 0;
pfs_prefix_upper_bound(prefix, hi, sizeof(hi));
return treap_range_nonempty(s->root, prefix, hi);
}
void upper_visit_range(UpperSnapshot *s, const char *lo, const char *hi, UpperVisitFn visit, void *ctx) {
treap_visit_range(s->root, lo, hi, visit, ctx);
}
/* Builds a new snapshot with one entry inserted or replaced at `name`. */
static UpperSnapshot *snapshot_upsert(UpperSnapshot *base, const char *name,
UpperEntryKind kind, MutCell *cell) {
pfs_usize old_count = base ? base->count : 0;
pfs_usize lo = 0, hi = old_count;
int found = 0;
while (lo < hi) {
pfs_usize mid = lo + (hi - lo) / 2;
int c = strcmp(base->entries[mid].name, name);
if (c == 0) { lo = mid; found = 1; break; }
if (c < 0) lo = mid + 1; else hi = mid;
}
pfs_usize new_count = found ? old_count : old_count + 1;
UpperSnapshot *ns = (UpperSnapshot *)calloc(1, sizeof(UpperSnapshot));
ns->entries = (UpperEntry *)calloc(new_count ? new_count : 1, sizeof(UpperEntry));
ns->count = new_count;
atomic_init(&ns->refcount, 1);
pfs_usize w = 0;
for (pfs_usize i = 0; i < old_count; i++) {
if (i == lo) {
ns->entries[w].name = strdup(name);
ns->entries[w].kind = kind;
ns->entries[w].cell = cell;
mutcell_ref(cell);
w++;
if (found) continue; /* replaces base->entries[lo] */
}
ns->entries[w].name = strdup(base->entries[i].name);
ns->entries[w].kind = base->entries[i].kind;
ns->entries[w].cell = base->entries[i].cell;
mutcell_ref(ns->entries[w].cell);
w++;
}
if (lo == old_count) {
ns->entries[w].name = strdup(name);
ns->entries[w].kind = kind;
ns->entries[w].cell = cell;
mutcell_ref(cell);
w++;
}
ns->root = treap_upsert(node_ref(base ? base->root : NULL), name, kind, cell);
return ns;
}
static UpperSnapshot *snapshot_remove(UpperSnapshot *base, const char *name) {
pfs_usize lo = 0, hi = base->count;
int found = 0;
while (lo < hi) {
pfs_usize mid = lo + (hi - lo) / 2;
int c = strcmp(base->entries[mid].name, name);
if (c == 0) { lo = mid; found = 1; break; }
if (c < 0) lo = mid + 1; else hi = mid;
}
if (!found) return NULL;
UpperSnapshot *ns = (UpperSnapshot *)calloc(1, sizeof(UpperSnapshot));
ns->count = base->count - 1;
ns->entries = ns->count ? (UpperEntry *)calloc(ns->count, sizeof(UpperEntry)) : NULL;
atomic_init(&ns->refcount, 1);
pfs_usize w = 0;
for (pfs_usize i = 0; i < base->count; i++) {
if (i == lo) continue;
ns->entries[w].name = strdup(base->entries[i].name);
ns->entries[w].kind = base->entries[i].kind;
ns->entries[w].cell = base->entries[i].cell;
mutcell_ref(ns->entries[w].cell);
w++;
}
ns->root = treap_remove_key(node_ref(base->root), name);
return ns;
}
@@ -465,39 +619,49 @@ void upper_cell_truncate(UpperStore *u, MutCell *c, const char *path) {
/* ---- compaction support ---- */
void upper_walk_live(UpperStore *u, const UpperWalkCb *cb) {
UpperSnapshot *s = upper_acquire(u);
for (pfs_usize i = 0; i < s->count; i++) {
UpperEntry *e = &s->entries[i];
if (e->kind == ENTRY_WHITEOUT) continue;
if (e->kind == ENTRY_DIR_MARKER) {
cb->visit(cb->ctx, e->name, ENTRY_DIR_MARKER, NULL, 0, 0);
continue;
}
/* In-order (= sorted-by-name) full traversal for compaction. Unlike
* treap_visit_range, this needs every entry including whiteouts filtered
* here rather than left to the caller, and needs per-entry I/O (reading
* mem's buffer or a dir file's content) that doesn't fit the generic
* UpperVisitFn shape, so it is its own recursive walker rather than a
* caller of upper_visit_range. */
static void walk_node(UpperStore *u, TreapNode *n, const UpperWalkCb *cb) {
if (!n) return;
walk_node(u, n->left, cb);
if (n->pub.kind == ENTRY_DIR_MARKER) {
cb->visit(cb->ctx, n->pub.name, ENTRY_DIR_MARKER, NULL, 0, 0);
} else if (n->pub.kind == ENTRY_FILE) {
if (u->kind == UPPER_MEM) {
pthread_rwlock_rdlock(&e->cell->buf_lock);
pfs_usize size = atomic_load_explicit(&e->cell->size, memory_order_acquire);
cb->visit(cb->ctx, e->name, ENTRY_FILE, e->cell->data, size,
atomic_load_explicit(&e->cell->mtime, memory_order_acquire));
pthread_rwlock_unlock(&e->cell->buf_lock);
pthread_rwlock_rdlock(&n->pub.cell->buf_lock);
pfs_usize size = atomic_load_explicit(&n->pub.cell->size, memory_order_acquire);
cb->visit(cb->ctx, n->pub.name, ENTRY_FILE, n->pub.cell->data, size,
atomic_load_explicit(&n->pub.cell->mtime, memory_order_acquire));
pthread_rwlock_unlock(&n->pub.cell->buf_lock);
} else {
int err = 0;
int fd = pfs_dir_openat(u->root_fd, rel(e->name), O_RDONLY, 0, &err);
if (fd < 0) continue;
struct stat st;
fstat(fd, &st);
void *buf = st.st_size ? malloc((size_t)st.st_size) : NULL;
size_t total = 0;
while (buf && total < (size_t)st.st_size) {
ssize_t r = read(fd, (char *)buf + total, (size_t)st.st_size - total);
if (r <= 0) break;
total += (size_t)r;
int fd = pfs_dir_openat(u->root_fd, rel(n->pub.name), O_RDONLY, 0, &err);
if (fd >= 0) {
struct stat st;
fstat(fd, &st);
void *buf = st.st_size ? malloc((size_t)st.st_size) : NULL;
size_t total = 0;
while (buf && total < (size_t)st.st_size) {
ssize_t r = read(fd, (char *)buf + total, (size_t)st.st_size - total);
if (r <= 0) break;
total += (size_t)r;
}
close(fd);
cb->visit(cb->ctx, n->pub.name, ENTRY_FILE, buf, total, (int64_t)st.st_mtime);
free(buf);
}
close(fd);
cb->visit(cb->ctx, e->name, ENTRY_FILE, buf, total, (int64_t)st.st_mtime);
free(buf);
}
}
} /* ENTRY_WHITEOUT: skipped, as before */
walk_node(u, n->right, cb);
}
void upper_walk_live(UpperStore *u, const UpperWalkCb *cb) {
UpperSnapshot *s = upper_acquire(u);
walk_node(u, s->root, cb);
upper_release(s);
}
@@ -604,6 +768,35 @@ static int upstd_stat(Backend *b, const char *path, VfsStat *out) {
return rc < 0 ? err : VFS_OK;
}
typedef struct ReaddirCtx {
size_t plen;
VfsDirEntry *entries;
pfs_usize count, cap;
} ReaddirCtx;
/* upper_visit_range already restricts us to [prefix, prefix-upper-bound)
* in sorted order, so "is this a duplicate immediate child" only ever
* needs to check the immediately-preceding output entry, not a scan. */
static void readdir_visit(void *ctx_, const UpperEntry *e) {
ReaddirCtx *ctx = (ReaddirCtx *)ctx_;
if (e->kind == ENTRY_WHITEOUT) return;
const char *restp = e->name + ctx->plen;
const char *slash = strchr(restp, '/');
size_t clen = slash ? (size_t)(slash - restp) : strlen(restp);
if (clen == 0 || clen >= sizeof(ctx->entries[0].name)) return;
if (ctx->count > 0 && strncmp(ctx->entries[ctx->count - 1].name, restp, clen) == 0 &&
ctx->entries[ctx->count - 1].name[clen] == '\0') return;
if (ctx->count == ctx->cap) {
ctx->cap = ctx->cap ? ctx->cap * 2 : 8;
ctx->entries = (VfsDirEntry *)realloc(ctx->entries, ctx->cap * sizeof(VfsDirEntry));
}
memcpy(ctx->entries[ctx->count].name, restp, clen);
ctx->entries[ctx->count].name[clen] = '\0';
ctx->entries[ctx->count].kind = slash ? VFS_KIND_DIR
: (e->kind == ENTRY_DIR_MARKER ? VFS_KIND_DIR : VFS_KIND_FILE);
ctx->count++;
}
static int upstd_readdir(Backend *b, const char *path, VfsDir *out) {
UpperStore *u = (UpperStore *)b->state;
UpperSnapshot *s = upper_acquire(u);
@@ -616,36 +809,16 @@ static int upstd_readdir(Backend *b, const char *path, VfsDir *out) {
return self_found ? VFS_ERR_NOTDIR : VFS_ERR_NOENT;
}
char prefix[PFS_PATH_MAX];
char prefix[PFS_PATH_MAX], hi[PFS_PATH_MAX];
if (strcmp(path, "/") == 0) strcpy(prefix, "/");
else snprintf(prefix, sizeof(prefix), "%s/", path);
size_t plen = strlen(prefix);
pfs_prefix_upper_bound(prefix, hi, sizeof(hi));
VfsDirEntry *entries = NULL;
pfs_usize count = 0, cap = 0;
for (pfs_usize i = 0; i < s->count; i++) {
if (s->entries[i].kind == ENTRY_WHITEOUT) continue;
const char *name = s->entries[i].name;
if (strncmp(name, prefix, plen) != 0) continue;
const char *restp = name + plen;
const char *slash = strchr(restp, '/');
size_t clen = slash ? (size_t)(slash - restp) : strlen(restp);
if (clen == 0 || clen >= sizeof(entries[0].name)) continue;
if (count > 0 && strncmp(entries[count - 1].name, restp, clen) == 0 &&
entries[count - 1].name[clen] == '\0') continue; /* sorted -> dup is adjacent */
if (count == cap) {
cap = cap ? cap * 2 : 8;
entries = (VfsDirEntry *)realloc(entries, cap * sizeof(VfsDirEntry));
}
memcpy(entries[count].name, restp, clen);
entries[count].name[clen] = '\0';
entries[count].kind = slash ? VFS_KIND_DIR
: (s->entries[i].kind == ENTRY_DIR_MARKER ? VFS_KIND_DIR : VFS_KIND_FILE);
count++;
}
ReaddirCtx ctx = { strlen(prefix), NULL, 0, 0 };
upper_visit_range(s, prefix, hi, readdir_visit, &ctx);
upper_release(s);
out->entries = entries;
out->count = count;
out->entries = ctx.entries;
out->count = ctx.count;
return VFS_OK;
}
tests/test_index_stress.c
+179
View File
@@ -0,0 +1,179 @@
/*
* test_index_stress.c — correctness of the persistent treap index
* (Section 9.1's sort order; see CLAUDE.md's "Known performance
* characteristics" and BENCH.md for why it replaced a flat array) under
* heavy, randomized structural churn, cross-checked against an
* independent reference model rather than just "did it not crash."
*
* This exercises exactly the code path BENCH.md's O(n^2) finding and its
* fix live in: upper.c's snapshot_upsert/snapshot_remove and the treap
* (split3/merge, node refcounting) beneath them, at a scale (thousands
* of entries, non-sequential insertion/deletion order, nested
* directories, renames) the other test files don't reach.
*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "packfs.h"
#include "test_harness.h"
#define N 8000
static int g_alive[N];
static void name_for(int id, char *buf, size_t cap) {
/* deliberately nested, to exercise readdir's prefix-range logic at
* more than one level, not just a flat root directory */
snprintf(buf, cap, "/d%03d/f%06d.dat", id % 50, id);
}
static void shuffle(int *order, int n) {
for (int i = n - 1; i > 0; i--) {
int j = rand() % (i + 1);
int t = order[i]; order[i] = order[j]; order[j] = t;
}
}
static void create_one(Vfs *v, int id) {
char path[64], parent[16];
name_for(id, path, sizeof(path));
snprintf(parent, sizeof(parent), "/d%03d", id % 50);
int err = 0;
if (vfs_stat(v, parent, &(VfsStat){0}) != VFS_OK) vfs_mkdir(v, parent);
VfsFile *f = vfs_open(v, path, VFS_O_WRONLY | VFS_O_CREAT | VFS_O_TRUNC, &err);
CHECK(f != NULL);
if (!f) return;
char payload[32];
int n = snprintf(payload, sizeof(payload), "id=%d", id);
CHECK_EQ_INT(vfs_write(f, payload, (pfs_usize)n), n);
vfs_close(f);
g_alive[id] = 1;
}
static void delete_one(Vfs *v, int id) {
char path[64];
name_for(id, path, sizeof(path));
CHECK_EQ_INT(vfs_unlink(v, path), VFS_OK);
g_alive[id] = 0;
}
/* Cross-checks the live tree against g_alive[]: every alive id must
* stat successfully with the right content; every dead id must NOENT;
* and the total count reachable via nested readdir must match the
* reference model's count exactly (not just "at least"). */
static void verify_consistency(Vfs *v) {
int expected_total = 0;
for (int id = 0; id < N; id++) {
char path[64];
name_for(id, path, sizeof(path));
VfsStat st;
int rc = vfs_stat(v, path, &st);
if (g_alive[id]) {
expected_total++;
CHECK_EQ_INT(rc, VFS_OK);
if (rc == VFS_OK) {
CHECK_EQ_INT(st.kind, VFS_KIND_FILE);
int err = 0;
VfsFile *f = vfs_open(v, path, VFS_O_RDONLY, &err);
CHECK(f != NULL);
if (f) {
char buf[32] = {0}, expect[32];
vfs_read(f, buf, sizeof(buf) - 1);
snprintf(expect, sizeof(expect), "id=%d", id);
CHECK_STR_EQ(buf, expect);
vfs_close(f);
}
}
} else {
CHECK_EQ_INT(rc, VFS_ERR_NOENT);
}
}
/* Full nested readdir sweep: sum of children across every /dNNN that
* still has any live file must equal the reference model's count.
* This exercises upper_visit_range's prefix pruning at every
* directory, not just the root. */
int counted = 0;
for (int d = 0; d < 50; d++) {
char dirpath[16];
snprintf(dirpath, sizeof(dirpath), "/d%03d", d);
VfsDir dir;
if (vfs_readdir(v, dirpath, &dir) == VFS_OK) {
counted += (int)dir.count;
vfs_dir_free(&dir);
}
}
CHECK_EQ_INT(counted, expected_total);
}
int main(void) {
srand(20260914);
Vfs *v = vfs_new();
Backend *mem = backend_mem_new();
CHECK_EQ_INT(vfs_mount(v, "/", mem), VFS_OK);
int order[N];
for (int i = 0; i < N; i++) order[i] = i;
/* round 1: create everything, in randomized (non-sequential) order —
* the exact pattern that made the flat array O(n^2); the treap must
* both stay correct and (implicitly, via this test completing in
* reasonable time under sanitizers) stay fast. */
shuffle(order, N);
for (int i = 0; i < N; i++) create_one(v, order[i]);
verify_consistency(v);
/* round 2: delete a random 40%, in a different random order */
shuffle(order, N);
for (int i = 0; i < N * 2 / 5; i++) delete_one(v, order[i]);
verify_consistency(v);
/* round 3: recreate half of what was just deleted, delete a
* different random slice of what's still alive, interleaved by
* iterating one combined shuffled order over "toggle this id" */
shuffle(order, N);
for (int i = 0; i < N; i++) {
int id = order[i];
if (id % 3 == 0) {
if (g_alive[id]) delete_one(v, id); else create_one(v, id);
}
}
verify_consistency(v);
/* renames: move a sample of live files to a fresh, previously-unused
* path, verify old path gone / new path present with right content */
for (int id = 0; id < N; id += 37) {
if (!g_alive[id]) continue;
char from[64], to[80];
name_for(id, from, sizeof(from));
snprintf(to, sizeof(to), "%s.moved", from);
CHECK_EQ_INT(vfs_rename(v, from, to), VFS_OK);
VfsStat st;
CHECK_EQ_INT(vfs_stat(v, from, &st), VFS_ERR_NOENT);
CHECK_EQ_INT(vfs_stat(v, to, &st), VFS_OK);
CHECK_EQ_INT(vfs_rename(v, to, from), VFS_OK); /* move it back so verify_consistency's model still holds */
}
verify_consistency(v);
/* final: delete everything still alive, in yet another random order;
* the tree must end up empty and every readdir empty. */
shuffle(order, N);
for (int i = 0; i < N; i++) if (g_alive[order[i]]) delete_one(v, order[i]);
verify_consistency(v);
for (int d = 0; d < 50; d++) {
char dirpath[16];
snprintf(dirpath, sizeof(dirpath), "/d%03d", d);
VfsDir dir;
if (vfs_readdir(v, dirpath, &dir) == VFS_OK) {
CHECK_EQ_INT(dir.count, 0);
vfs_dir_free(&dir);
}
}
vfs_unmount(v, "/");
backend_free(mem);
vfs_free(v);
TEST_MAIN_END();
}