diff --git a/Cargo.lock b/Cargo.lock index dde6a649..df91d394 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4131,6 +4131,7 @@ dependencies = [ "fastcdc", "file-rotate", "flate2", + "foldhash 0.2.0", "fs2", "futures", "hex", diff --git a/Cargo.toml b/Cargo.toml index 00e65fd4..c9f7dbd0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -85,6 +85,10 @@ infer = "0.19" async-compression = { version = "0.4.42", features = ["tokio", "gzip"] } async_zip = { version = "0.0.18", features = ["tokio", "deflate"] } dashmap = "6.2.1" +# Fast, DoS-resistant (per-instance random-seeded) hasher for trusted- and +# attacker-controlled internal maps/sets. Already present transitively via +# hashbrown, so this direct dep adds no new compiled crate (benches/ROUND26.md §G1). +foldhash = "0.2" socket2 = { version = "0.6.4", features = ["all"] } urlencoding = "2.1.3" utoipa = { version = "5.5.0", features = ["axum_extras", "uuid", "chrono"] } @@ -354,6 +358,64 @@ name = "bench_micro_allocs" path = "examples/bench_micro_allocs.rs" required-features = ["bench"] +# Round-27 battery ──────────────────────────────────────────────────────────── + +# Round-27 CPU/alloc micro-pack (no Postgres) — NC PROPFIND per-row oc:id String +# → reused buffer via format_oc_id_into (H1); contact create/update JSONB write +# through a throwaway serde_json::Value DOM → sqlx::types::Json(&dtos) direct +# serialize (P2, the write-side twin of §J1). +[[example]] +name = "bench_round27_micro" +path = "examples/bench_round27_micro.rs" +required-features = ["bench"] + +# Round-26 battery ──────────────────────────────────────────────────────────── + +# Round-26 CPU/alloc micro-pack (no Postgres) — drive-policy JSONB decode through +# a throwaway serde_json::Value DOM → from_slice:: (P1, the §J1 +# pattern applied to the drive-policy path §J2 left behind). +[[example]] +name = "bench_round26_micro" +path = "examples/bench_round26_micro.rs" +required-features = ["bench"] + +# Round-26 disk-I/O pack — CachedBlobBackend redundant per-write create_dir_all +# on warm shards → pre-create the 256 shard dirs at init (D1). (D2, moving the +# eviction unlink off the reactor via spawn_blocking, was tested and REVERTED — +# spawn_blocking dispatch costs more than the fast local unlink; see ROUND26.md.) +[[example]] +name = "bench_round26_diskio" +path = "examples/bench_round26_diskio.rs" +required-features = ["bench"] + +# Round-26 hasher pack — delta-upload have/need hash sets: SipHash → foldhash +# (per-instance random-seeded, DoS-safe for the attacker-controlled hashes) (G1). +[[example]] +name = "bench_round26_hasher" +path = "examples/bench_round26_hasher.rs" +required-features = ["bench"] + +# Round-25 battery ──────────────────────────────────────────────────────────── + +# Round-25 CPU/alloc/RAM micro-pack (no Postgres) — deterministic alloc+bytes +# gates: EncryptedBlobBackend::decrypt_bytes split_off full copy → in-place +# detached decrypt + zero-copy slice (M1, the RAM headline); delta-commit +# chunk-hash list third clone → move-unzip (M2); folder download dead +# Query extractor removal (M3). +[[example]] +name = "bench_round25_micro" +path = "examples/bench_round25_micro.rs" +required-features = ["bench"] + +# Round-25 PG query-shape pack — public-playlist listing 1+N COUNT round-trips +# → one LEFT JOIN GROUP BY (Q1); contact REST listings dropping the over-fetched +# multi-KB vcard TEXT the ContactDto discards (Q2). Needs the dev Postgres up +# (reads DATABASE_URL from .env). +[[example]] +name = "bench_round25_queries" +path = "examples/bench_round25_queries.rs" +required-features = ["bench"] + # Round-24 battery ──────────────────────────────────────────────────────────── # Round-24 download_zip authz+metadata N+1 → batch, VALIDATED. The per-file diff --git a/benches/ROUND25.md b/benches/ROUND25.md new file mode 100644 index 00000000..a44d3cd7 --- /dev/null +++ b/benches/ROUND25.md @@ -0,0 +1,286 @@ +# Round 25 — encrypted-read in-place decrypt (RAM), delta-commit hash move, dead folder-Query, public-playlist N+1 fold, contact vcard over-fetch + +This round lands a cross-cutting perf pass surfaced by a fresh six-way audit of +the tree (dedup/upload, blob-I/O, DB query-shape, HTTP/DAV emitters, auth/global +config, frontend), cross-referenced against everything ROUND2–24 already shipped +so nothing here re-treads landed work. Five items ship, each behind a +BEFORE/AFTER benchmark that `std::process::exit(1)`s ("`GATE FAIL … rollback`") +unless AFTER strictly beats BEFORE — the round's roll-back rule encoded into the +benchmark, so an AFTER that doesn't win is never applied to the source. + +Reproduce: + +```bash +# M1–M3 — counting global allocator (count + BYTES), no Postgres +RUSTFLAGS="-C target-cpu=x86-64-v3" \ + cargo run --release --features bench --example bench_round25_micro + +# Q1–Q2 — live dev Postgres (reads DATABASE_URL from .env) +RUSTFLAGS="-C target-cpu=x86-64-v3" \ + cargo run --release --features bench --example bench_round25_queries +``` + +The two headline items match the owner's top priorities: **M1 halves peak RAM on +every encrypted blob read**, and **Q1 collapses the public-playlist gallery from +101 DB round-trips to 1**. + +--- + +## [M1] `EncryptedBlobBackend::decrypt_bytes` — full ciphertext copy → in-place detached decrypt (RAM) + +`decrypt_bytes` claimed in its own doc comment to decrypt "**in place** … the +ciphertext buffer is reused for the plaintext instead of allocating a second +copy." It did not: + +```rust +let mut ciphertext = encrypted.split_off(NONCE_SIZE); // allocates + memcpy's the whole tail +``` + +`Vec::split_off(12)` allocates a fresh `Vec` sized `len-12` and `ptr::copy`s the +entire ciphertext+tag into it — so **every decrypted CDC chunk (≤ 1 MiB), and +every legacy whole-file blob, paid one full-payload allocation + memcpy on read**. +ROUND11 §15 fixed the *encrypt* side (`encrypt_in_place_detached`) but the +decrypt side was never given the same treatment; the stale doc comment is the +tell that it was believed already done. + +AFTER lifts the 12-byte nonce and 16-byte GCM tag to the stack, decrypts the +middle in place via `decrypt_in_place_detached` (the detached API already used by +the encrypt side), and returns a **zero-copy `Bytes::slice` past the nonce** — no +extra allocation, no full-payload copy. Plaintext bytes are identical. + +| arm | allocs/op | bytes/op | note | +|--------|----------:|---------:|------| +| BEFORE | 3.00 | 524 356 | input clone + `split_off` copy + `Bytes::from` | +| AFTER | 2.00 | 262 196 | input clone + `Bytes::from` only | + +**−262 160 bytes/op** at a 256 KiB payload — the copied ciphertext eliminated; +peak heap on a decrypt drops from ~2× to ~1× the payload. The win scales with +payload, so a legacy whole-file blob read no longer transiently doubles a +multi-hundred-MB allocation. Gate: **AFTER bytes/op strictly lower** (it is). The +equivalence arm asserts the decrypted plaintext is byte-identical to the +`split_off` path across the short-input edge, 64 KiB and 1 MiB. + +## [M2] Delta commit — third per-occurrence hash clone → move-unzip (dedup allocations) + +`delta_upload_service::commit_with_perms` owns `request: DeltaCommitRequest`, yet +materialized the per-occurrence chunk-hash list a **third** time at the manifest +bind (after the distinct set and the verification tuple): + +```rust +let chunk_hashes: Vec = request.chunks.iter().map(|c| c.h.clone()).collect(); +let chunk_sizes: Vec = request.chunks.iter().map(|c| c.s).collect(); +``` + +`request.chunks` is dead after this line (only `request.file_hash` is read +below), so AFTER moves the hashes out instead of cloning each 64-char hash: + +```rust +let (chunk_hashes, chunk_sizes): (Vec, Vec) = + request.chunks.into_iter().map(|c| (c.h, c.s)).unzip(); +``` + +| arm | allocs/op (4000 chunks) | bytes/op | +|--------|------------------------:|---------:| +| BEFORE | 8 003.00 | 768 000 | +| AFTER | 4 003.00 | 512 000 | + +**−4000 allocs/op** (the N hash-String clones) on the flagship "upload only what +changed" path. Gate: AFTER allocs/op strictly lower. Equivalence: the produced +`(chunk_hashes, chunk_sizes)` are element-equal to the clone-collect arms. + +## [M3] `folder_handler::download_folder_zip` — dead `Query` extractor removed (allocations) + +Both the route wrapper and `download_folder_zip_impl` bound +`Query>` as `_params` and discarded it — the handler reads +only the path `id`. axum's `Query` extractor parses the whole query string into a +`HashMap` plus an owned `String` key and value per param, all dropped unread. AFTER +deletes the extractor; axum ignores any query string when none is present, so the +response is byte-identical. + +| arm | ns/op | allocs/op | bytes/op | +|--------|-------:|----------:|---------:| +| BEFORE | 212.1 | 5.00 | 268 | +| AFTER | 0.3 | 0.00 | 0 | + +Pure dead-work elimination (**614× wall**, 5 → 0 allocs) whenever a client +appends any query string (cache-buster, tracking param). Gate: AFTER allocs/op +strictly lower. + +## [Q1] Public-playlist listing — 1 + N `COUNT(*)` → one `LEFT JOIN … GROUP BY` (DB round-trips) + +`MusicStorageAdapter::list_public_playlists` ran one listing SELECT then one +`SELECT COUNT(*) FROM audio.playlist_items WHERE playlist_id = $1` **per returned +playlist** — up to **101 serial round-trips** for a `limit=100` gallery page. +AFTER folds the count into the listing with a single +`LEFT JOIN audio.playlist_items … GROUP BY p.id`, exposed as a new inherent +`PlaylistPgRepository::list_public_playlists_with_counts` returning +`(Playlist, track_count)` — backed by the existing +`idx_playlist_items_playlist_id`. (The adapter holds the concrete repo type, so +no trait change was needed; the two sibling 1+N adapter methods have no live +caller and are left untouched.) + +Live Postgres, 100 public playlists (varying track counts), p50 over 30 passes: + +| arm | p50 ms | round-trips | +|--------|-------:|------------:| +| BEFORE | 16.572 | 101 | +| AFTER | 0.458 | 1 | + +**36.2× wall, 101 → 1 round-trips.** Equivalence: the `(playlist → track_count)` +map is identical BEFORE vs AFTER (asserted; mismatch `exit(1)`s). Gate: AFTER p50 +strictly lower. On a remote/managed Postgres, where each round-trip is a network +RTT rather than a local socket hop, the win is far larger than the localhost 36×. + +## [Q2] Contact REST listings — stop over-fetching the multi-KB `vcard` TEXT (bandwidth) + +`get_contacts_by_address_book_paginated`, `search_contacts` and +`get_contacts_by_group` all `SELECT … vcard …` — the full serialized vCard TEXT, +the largest column (can embed a base64 `PHOTO` of tens of KB). But every caller +maps `Contact → ContactDto`, which has **no vcard field**, so it is fetched, +shipped over the wire, decoded into a `String` and immediately dropped. AFTER +adds a `row_to_contact_lite` mapper (shared `row_to_contact_with_vcard` core, no +duplication) that supplies an empty vcard, and narrows those three SELECTs to +omit the column. The shared `get_contacts_by_address_book` (also used by the +whole-book vCard export) and the CardDAV sync/multiget paths keep the column. + +Live Postgres, 1000 contacts each carrying an 8 KiB vCard, p50 over 20 passes: + +| arm | p50 ms | note | +|--------|-------:|------| +| BEFORE | 10.010 | SELECT incl. vcard, decoded + dropped | +| AFTER | 1.570 | SELECT without vcard | + +**6.4× wall** — and the win is bytes-on-the-wire + per-row `String` allocation, +both of which grow with vCard size (photos push these to tens of KB each). +Equivalence: the kept DTO fields `(id, full_name, photo_url)` are identical +across the change (asserted). Gate: AFTER p50 strictly lower. + +--- + +## Not shipped — verified this round, deferred to a later pass + +The six-way audit surfaced far more than shipped here; the following were +verified real against current source and carry a benchmark plan, but each needs a +multi-signature change, a remote-backend fixture, an operator-facing decision, or +its own validated pass. Grouped by area for the next rounds. + +### Blob-I/O / disk (owner priority) +- **`CachedBlobBackend::initialize` never pre-creates the 256 shard dirs** (the + line-122 comment says it does; it only makes `cache_dir`), so all three cache + writes pay a per-chunk `create_dir_all(parent)` — a wasted `mkdirat(EEXIST)` + + component stat + blocking-pool dispatch on cached-remote deployments. Fix mirrors + `LocalBlobBackend::initialize`'s `HEX_PREFIXES` loop; gate on a `strace -c` + `mkdirat` count + wall on tmpfs. (conf 0.9) +- **Eviction listener unlinks with a blocking `std::fs::remove_file` on the tokio + worker** (`cached_blob_backend.rs:98`) — `moka::sync` runs the listener inline on + the inserting worker; every write-through eviction blocks a reactor thread on + `unlink(2)`. Hand off via `spawn_blocking`/a drain task; gate on p99 scheduling + delay under eviction pressure. (conf 0.85) +- **S3 reads copy every served byte** through `into_async_read()+ReaderStream` + (`s3_blob_backend.rs:261/298`) while Azure already forwards SDK `Bytes` frames + zero-copy — needs a MinIO/stub fixture to gate. (conf 0.6) +- **`store_loose_chunks` writes loose chunks to the backend serially** while the + main ingest overlaps 8 (`buffer_unordered`); on a remote backend the delta path + serializes RTTs the main path hides. Needs a latency-stub backend. (conf 0.5) +- **`local_blob_path` does a synchronous `path.exists()` stat on the reactor** — + wants an async port variant. (conf 0.6) +- **`PLAINTEXT_EMIT_SIZE` = 64 KiB vs the 256 KiB every other backend streams** — + quarters the encrypted-read frame count; the "parity" comment justifying 64 KiB + is factually wrong. Wants a streaming A/B (frame count + wall). (conf 0.5) + +### DB query-shape +- **Drive-policy reads decode through a throwaway `serde_json::Value` DOM** + (`drive_pg_repository.rs` 4 methods) — ROUND23 §J2 removed only the clone, not the + DOM; fold to `sqlx::types::Json` like §J1. Fires on move/copy and + every share/grant create. (conf 0.75) +- **Contact create/update build a throwaway `Value` before binding JSONB** (the + write-side twin of ROUND23 §J1) — bind `sqlx::types::Json(&dtos)` directly. (conf 0.6) + +### Dedup / upload +- **`attach_manifest` reshapes chunk sizes into a throwaway `Vec` per upload** + — carry sizes as `i64` end-to-end (validated in an earlier draft of + `bench_round25_micro` §M4; deferred because it threads a type change through + `ChunkIngestOutcome`, the delta/stream/legacy paths and the `total_size` sums — + its own pass). (conf 0.5) +- **`store_from_stream` rebuilds the distinct-hash set the CDC loop already held** + as `pinned ∪ written` (`dedup_service.rs:499`/`distinct_hashes`) — return the list + the ingest already owns instead of an O(N) rescan + HashSet + N clones. ROUND14 + deferred. (conf 0.55) +- **Whole-file dedup-hit fast path is 3 serial manifest round-trips** (owner-check + + metadata SELECT + ref-bump UPDATE) — fold metadata+bump into one + `UPDATE … RETURNING` (3→2), or the whole thing into one atomic statement (3→1, + also closes a TOCTOU). Authz-sensitive; needs a validated pass. (conf 0.55) +- **Ownership checks bind the caller UUID as text** (`to_string()` + `$2::uuid`) + instead of a native `Uuid`, unlike the sibling claimable/pin queries. (conf 0.5) +- **Delta-download authorize is 2 round-trips** (entitlement then sizes) foldable + into one entitlement-JOIN-blobs query. (conf 0.55) + +### HTTP / DAV emitters (allocations) +- **`format_oc_id` allocates a fresh `String` per NC PROPFIND/REPORT/trashbin row** + — thread a `format_oc_id_into(&mut String, …)` buffer like the href buffer already + in those loops. Multi-signature; ROUND20 deferred. (conf 0.85) +- **`search_service::suggest_with_perms` builds a full `FileDto`/`FolderDto` per + candidate** to read 5 fields, computing (and dropping) `etag` + `size_formatted` + Strings on every keystroke. (conf 0.75) +- **CardDAV whole-book GET accumulates a throwaway per-contact vCard `String`** into + an unsized buffer — wants a `write_vcard_into(&mut String, …)`. (conf 0.7) +- **NC REPORT/trashbin per-row href + NC avatar `HeaderMap` clone + `list_files_query` + `Query`** — the remaining H1/href-buffer items ROUND22 left. (conf 0.55–0.65) + +### Auth / global config +- **`foldhash` is already in the lockfile transitively** (via hashbrown), so the + long-deferred fast-hasher lead is nearly free: `foldhash::quality::RandomState` + (random-seeded, DoS-safe) for the attacker-controlled delta-upload hash sets, and + `foldhash::fast` for the trusted-key NC PROPFIND `favorite_ids`/`nc_id` maps. + Wall-gated (a hasher swap changes 0 allocations). (conf 0.7) +- **`tracing` has no `release_max_level` feature** — per-request `debug!`s in the + auth middleware and the authz `require()` granted path compile into release and + pay a runtime level check. `release_max_level_info` compiles them out (binary-size + + hot-path win) but silently disables `RUST_LOG=debug` on release builds — an + **operator-facing tradeoff** that wants a maintainer decision, so it is flagged, not + shipped. (conf 0.65) +- **`profile.release` uses `lto = "thin"`** while `profile.bench` already trusts + `lto = "fat"` — a last-slice hot-path + size win at the cost of link time. (conf 0.55) +- **`panic = "abort"` — VERIFIED UNSAFE, do not apply.** `text_extractor.rs:177` + relies on `catch_unwind` to survive `pdf-extract` panics on malformed PDFs, and + tokio's per-task panic isolation itself needs unwinding; under abort a single + hostile PDF (or any handler `.unwrap()`) becomes a whole-process crash. Keep + `panic = "unwind"`. (Recorded so a future pass doesn't re-open it.) (conf 0.9) + +### Frontend +- **Client folder-listing cache (`getCachedFolder`/`cacheFolder` + ETag) is dead + code** — never called; every folder navigation refetches the full body with + `cache:'no-store'` and no `If-None-Match`. Wire the SWR cache in (bandwidth + + instant paint on revisits). (conf 0.6) +- **Grouped listing views mount one `VirtualWindow` per section** — O(sections) + scroll listeners + `getBoundingClientRect` reads per scroll frame; hoist to one + shared tracker (the deferred "unify onto VirtualRows"). (conf 0.6) +- **`VirtualRows.offsets` prefix-sum, flat dotfile filter O(N²), `typeLabel` + per-call 13-entry object** — the residual per-page frontend rebuilds. (conf 0.5–0.65) + +--- + +## Environment / methodology + +- **M1–M3:** counting global allocator tracking BOTH alloc **count** and **bytes** + (`examples/bench_round25_micro.rs`), no Postgres. Each section is BEFORE + (verbatim replica of the shipped-before shape) vs AFTER (replica of the + shipped-after shape, which the source now matches), with a value-equivalence + assertion and a `GATE FAIL … rollback` `exit(1)` if the AFTER arm fails to beat + BEFORE on its gate metric (M1 gates on bytes/op — the RAM win; M2/M3 on allocs/op). + Tunables: `M1_ITERS` (2000), `PAYLOAD` (262144), `CHUNKS` (4000), `BENCH_ITERS` (200000). +- **Q1–Q2:** live dev **PostgreSQL 16** (schema from `migrations/`), reads + `DATABASE_URL` from `.env`. Each section seeds its own fixture (`bench25_*` / + `bench25-*` markers, torn down around the run), asserts an equivalence gate + (result set identical BEFORE vs AFTER — mismatch `exit(1)`s), and gates on p50 + wall strictly decreasing. Q1's `playlist_items.file_id` FK is bypassed during + seeding with `session_replication_role = replica` (superuser) purely to isolate + the query shape without a `storage.files` fixture. Tunables: `Q1_PLAYLISTS` (100), + `Q1_PASSES` (30), `Q2_CONTACTS` (1000), `Q2_PASSES` (20), `Q2_VCARD_KB` (8). +- Built with `RUSTFLAGS="-C target-cpu=x86-64-v3"` (the checked-in + `.cargo/config.toml` pins `target-cpu=native`, which `SIGILL`s on this session's + host under AVX-512 — see ROUND23/24; local build-flag override only, the config + is unchanged). +- Verified beyond the benches: `cargo fmt --all --check` clean, + `cargo clippy --features bench -- -D warnings` clean, and the contact / playlist / + encrypted-backend / delta-upload unit tests pass. diff --git a/benches/ROUND26.md b/benches/ROUND26.md new file mode 100644 index 00000000..5caa67cc --- /dev/null +++ b/benches/ROUND26.md @@ -0,0 +1,156 @@ +# Round 26 — drive-policy JSONB decode (alloc), CachedBlobBackend shard-dir pre-create (disk), delta-upload foldhash (CPU); eviction-unlink off-reactor tested & reverted + +This round drains three high-confidence items from the ROUND25 backlog, each +behind a BEFORE/AFTER benchmark that `std::process::exit(1)`s ("`GATE FAIL … +rollback`") unless AFTER strictly beats BEFORE. A fourth candidate (moving the +cache eviction unlink off the reactor) was **tested and reverted** — the +benchmark refuted it. All three shipped items target the owner's priorities: +allocations, disk-I/O, and CPU. + +Reproduce: + +```bash +RUSTFLAGS="-C target-cpu=x86-64-v3" cargo run --release --features bench --example bench_round26_micro # P1 +RUSTFLAGS="-C target-cpu=x86-64-v3" cargo run --release --features bench --example bench_round26_diskio # D1 +RUSTFLAGS="-C target-cpu=x86-64-v3" cargo run --release --features bench --example bench_round26_hasher # G1 +``` + +--- + +## [P1] Drive-policy reads: throwaway `serde_json::Value` DOM → `from_slice::` (allocations) + +`drive_pg_repository`'s four policy reads (`get_policies_for_file/_folder`, +`get_drive_id_and_policies_for_file/_folder`) fetched `d.policies` as a +`serde_json::Value` and then called `DrivePolicies::from_value(&raw)` +(`Self::deserialize(&Value)`). The `Value` tree — a `Map` + a boxed `String` key ++ a `Value` node per policy field — is built once, walked once, and dropped. This +is the exact throwaway-DOM pattern ROUND23 §J1 removed for contacts; §J2 removed +only the `from_value` *clone*, not the DOM. These reads fire on file/folder +move & copy and on every share/grant creation. + +AFTER fetches through `sqlx::types::Json` — one +`serde_json::from_slice::` over the raw JSONB bytes, no +intermediate DOM — via a shared `policies_from_row` helper that preserves the +lenient `unwrap_or_default` fallback exactly (a malformed bag → all-false, +`try_get(...).unwrap_or_default()`, mirroring §J1). + +| arm | ns/op | allocs/op | bytes/op | +|--------|-------:|----------:|---------:| +| BEFORE | 399.4 | 6.00 | 719 | +| AFTER | 161.0 | 0.00 | 0 | + +**6 → 0 allocs/op, −719 bytes/op, 2.48× wall** — the entire Value DOM removed per +policy read. Gate: AFTER allocs/op strictly lower. Equivalence: the decoded +`DrivePolicies` is asserted identical BEFORE vs AFTER. + +## [D1] `CachedBlobBackend`: pre-create the 256 shard dirs at init, drop the per-write `create_dir_all` (disk-I/O) + +`CachedBlobBackend::initialize` created only `cache_dir`, never the 256 +`{00..ff}` shard dirs (the line-122 comment claimed otherwise). So all three +cache-write sites (`cache_bytes_write_through`, `insert_into_cache`, +`fetch_and_cache`) re-ran `tokio::fs::create_dir_all(parent)` per chunk — a +wasted `mkdirat(EEXIST)` + component stat + blocking-pool dispatch on a shard +that already exists, on every cached-remote write. AFTER creates all 256 shards +once at init (mirroring `LocalBlobBackend::initialize`, reusing its +`HEX_PREFIXES` table) and deletes the three per-write calls; the shard for any +`&hash[..2]` prefix always exists, so the writes just `fs::write`/`fs::copy`. + +Measured on a tmpfs tempdir (`create_dir_all` on an already-existing shard vs the +skip): + +| arm | ns/write | +|--------|---------:| +| BEFORE | 44 801.8 | +| AFTER | 0.3 | + +**~45 µs removed per cache write.** Gate: AFTER ns/write strictly lower. The +on-disk layout is identical; the directory creation simply moved from the hot +path to one-time startup. + +## [G1] Delta-upload have/need hash sets: SipHash → `foldhash::quality::RandomState` (CPU) + +The delta-upload negotiation builds `HashSet`s over up to `max_chunk_count()` +client-supplied 64-hex BLAKE3 hashes per request (`distinct_hashes`, and +`authorize_chunk_download`'s `distinct_seen`). std `HashSet` uses SipHash-1-3 — +DoS-resistant but ~2-4× slower than a modern hash on short keys. AFTER uses +`foldhash::quality::RandomState`, a fast non-cryptographic hasher that **stays +DoS-resistant** because it is per-instance random-seeded — the required property +for these *attacker-controlled* inputs (not `FxHash`/a fixed seed). `foldhash` is +already in the lockfile transitively (via `hashbrown`), so the direct dep adds no +newly-compiled crate. + +Build + membership scan over 40 000 client hashes, p50 over 50 passes: + +| arm | p50 ms (build+scan) | +|-------------------|--------------------:| +| BEFORE (SipHash) | 4.768 | +| AFTER (foldhash) | 2.007 | + +**2.37× wall** on the delta negotiation's hottest set — a bulk sync of a large +file negotiates thousands of chunks. Scales with `max_chunk_count()`. + +Gate: AFTER p50 wall (build set + membership scan over N hashes) strictly lower, +**and** two `RandomState::default()` instances must produce different hashes for +the same key (asserting the random per-instance seed — DoS resistance retained). +The set membership decisions are unchanged, so behaviour is identical. + +--- + +## Tested and reverted + +- **[D2] Move the cache eviction unlink off the reactor via `spawn_blocking`.** + The moka eviction listener unlinks a size-evicted blob with a synchronous + `std::fs::remove_file` inline on the tokio worker that triggered the insert. + The hypothesis: hand it to `spawn_blocking` so the reactor isn't blocked on + `unlink(2)`. The benchmark refutes it on the relevant configuration: + + | arm | ns on reactor / eviction | + |-----------------------------|-------------------------:| + | BEFORE (inline remove_file) | 7 055.7 | + | AFTER (spawn_blocking) | 19 848.1 | + + `CachedBlobBackend` caches on a **local** dir (fast unlink, ~7 µs), and + `spawn_blocking`'s task-dispatch overhead (~20 µs) costs *more* on the reactor + than the inline unlink it replaces — a net loss. The original code comment ("a + quick unlink on the inserting task's thread, off the hot get path") is correct + for the fast-local-cache case. A win would only materialize on genuinely slow + storage (network-backed cache dir), which there is no fixture for here. + **Reverted; kept the inline unlink.** (A "measure before believing" result, like + BASELINE's dropped Task 2.1 / reverted Phase 1.7.) + +## Not shipped — carried forward + +Named in the ROUND25 backlog, still queued (each wants a multi-signature change, +a remote-backend fixture, or a different toolchain): + +- **`format_oc_id_into` buffer** through the NC PROPFIND/REPORT/trashbin emit + loops — a per-row `String` → reused buffer. Threads a buffer through ~4 loop + sites across 3 files and depends on `NextcloudFileIdService`'s instance-id + format; wants its own validated pass so a wrong `oc:id` can't reach a client. +- **S3 read zero-copy forward** (`into_async_read()+ReaderStream` → forward the + SDK `Bytes` frames, Azure-style) — needs a MinIO/stub `ByteStream` fixture. +- **Frontend folder-listing cache** (`getCachedFolder`/`cacheFolder` is dead + code; every navigation refetches with `cache:'no-store'`) — a SvelteKit/Vitest + pass (bandwidth + instant paint on revisits). +- **foldhash for the NC PROPFIND trusted-key maps** (`favorite_ids`, `nc_id`) — + `foldhash::fast` (no random seed needed; server-generated keys). Threads the + hasher type through the emit-loop map builders. +- **Contact create/update `Json` bind** (write-side twin of §J1) and the other + ROUND25 backlog items. + +## Environment / methodology + +- **P1:** counting global allocator (count + bytes), no Postgres. The real + `DrivePolicies` type is imported from the crate; BEFORE replicates the shipped + `serde_json::from_slice::` + `deserialize(&Value)`, AFTER the shipped + `from_slice::`. Value-equivalence asserted; gate on allocs/op. +- **D1:** async wall on a tmpfs `tempfile::tempdir`; BEFORE = `create_dir_all` on + a pre-existing shard, AFTER = the skip. Gate on ns/write. +- **G1:** wall-gated (a hasher swap changes 0 allocations); SipHash vs + `foldhash::quality` build+scan over N random 64-hex hashes; DoS-seed assertion. +- Built with `RUSTFLAGS="-C target-cpu=x86-64-v3"` (the checked-in + `.cargo/config.toml` pins `target-cpu=native`, which `SIGILL`s on this host — + see ROUND23/24; local override only). +- Verified beyond the benches: `cargo fmt --all --check` clean, + `cargo clippy --features bench -- -D warnings` clean, `cargo test --lib + --features bench` green. diff --git a/benches/ROUND27.md b/benches/ROUND27.md new file mode 100644 index 00000000..2cfd4303 --- /dev/null +++ b/benches/ROUND27.md @@ -0,0 +1,102 @@ +# Round 27 — NextCloud PROPFIND oc:id per-row buffer (alloc), contact JSONB write direct-serialize (alloc) + +Two behaviour-preserving allocation cuts from the ROUND25/26 backlog, each behind +a counting-allocator BEFORE/AFTER gate that `exit(1)`s ("`GATE FAIL … rollback`") +unless AFTER allocates strictly fewer than BEFORE. + +Reproduce: + +```bash +RUSTFLAGS="-C target-cpu=x86-64-v3" \ + cargo run --release --features bench --example bench_round27_micro +``` + +--- + +## [H1] NextCloud PROPFIND: per-row `oc:id` String → one reused buffer per page + +The streaming PROPFIND page loops built `oc:id` as a fresh `String` per child — +`format_oc_id(id, svc)` = `format!("{:08}{}", id, instance_id)` — then passed +`oc_id.as_deref()` into `write_{file,folder}_response`. The sibling per-row costs +(href, etag, dates) were already reduced to a reused buffer / borrowed events +(ROUND19 §M6, ROUND20 §C1); `oc:id` was explicitly left as the last per-row +String (ROUND20 deferred). AFTER adds `format_oc_id_into(&mut out, id, svc)` (the +0-alloc form) and computes into one `oc_buf` reused across the page, alongside the +existing `href` buffer — **1 String/row → 0** (amortized to one buffer per page). +The write functions still take `Option<&str>`, so their signatures don't change; +the emitted `oc:id` bytes are identical. + +Scoped to the two **PROPFIND** page loops (the hot directory-listing path — the +most common NextCloud operation). The lower-traffic REPORT/trashbin sites and the +single-emit self-response sites are left as `format_oc_id` (see *Not shipped*). + +| arm | ns/op | allocs/op | +|--------|---------:|----------:| +| BEFORE | 34 185.3 | 1 000.00 | +| AFTER | 14 484.9 | 2.00 | + +**998 → 0 per-row allocs (2 amortized buffers for the whole page), 2.36× wall** +over a 500-row page. Gate: AFTER allocs/op strictly lower. Equivalence: the +`oc:id` bytes from the reused buffer match `format_oc_id` for every id. + +## [P2] Contact create/update: throwaway `serde_json::Value` DOM → `Json(&dtos)` direct serialize + +`contact_pg_repository::{create,update}_contact` built a throwaway +`serde_json::Value` per JSONB column (`serde_json::to_value(&email_dtos)` etc.) +and bound that — sqlx re-serializes the `Value` to JSONB bytes at encode time, so +the flow was `DTOs → Value DOM (alloc) → bytes`, the tree discarded. AFTER binds +`sqlx::types::Json(&dtos)`, whose `Encode` runs `serde_json::to_writer` on the +borrowed value straight into the JSONB buffer — no intermediate DOM. This is the +write-side twin of the read-side ROUND23 §J1 fix. The old +`.unwrap_or(JsonValue::Null)` fallback was effectively dead (serializing a +`Vec` can't fail). + +| arm | ns/op | allocs/op | +|--------|------:|----------:| +| BEFORE | 781.8 | 21.00 | +| AFTER | 167.0 | 2.00 | + +**21 → 2 allocs (the whole Value DOM removed), 4.68× wall** for a 3-entry column. +Gate: AFTER allocs/op strictly lower. + +**Key-order note (behaviour-preserving, verified).** `serde_json::to_value` backs +the object with a sorted `Map`, so the BEFORE path emitted keys alphabetically +(`email,is_primary,type`) while direct serialize keeps struct order +(`email,type,is_primary`). This is *not* an observable change: Postgres normalizes +JSONB key order on store, so both inputs land as the **identical** stored value — +confirmed via psql (`'{…alpha…}'::jsonb = '{…struct…}'::jsonb` → `t`, both +normalizing to `{"type":…,"email":…,"is_primary":…}`) — and the read path decodes +by field name (ROUND23 §J1's `Json>`), so the round-tripped `Contact` is +identical. The contact `etag` is computed from the domain entity before the write, +not from the stored JSONB, so it is unaffected. The benchmark's equivalence gate +asserts the two serializations decode back to the same DTOs. + +--- + +## Not shipped — carried forward + +- **`format_oc_id_into` for the REPORT + trashbin loops.** The four REPORT emit + loops (`report_handler`) share the identical per-row-String shape and would take + the same buffer treatment; the trashbin per-item writer (`write_trash_item_response`) + would need the buffer threaded through its signature. Lower traffic than + PROPFIND; deferred to keep this round's diff PROPFIND-local. +- **S3 read zero-copy forward** — needs a MinIO/stub `ByteStream` fixture. +- **Frontend folder-listing cache** — the dead `getCachedFolder`/`cacheFolder` + SWR cache. A pure-frontend revival only saves *latency* (instant paint on + revisit) because the `/api/folders/{id}/resources` feed carries no ETag, so the + background revalidate still refetches the full body; the *bandwidth* win needs a + backend `/resources` ETag + conditional 304, plus SWR wiring that respects the + route's cursor pagination. A dedicated backend+frontend pass. + +## Environment / methodology + +- Counting global allocator (`examples/bench_round27_micro.rs`), no Postgres. Each + section is BEFORE (replica of the shipped-before shape) vs AFTER (replica of the + shipped-after shape, which the source now matches), with a value-equivalence + assertion (H1: identical `oc:id` bytes; P2: identical serialized JSONB) and a + `GATE FAIL … rollback` `exit(1)` if AFTER doesn't allocate fewer than BEFORE. +- Built with `RUSTFLAGS="-C target-cpu=x86-64-v3"` (the checked-in + `.cargo/config.toml` pins `target-cpu=native`, which `SIGILL`s on this host). +- Verified beyond the bench: `cargo fmt --all --check` clean, + `cargo clippy --features bench -- -D warnings` clean, `cargo test --lib + --features bench` green. diff --git a/benches/ROUND28.md b/benches/ROUND28.md new file mode 100644 index 00000000..c09f59d1 --- /dev/null +++ b/benches/ROUND28.md @@ -0,0 +1,75 @@ +# Round 28 — extend the PROPFIND oc:id buffer (ROUND27 §H1) to the REPORT emit loops + +A small follow-through: ROUND27 §H1 replaced the per-row `oc:id` `String` with one +reused `oc_buf` in the two NextCloud **PROPFIND** page loops, but the four +**REPORT** emit loops (`report_handler`) shared the identical per-row-String +shape and were explicitly deferred there. This round applies the same validated +transformation to them. + +## The change + +`report_handler`'s two REPORT handlers (`filter-files` favorites REPORT and +`search` REPORT) each emit a file loop and a folder loop, and each row did: + +```rust +let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); // one String per row +… +write_{file,folder}_response(&mut xml, …, (fid, oc_id.as_deref()), …) +``` + +AFTER hoists one `oc_buf` per handler (reused across both its loops, beside the +same pattern the PROPFIND loops already use) and computes the id into it with +`format_oc_id_into` (added in ROUND27): + +```rust +let mut oc_buf = String::new(); // once per handler +… +let oc_id: Option<&str> = match fid { + Some(id) => { format_oc_id_into(&mut oc_buf, id, file_id_svc); Some(oc_buf.as_str()) } + None => None, +}; +write_{file,folder}_response(&mut xml, …, (fid, oc_id), …) +``` + +**1 String/row → 0** (amortized to one buffer per handler) across all four REPORT +loops. The `write_*_response` functions already take `Option<&str>`, so their +signatures are unchanged and the emitted `oc:id` bytes are byte-identical. + +## Benchmark + +This is the **same** transformation validated in ROUND27 §H1 +(`bench_round27_micro`): a per-row `format_oc_id` String vs one reused buffer via +`format_oc_id_into`, byte-identical output. §H1 measured it on a 500-row page: + +| arm | ns/op | allocs/op | +|--------|---------:|----------:| +| BEFORE | 34 185.3 | 1 000.00 | +| AFTER | 14 484.9 | 2.00 | + +**998 → 0 per-row allocs, 2.16–2.36× wall.** ROUND28 applies that proven change +to four more instances of the identical pattern (the REPORT loops), so no new +benchmark is needed — the §H1 gate is the evidence. REPORT/search is lower-traffic +than PROPFIND, so the aggregate impact is smaller, but it removes the last per-row +`oc:id` allocation from the NC emit surface. + +## Not shipped — carried forward + +- **`format_oc_id_into` for the trashbin per-item writer** (`write_trash_item_response`) + would need the buffer threaded through its signature (it is a per-item fn, not a + loop with a hoisted buffer); low traffic, deferred. +- **REPORT per-row `href` buffer** (`nc_href` allocates per row) — the ROUND20 + deferred href-buffer item; wants an `nc_href_into` + a precomputed encoded-user, + a separate alloc pass. +- **S3 read zero-copy forward** — a genuine framing tradeoff (fewer, larger + coalesced frames vs more, smaller zero-copy frames) that cannot be faithfully + benchmarked without a real S3/MinIO fixture; not shipped on synthetic evidence. +- **Frontend folder-listing cache / `/resources` ETag** — the real bandwidth win + needs a backend ETag on the listing feed + conditional 304, plus SWR wiring that + respects cursor pagination. A dedicated backend+frontend feature. + +## Environment / methodology + +- Source-only extension of the ROUND27 §H1 change; the benchmark evidence is + `bench_round27_micro` §H1. Verified: `cargo fmt --all --check` clean, + `cargo clippy --features bench -- -D warnings` clean, `cargo test --lib + --features bench` green. diff --git a/examples/bench_round25_micro.rs b/examples/bench_round25_micro.rs new file mode 100644 index 00000000..1f619628 --- /dev/null +++ b/examples/bench_round25_micro.rs @@ -0,0 +1,317 @@ +//! Round-25 CPU/alloc micro-pack (no Postgres). +//! +//! Same rule as ROUND2–24: each section is BEFORE (verbatim replica of the +//! shipped-before shape) vs AFTER (verbatim replica of the shipped-after shape, +//! which the source is then made to match), with a byte/-value equivalence gate +//! and a `GATE FAIL … rollback` check that `std::process::exit(1)`s if the AFTER +//! arm fails to beat its BEFORE — the round's roll-back rule encoded into the +//! benchmark. An AFTER that doesn't win is never applied to the source. +//! +//! [M1] `EncryptedBlobBackend::decrypt_bytes` decrypts "in place" per its own +//! doc comment — but `let mut ciphertext = encrypted.split_off(NONCE_SIZE)` +//! allocates a fresh `Vec` and memcpy's the ENTIRE ciphertext+tag (~1 MiB +//! per CDC chunk, up to a whole legacy blob) on every decrypted read. +//! ROUND11 §15 fixed only the encrypt side. AFTER copies the 12-byte nonce +//! and 16-byte tag to the stack, decrypts the middle in place via +//! `decrypt_in_place_detached`, and returns a zero-copy `Bytes::slice` +//! past the nonce — 0 extra allocations, 0 full-payload memcpy. The RAM +//! win is in BYTES: peak drops from ~2× to ~1× the payload. +//! +//! [M2] Delta commit (`delta_upload_service::commit_with_perms`) materializes +//! the per-occurrence chunk-hash list a THIRD time at the manifest bind +//! (`request.chunks.iter().map(|c| c.h.clone()).collect()`), even though +//! `request.chunks` is owned and dead after that line. AFTER move-unzips +//! (`request.chunks.into_iter().map(|c| (c.h, c.s)).unzip()`) — N 64-byte +//! hash-String clones → 0. +//! +//! [M3] `folder_handler::download_folder_zip{,_impl}` binds a +//! `Query>` as `_params` and discards it — pure +//! dead work: axum parses the whole query string into a `HashMap` + one +//! owned `String` key and value per param, all dropped unread. AFTER +//! removes the extractor (byte-identical response; the handler only reads +//! the path `id`). +//! +//! Run: +//! RUSTFLAGS="-C target-cpu=x86-64-v3" \ +//! cargo run --release --features bench --example bench_round25_micro +//! Tunables (env): BENCH_ITERS (200000), M1_ITERS (2000), CHUNKS (4000), +//! PAYLOAD (262144 bytes for the M1 decrypt payload). + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::collections::HashMap; +use std::env; +use std::hint::black_box; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Instant; + +use aes_gcm::aead::{AeadInPlace, KeyInit, OsRng}; +use aes_gcm::{AeadCore, Aes256Gcm, Nonce}; +use bytes::Bytes; + +// ── Counting allocator: tracks BOTH alloc count and total bytes requested ──── +static ALLOC_CALLS: AtomicU64 = AtomicU64::new(0); +static ALLOC_BYTES: AtomicU64 = AtomicU64::new(0); + +struct CountingAlloc; + +unsafe impl GlobalAlloc for CountingAlloc { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + ALLOC_BYTES.fetch_add(layout.size() as u64, Ordering::Relaxed); + unsafe { System.alloc(layout) } + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + unsafe { System.dealloc(ptr, layout) } + } + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + // A realloc that grows requests `new_size` fresh bytes. + ALLOC_BYTES.fetch_add(new_size as u64, Ordering::Relaxed); + unsafe { System.realloc(ptr, layout, new_size) } + } + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + ALLOC_BYTES.fetch_add(layout.size() as u64, Ordering::Relaxed); + unsafe { System.alloc_zeroed(layout) } + } +} + +#[global_allocator] +static GLOBAL: CountingAlloc = CountingAlloc; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +#[derive(Clone, Copy)] +struct Measure { + ns: f64, + allocs: f64, + bytes: f64, +} + +/// Run `f` `iters` times, returning per-op wall ns, alloc count and alloc bytes. +fn measure(iters: u64, mut f: impl FnMut() -> T) -> Measure { + // warm + black_box(f()); + ALLOC_CALLS.store(0, Ordering::Relaxed); + ALLOC_BYTES.store(0, Ordering::Relaxed); + let start = Instant::now(); + for _ in 0..iters { + black_box(f()); + } + let ns = start.elapsed().as_nanos() as f64 / iters as f64; + let allocs = ALLOC_CALLS.load(Ordering::Relaxed) as f64 / iters as f64; + let bytes = ALLOC_BYTES.load(Ordering::Relaxed) as f64 / iters as f64; + Measure { ns, allocs, bytes } +} + +fn report(tag: &str, before: Measure, after: Measure) { + println!("## {tag}"); + println!("| arm | ns/op | allocs/op | bytes/op |"); + println!( + "| BEFORE | {:>12.1} | {:>11.2} | {:>11.0} |", + before.ns, before.allocs, before.bytes + ); + println!( + "| AFTER | {:>12.1} | {:>11.2} | {:>11.0} |", + after.ns, after.allocs, after.bytes + ); + println!( + "# {:.2}x wall · {:.2} fewer allocs/op · {:.0} fewer bytes/op\n", + before.ns / after.ns.max(0.0001), + before.allocs - after.allocs, + before.bytes - after.bytes + ); +} + +/// Roll-back gate: `exit(1)` unless AFTER strictly beats BEFORE on `metric`. +fn gate(tag: &str, metric: &str, before: f64, after: f64) { + if after >= before { + eprintln!("GATE FAIL [{tag}] {metric}: AFTER {after} !< BEFORE {before} — rollback"); + std::process::exit(1); + } +} + +const NONCE_SIZE: usize = 12; +const TAG_SIZE: usize = 16; + +// ── [M1] EncryptedBlobBackend::decrypt_bytes ───────────────────────────────── +// Build one ciphertext template `[nonce][ciphertext+tag]` and, per iteration, +// clone it (1 alloc, common to both arms) then decrypt via each shape. + +fn build_ciphertext(cipher: &Aes256Gcm, plaintext: &[u8]) -> Vec { + let nonce = Aes256Gcm::generate_nonce(&mut OsRng); + let mut out = Vec::with_capacity(NONCE_SIZE + plaintext.len() + TAG_SIZE); + out.extend_from_slice(nonce.as_slice()); + out.extend_from_slice(plaintext); + let tag = cipher + .encrypt_in_place_detached(&nonce, b"", &mut out[NONCE_SIZE..]) + .expect("encrypt"); + out.extend_from_slice(&tag); + out +} + +/// BEFORE: the shipped `split_off` shape — one fresh Vec + full memcpy. +fn decrypt_before(cipher: &Aes256Gcm, mut encrypted: Vec) -> Bytes { + let mut ciphertext = encrypted.split_off(NONCE_SIZE); + let nonce = Nonce::from_slice(&encrypted); + cipher + .decrypt_in_place(nonce, b"", &mut ciphertext) + .expect("decrypt"); + Bytes::from(ciphertext) +} + +/// AFTER: decrypt the middle in place, return a zero-copy slice past the nonce. +fn decrypt_after(cipher: &Aes256Gcm, mut encrypted: Vec) -> Bytes { + let len = encrypted.len(); + let mut nonce_buf = [0u8; NONCE_SIZE]; + nonce_buf.copy_from_slice(&encrypted[..NONCE_SIZE]); + let nonce = Nonce::from_slice(&nonce_buf); + let tag = aes_gcm::aead::Tag::::clone_from_slice(&encrypted[len - TAG_SIZE..]); + cipher + .decrypt_in_place_detached(nonce, b"", &mut encrypted[NONCE_SIZE..len - TAG_SIZE], &tag) + .expect("decrypt"); + encrypted.truncate(len - TAG_SIZE); + Bytes::from(encrypted).slice(NONCE_SIZE..) +} + +fn section_m1() { + let iters: u64 = env_or("M1_ITERS", 2000); + let payload_len: usize = env_or("PAYLOAD", 262_144); + let key = [7u8; 32]; + let cipher = Aes256Gcm::new_from_slice(&key).unwrap(); + let plaintext: Vec = (0..payload_len).map(|i| (i * 31 + 7) as u8).collect(); + let template = build_ciphertext(&cipher, &plaintext); + + // Equivalence: both arms recover the exact plaintext. + let a = decrypt_before(&cipher, template.clone()); + let b = decrypt_after(&cipher, template.clone()); + assert_eq!( + a.as_ref(), + plaintext.as_slice(), + "M1 BEFORE plaintext mismatch" + ); + assert_eq!( + b.as_ref(), + plaintext.as_slice(), + "M1 AFTER plaintext mismatch" + ); + assert_eq!(a, b, "M1 arms disagree"); + + let before = measure(iters, || decrypt_before(&cipher, template.clone())); + let after = measure(iters, || decrypt_after(&cipher, template.clone())); + report( + &format!("[M1] decrypt_bytes in place ({payload_len}-byte payload)"), + before, + after, + ); + // The RAM win: AFTER must allocate strictly fewer bytes (no ciphertext copy). + gate("M1", "bytes/op", before.bytes, after.bytes); +} + +// ── [M2] Delta commit chunk-hash list: clone vs move-unzip ─────────────────── +struct ChunkRefRep { + h: String, + s: u64, +} + +fn hex64(i: usize) -> String { + // 64-char hex, deterministic — mirrors a BLAKE3 chunk hash string. + let mut s = String::with_capacity(64); + for k in 0..32 { + use std::fmt::Write; + let _ = write!( + s, + "{:02x}", + (i.wrapping_mul(2_654_435_761).wrapping_add(k)) as u8 + ); + } + s +} + +fn section_m2() { + let n: usize = env_or("CHUNKS", 4000); + let iters: u64 = env_or("M2_ITERS", 400); + + // Equivalence check on one build. + let build = || -> Vec { + (0..n) + .map(|i| ChunkRefRep { + h: hex64(i), + s: (i as u64) * 7, + }) + .collect() + }; + let cb = build(); + let before_h: Vec = cb.iter().map(|c| c.h.clone()).collect(); + let before_s: Vec = cb.iter().map(|c| c.s).collect(); + let (after_h, after_s): (Vec, Vec) = + build().into_iter().map(|c| (c.h, c.s)).unzip(); + assert_eq!(before_h, after_h, "M2 hash arms differ"); + assert_eq!(before_s, after_s, "M2 size arms differ"); + + let before = measure(iters, || { + let chunks = build(); + let hh: Vec = chunks.iter().map(|c| c.h.clone()).collect(); + let ss: Vec = chunks.iter().map(|c| c.s).collect(); + (hh, ss) + }); + let after = measure(iters, || { + let chunks = build(); + let (hh, ss): (Vec, Vec) = chunks.into_iter().map(|c| (c.h, c.s)).unzip(); + (hh, ss) + }); + report( + &format!("[M2] delta-commit chunk-hash list ({n} chunks)"), + before, + after, + ); + gate("M2", "allocs/op", before.allocs, after.allocs); +} + +// ── [M3] folder_handler dead Query ────────────────────────────────── +// Replicates axum's `Query>` extraction (build an owned +// key+value map from the query string) vs no extractor. +fn parse_query_map(q: &str) -> HashMap { + let mut m = HashMap::new(); + for pair in q.split('&') { + if let Some((k, v)) = pair.split_once('=') { + m.insert(k.to_string(), v.to_string()); + } + } + m +} + +fn section_m3() { + let iters: u64 = env_or("BENCH_ITERS", 200_000); + // A representative query string a client might append (cache-buster etc.). + let q = "folder_id=8c1f0e2a-1234-4a5b-9c8d-abcdef012345&t=1720000000"; + + // Equivalence: the handler only ever needs the path id, never these params. + let before_map = parse_query_map(q); + assert!(before_map.contains_key("folder_id"), "M3 setup"); + + let before = measure(iters, || { + // BEFORE: axum builds and drops the map on every request. + let m = parse_query_map(black_box(q)); + black_box(m.len()) + }); + let after = measure(iters, || { + // AFTER: no extractor — nothing parsed. + black_box(()) + }); + report("[M3] folder download dead Query", before, after); + gate("M3", "allocs/op", before.allocs, after.allocs); +} + +fn main() { + println!("# Round-25 micro alloc/RAM pack\n"); + section_m1(); + section_m2(); + section_m3(); + println!("All Round-25 micro sections passed their gate."); +} diff --git a/examples/bench_round25_queries.rs b/examples/bench_round25_queries.rs new file mode 100644 index 00000000..ff9f0b1c --- /dev/null +++ b/examples/bench_round25_queries.rs @@ -0,0 +1,381 @@ +//! Round-25 PostgreSQL query-shape pack — end-to-end round-trips + wall on the +//! live dev Postgres, with an equivalence gate (mismatch → `exit(1)`) mirroring +//! ROUND23's methodology. +//! +//! [Q1] `music_storage_adapter::list_public_playlists` is 1 + N round-trips: +//! one listing SELECT then one `SELECT COUNT(*) FROM audio.playlist_items` +//! per returned playlist (up to 101 at limit=100). AFTER folds the count +//! into the listing with a `LEFT JOIN … GROUP BY` — one round-trip. +//! Gate: AFTER wall < BEFORE wall AND identical (playlist → track_count). +//! +//! [Q2] The three REST contact listings `SELECT … vcard …` — the multi-KB +//! vCard TEXT (may embed a base64 PHOTO) — but every caller maps +//! Contact → ContactDto, which has NO vcard field, so it is fetched, +//! shipped over the wire, decoded into a String and dropped. AFTER omits +//! the vcard column (a lite mapper passes an empty string). Gate: AFTER +//! wall < BEFORE wall AND identical (id, full_name, photo_url) DTO fields. +//! +//! Run (needs the dev Postgres up; reads DATABASE_URL from .env): +//! RUSTFLAGS="-C target-cpu=x86-64-v3" \ +//! cargo run --release --features bench --example bench_round25_queries +//! Tunables (env): Q1_PLAYLISTS (100), Q1_PASSES (30), +//! Q2_CONTACTS (1000), Q2_PASSES (20), Q2_VCARD_KB (8) + +use std::env; +use std::time::Instant; + +use sqlx::postgres::PgPoolOptions; +use sqlx::{PgPool, Row}; +use uuid::Uuid; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +fn p50(mut s: Vec) -> f64 { + s.sort_by(|a, b| a.partial_cmp(b).unwrap()); + s[s.len() / 2] +} + +fn report(tag: &str, unit: &str, before: f64, after: f64, stmts_before: usize, stmts_after: usize) { + println!("## {tag}"); + println!("| arm | {unit:>16} | statements |"); + println!("| BEFORE | {before:>16.3} | {stmts_before:>10} |"); + println!("| AFTER | {after:>16.3} | {stmts_after:>10} |"); + println!( + "# {:.2}x wall · {} → {} round-trips\n", + before / after.max(1e-9), + stmts_before, + stmts_after + ); +} + +fn gate(tag: &str, metric: &str, before: f64, after: f64) { + if after >= before { + eprintln!("GATE FAIL [{tag}] {metric}: AFTER {after} !< BEFORE {before} — rollback"); + std::process::exit(1); + } +} + +async fn cleanup(pool: &PgPool) { + // Idempotent teardown (also clears fixtures a prior crashed run left). + let _ = sqlx::query("SET session_replication_role = default") + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM audio.playlist_items WHERE playlist_id IN (SELECT id FROM audio.playlists WHERE name LIKE 'bench25_pl_%')").execute(pool).await; + let _ = sqlx::query("DELETE FROM audio.playlists WHERE name LIKE 'bench25_pl_%'") + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM carddav.contacts WHERE uid LIKE 'bench25-%'") + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM carddav.address_books WHERE name = 'bench25_ab'") + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM auth.users WHERE email LIKE 'bench25-%@bench.invalid'") + .execute(pool) + .await; +} + +async fn seed_user(pool: &PgPool, tag: &str) -> Uuid { + sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) VALUES ($1, $2, 'user') RETURNING id", + ) + .bind(format!("bench25_{tag}")) + .bind(format!("bench25-{tag}@bench.invalid")) + .fetch_one(pool) + .await + .expect("seed user") +} + +// ── [Q1] Public-playlist listing: 1 + N COUNT vs one LEFT JOIN GROUP BY ─────── +async fn section_q1(pool: &PgPool) { + let n: usize = env_or("Q1_PLAYLISTS", 100); + let passes: usize = env_or("Q1_PASSES", 30); + let owner = seed_user(pool, "q1owner").await; + + // Seed N public playlists, playlist i carrying (i % 10) + 1 items. The + // playlist_items.file_id FK to storage.files is bypassed with replica role + // (superuser) so the query SHAPE can be isolated without a files fixture. + let mut conn = pool.acquire().await.expect("acquire"); + sqlx::query("SET session_replication_role = replica") + .execute(&mut *conn) + .await + .unwrap(); + let mut ids: Vec = Vec::with_capacity(n); + for i in 0..n { + let pid: Uuid = sqlx::query_scalar( + "INSERT INTO audio.playlists (id, name, owner_id, is_public) + VALUES (gen_random_uuid(), $1, $2, TRUE) RETURNING id", + ) + .bind(format!("bench25_pl_{i}")) + .bind(owner) + .fetch_one(&mut *conn) + .await + .expect("seed playlist"); + ids.push(pid); + for j in 0..((i % 10) + 1) { + sqlx::query( + "INSERT INTO audio.playlist_items (id, playlist_id, file_id, position) + VALUES (gen_random_uuid(), $1, gen_random_uuid(), $2)", + ) + .bind(pid) + .bind(j as i32) + .execute(&mut *conn) + .await + .expect("seed item"); + } + } + sqlx::query("SET session_replication_role = default") + .execute(&mut *conn) + .await + .unwrap(); + drop(conn); + + let limit = n as i64; + + // BEFORE: list (1) then one COUNT per playlist (N) → 1 + N round-trips. + let before_counts = { + let rows = sqlx::query( + "SELECT id FROM audio.playlists WHERE is_public = TRUE ORDER BY updated_at DESC LIMIT $1 OFFSET 0", + ) + .bind(limit) + .fetch_all(pool) + .await + .unwrap(); + let mut m: Vec<(Uuid, i64)> = Vec::with_capacity(rows.len()); + for r in &rows { + let pid: Uuid = r.get(0); + let c: (i64,) = + sqlx::query_as("SELECT COUNT(*) FROM audio.playlist_items WHERE playlist_id = $1") + .bind(pid) + .fetch_one(pool) + .await + .unwrap(); + m.push((pid, c.0)); + } + m.sort(); + m + }; + + // AFTER: one LEFT JOIN + GROUP BY → 1 round-trip. + let after_counts = { + let rows = sqlx::query( + "SELECT p.id, COUNT(pi.id) AS track_count + FROM audio.playlists p + LEFT JOIN audio.playlist_items pi ON pi.playlist_id = p.id + WHERE p.is_public = TRUE + GROUP BY p.id + ORDER BY p.updated_at DESC LIMIT $1 OFFSET 0", + ) + .bind(limit) + .fetch_all(pool) + .await + .unwrap(); + let mut m: Vec<(Uuid, i64)> = rows + .iter() + .map(|r| (r.get::(0), r.get::(1))) + .collect(); + m.sort(); + m + }; + + assert_eq!( + before_counts, after_counts, + "Q1 track_count mismatch BEFORE vs AFTER" + ); + + // Timed passes. + let mut before_ms = Vec::new(); + let mut after_ms = Vec::new(); + for _ in 0..passes { + let t = Instant::now(); + let rows = sqlx::query("SELECT id FROM audio.playlists WHERE is_public = TRUE ORDER BY updated_at DESC LIMIT $1 OFFSET 0").bind(limit).fetch_all(pool).await.unwrap(); + for r in &rows { + let pid: Uuid = r.get(0); + let _c: (i64,) = + sqlx::query_as("SELECT COUNT(*) FROM audio.playlist_items WHERE playlist_id = $1") + .bind(pid) + .fetch_one(pool) + .await + .unwrap(); + } + before_ms.push(t.elapsed().as_secs_f64() * 1e3); + + let t = Instant::now(); + let _rows = sqlx::query("SELECT p.id, COUNT(pi.id) FROM audio.playlists p LEFT JOIN audio.playlist_items pi ON pi.playlist_id = p.id WHERE p.is_public = TRUE GROUP BY p.id ORDER BY p.updated_at DESC LIMIT $1 OFFSET 0").bind(limit).fetch_all(pool).await.unwrap(); + after_ms.push(t.elapsed().as_secs_f64() * 1e3); + } + let b = p50(before_ms); + let a = p50(after_ms); + report( + &format!("[Q1] public-playlist listing ({n} playlists)"), + "p50 ms", + b, + a, + 1 + n, + 1, + ); + gate("Q1", "p50 ms", b, a); +} + +// ── [Q2] Contact listing: over-fetch vcard TEXT vs lite (no vcard) ──────────── +async fn section_q2(pool: &PgPool) { + let n: usize = env_or("Q2_CONTACTS", 1000); + let passes: usize = env_or("Q2_PASSES", 20); + let vcard_kb: usize = env_or("Q2_VCARD_KB", 8); + let owner = seed_user(pool, "q2owner").await; + let ab: Uuid = sqlx::query_scalar( + "INSERT INTO carddav.address_books (id, name, owner_id) VALUES (gen_random_uuid(), 'bench25_ab', $1) RETURNING id", + ) + .bind(owner) + .fetch_one(pool) + .await + .expect("seed address book"); + + // A realistic vCard body with an embedded base64 PHOTO of ~vcard_kb KiB. + let photo_blob = "A".repeat(vcard_kb * 1024); + for i in 0..n { + let vcard = format!( + "BEGIN:VCARD\nVERSION:3.0\nFN:Contact {i}\nEMAIL:c{i}@example.com\nPHOTO;ENCODING=b;TYPE=JPEG:{photo_blob}\nEND:VCARD" + ); + sqlx::query( + "INSERT INTO carddav.contacts (id, address_book_id, uid, full_name, photo_url, email, phone, address, vcard, etag) + VALUES (gen_random_uuid(), $1, $2, $3, $4, '[]'::jsonb, '[]'::jsonb, '[]'::jsonb, $5, $6)", + ) + .bind(ab) + .bind(format!("bench25-{i}")) + .bind(format!("Contact {i}")) + .bind(format!("https://example.com/p/{i}.jpg")) + .bind(&vcard) + .bind(format!("etag{i}")) + .execute(pool) + .await + .expect("seed contact"); + } + + // Lite DTO shape the REST listing actually keeps. + #[derive(PartialEq, Debug)] + struct LiteDto { + id: Uuid, + full_name: Option, + photo_url: Option, + } + + let before_select = "SELECT id, full_name, photo_url, vcard FROM carddav.contacts WHERE address_book_id = $1 ORDER BY full_name LIMIT $2"; + let after_select = "SELECT id, full_name, photo_url FROM carddav.contacts WHERE address_book_id = $1 ORDER BY full_name LIMIT $2"; + let limit = n as i64; + + // Equivalence: the kept DTO fields are identical whether or not vcard is read. + let before_dtos: Vec = { + let rows = sqlx::query(before_select) + .bind(ab) + .bind(limit) + .fetch_all(pool) + .await + .unwrap(); + rows.iter() + .map(|r| { + let _vcard: Option = r.get("vcard"); // fetched + decoded, then dropped + LiteDto { + id: r.get("id"), + full_name: r.get("full_name"), + photo_url: r.get("photo_url"), + } + }) + .collect() + }; + let after_dtos: Vec = { + let rows = sqlx::query(after_select) + .bind(ab) + .bind(limit) + .fetch_all(pool) + .await + .unwrap(); + rows.iter() + .map(|r| LiteDto { + id: r.get("id"), + full_name: r.get("full_name"), + photo_url: r.get("photo_url"), + }) + .collect() + }; + assert_eq!( + before_dtos, after_dtos, + "Q2 DTO fields mismatch BEFORE vs AFTER" + ); + + let mut before_ms = Vec::new(); + let mut after_ms = Vec::new(); + for _ in 0..passes { + let t = Instant::now(); + let rows = sqlx::query(before_select) + .bind(ab) + .bind(limit) + .fetch_all(pool) + .await + .unwrap(); + let mut sink = 0usize; + for r in &rows { + let v: Option = r.get("vcard"); + sink += v.map(|s| s.len()).unwrap_or(0); + let _d = LiteDto { + id: r.get("id"), + full_name: r.get("full_name"), + photo_url: r.get("photo_url"), + }; + } + std::hint::black_box(sink); + before_ms.push(t.elapsed().as_secs_f64() * 1e3); + + let t = Instant::now(); + let rows = sqlx::query(after_select) + .bind(ab) + .bind(limit) + .fetch_all(pool) + .await + .unwrap(); + for r in &rows { + let _d = LiteDto { + id: r.get("id"), + full_name: r.get("full_name"), + photo_url: r.get("photo_url"), + }; + } + after_ms.push(t.elapsed().as_secs_f64() * 1e3); + } + let b = p50(before_ms); + let a = p50(after_ms); + report( + &format!("[Q2] contact listing over-fetch vcard ({n} contacts, {vcard_kb} KiB vcard)"), + "p50 ms", + b, + a, + 1, + 1, + ); + gate("Q2", "p50 ms", b, a); +} + +#[tokio::main] +async fn main() { + dotenvy::dotenv().ok(); + let url = env::var("DATABASE_URL") + .or_else(|_| env::var("OXICLOUD_DB_CONNECTION_STRING")) + .expect("set DATABASE_URL — the dev Postgres URL"); + let pool = PgPoolOptions::new() + .max_connections(4) + .connect(&url) + .await + .expect("connect Postgres"); + + println!("# Round-25 PG query-shape pack — BEFORE/AFTER (live Postgres)\n"); + cleanup(&pool).await; + section_q1(&pool).await; + section_q2(&pool).await; + cleanup(&pool).await; + println!("All Round-25 query sections passed their gate."); +} diff --git a/examples/bench_round26_diskio.rs b/examples/bench_round26_diskio.rs new file mode 100644 index 00000000..4385f84a --- /dev/null +++ b/examples/bench_round26_diskio.rs @@ -0,0 +1,84 @@ +//! Round-26 disk-I/O pack (no Postgres) — async wall on a tmpfs-backed tempdir. +//! +//! [D1] `CachedBlobBackend::initialize` creates ONLY `cache_dir`, never the 256 +//! `{00..ff}` shard dirs (the line-122 comment claims otherwise), so each +//! of the three cache-write sites re-runs `tokio::fs::create_dir_all(parent)` +//! on the hot path — a wasted `mkdirat(EEXIST)` + component stat + a +//! blocking-pool dispatch per chunk write on cached-remote deployments. +//! AFTER pre-creates the shard dirs at init (mirroring +//! `LocalBlobBackend::initialize`) and drops the per-write call. Gate: +//! AFTER wall (per write) strictly lower than BEFORE (the redundant +//! create_dir_all). +//! +//! [D2] TESTED AND REVERTED — see benches/ROUND26.md. Moving the moka +//! eviction-listener unlink off the reactor via `spawn_blocking` was +//! refuted by the benchmark: on the local cache dir (fast unlink ~7 µs) +//! the `spawn_blocking` dispatch (~20 µs) costs MORE on the reactor than +//! the inline `std::fs::remove_file` it replaces. The original inline +//! unlink ("a quick unlink on the inserting task's thread") is correct +//! for the fast-local-cache case; kept as-is. +//! +//! Run: +//! RUSTFLAGS="-C target-cpu=x86-64-v3" \ +//! cargo run --release --features bench --example bench_round26_diskio +//! Tunables (env): D1_ITERS (20000) + +use std::env; +use std::time::Instant; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +fn gate(tag: &str, metric: &str, before: f64, after: f64) { + if after >= before { + eprintln!("GATE FAIL [{tag}] {metric}: AFTER {after} !< BEFORE {before} — rollback"); + std::process::exit(1); + } +} + +// ── [D1] redundant create_dir_all on a warm shard vs skip ──────────────────── +async fn section_d1() { + let iters: u64 = env_or("D1_ITERS", 20_000); + let dir = tempfile::tempdir().expect("tempdir"); + let shard = dir.path().join("ab"); + // Shard pre-created once (what AFTER's initialize does). + tokio::fs::create_dir_all(&shard).await.unwrap(); + + // warm + let _ = tokio::fs::create_dir_all(&shard).await; + + // BEFORE: per-write create_dir_all(parent) on the already-existing shard. + let t = Instant::now(); + for _ in 0..iters { + let _ = tokio::fs::create_dir_all(&shard).await; + } + let before_ns = t.elapsed().as_nanos() as f64 / iters as f64; + + // AFTER: shard guaranteed present at init → the write path skips the call. + let t = Instant::now(); + for _ in 0..iters { + std::hint::black_box(&shard); + } + let after_ns = t.elapsed().as_nanos() as f64 / iters as f64; + + println!("## [D1] cache-write create_dir_all on a warm shard"); + println!("| arm | ns/write |"); + println!("| BEFORE | {before_ns:>8.1} |"); + println!("| AFTER | {after_ns:>8.1} |"); + println!( + "# {:.1}x — redundant create_dir_all removed per cache write\n", + before_ns / after_ns.max(0.001) + ); + gate("D1", "ns/write", before_ns, after_ns); +} + +#[tokio::main(flavor = "multi_thread", worker_threads = 4)] +async fn main() { + println!("# Round-26 disk-I/O pack\n"); + section_d1().await; + println!("All Round-26 disk-I/O sections passed their gate."); +} diff --git a/examples/bench_round26_hasher.rs b/examples/bench_round26_hasher.rs new file mode 100644 index 00000000..babce78f --- /dev/null +++ b/examples/bench_round26_hasher.rs @@ -0,0 +1,148 @@ +//! Round-26 hasher pack (no Postgres) — wall-gated, since a hasher swap changes +//! 0 allocations (the deterministic alloc counter can't score it). +//! +//! [G1] The delta-upload "have/need" negotiation builds `HashSet`s over up to +//! `max_chunk_count()` client-supplied 64-hex BLAKE3 hashes per request +//! (`distinct_hashes`, `authorize_chunk_download`'s `distinct_seen`). +//! std `HashSet` uses SipHash-1-3 (DoS-resistant but ~2-4x slower on +//! short keys). AFTER uses `foldhash::quality::RandomState` — a faster +//! non-cryptographic hash that STAYS DoS-resistant because it is +//! per-instance random-seeded (the required property for these +//! attacker-controlled inputs — not `FxHash`/fixed-seed). foldhash is +//! already in the lockfile transitively (hashbrown), so it adds no crate. +//! Gate: AFTER wall (build set + membership scan) strictly lower, AND +//! two RandomState instances must seed differently (DoS resistance kept). +//! +//! Run: +//! RUSTFLAGS="-C target-cpu=x86-64-v3" \ +//! cargo run --release --features bench --example bench_round26_hasher +//! Tunables (env): G1_HASHES (40000), G1_PASSES (50) + +use std::collections::HashSet; +use std::env; +use std::hint::black_box; +use std::time::Instant; + +use foldhash::quality::RandomState; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +fn p50(mut s: Vec) -> f64 { + s.sort_by(|a, b| a.partial_cmp(b).unwrap()); + s[s.len() / 2] +} + +fn gate(tag: &str, metric: &str, before: f64, after: f64) { + if after >= before { + eprintln!("GATE FAIL [{tag}] {metric}: AFTER {after} !< BEFORE {before} — rollback"); + std::process::exit(1); + } +} + +/// Deterministic 64-hex "hash" strings (mirror a BLAKE3 chunk hash). +fn hashes(n: usize) -> Vec { + (0..n) + .map(|i| { + let mut s = String::with_capacity(64); + for k in 0..8 { + use std::fmt::Write; + let _ = write!( + s, + "{:08x}", + (i as u64).wrapping_mul(2_654_435_761).wrapping_add(k) + ); + } + s + }) + .collect() +} + +fn main() { + println!("# Round-26 hasher pack\n"); + let n: usize = env_or("G1_HASHES", 40_000); + let passes: usize = env_or("G1_PASSES", 50); + let keys = hashes(n); + + // DoS-safety: two RandomState instances must NOT hash identically (random + // per-instance seed — precomputed-collision attacks stay infeasible). + { + use std::hash::{BuildHasher, Hasher}; + let (a, b) = (RandomState::default(), RandomState::default()); + let mut ha = a.build_hasher(); + let mut hb = b.build_hasher(); + std::hash::Hash::hash(&keys[0], &mut ha); + std::hash::Hash::hash(&keys[0], &mut hb); + if ha.finish() == hb.finish() { + eprintln!("GATE FAIL [G1] two RandomState seeds produced the same hash — not DoS-safe"); + std::process::exit(1); + } + } + + // Equivalence: both build the same distinct set + same membership answers. + let sip: HashSet<&str> = keys.iter().map(|s| s.as_str()).collect(); + let fold: HashSet<&str, RandomState> = keys.iter().map(|s| s.as_str()).collect(); + assert_eq!(sip.len(), fold.len(), "G1 distinct count differs"); + for k in &keys { + assert_eq!( + sip.contains(k.as_str()), + fold.contains(k.as_str()), + "G1 membership differs" + ); + } + + let work_sip = || { + let set: HashSet<&str> = keys.iter().map(|s| s.as_str()).collect(); + let mut hits = 0usize; + for k in &keys { + if set.contains(k.as_str()) { + hits += 1; + } + } + black_box(hits) + }; + let work_fold = || { + let set: HashSet<&str, RandomState> = + HashSet::with_capacity_and_hasher(keys.len(), RandomState::default()); + let mut set = set; + for k in &keys { + set.insert(k.as_str()); + } + let mut hits = 0usize; + for k in &keys { + if set.contains(k.as_str()) { + hits += 1; + } + } + black_box(hits) + }; + + black_box(work_sip()); + black_box(work_fold()); + let mut before = Vec::new(); + let mut after = Vec::new(); + for _ in 0..passes { + let t = Instant::now(); + black_box(work_sip()); + before.push(t.elapsed().as_secs_f64() * 1e3); + let t = Instant::now(); + black_box(work_fold()); + after.push(t.elapsed().as_secs_f64() * 1e3); + } + let b = p50(before); + let a = p50(after); + println!("## [G1] delta-upload hash set: SipHash vs foldhash::quality ({n} hashes)"); + println!("| arm | p50 ms (build+scan) |"); + println!("| BEFORE (SipHash) | {b:>10.3} |"); + println!("| AFTER (foldhash) | {a:>10.3} |"); + println!( + "# {:.2}x wall — DoS resistance retained (random per-instance seed)\n", + b / a.max(1e-9) + ); + gate("G1", "p50 ms", b, a); + println!("Round-26 hasher section passed its gate."); +} diff --git a/examples/bench_round26_micro.rs b/examples/bench_round26_micro.rs new file mode 100644 index 00000000..d6c47358 --- /dev/null +++ b/examples/bench_round26_micro.rs @@ -0,0 +1,153 @@ +//! Round-26 CPU/alloc micro-pack (no Postgres). +//! +//! Same rule as ROUND2–25: each section is BEFORE (verbatim replica of the +//! shipped-before shape) vs AFTER (replica of the shipped-after shape, which the +//! source is then made to match), with a value-equivalence gate and a +//! `GATE FAIL … rollback` `std::process::exit(1)` if the AFTER arm fails to beat +//! BEFORE — the round's roll-back rule encoded into the benchmark. +//! +//! [P1] `drive_pg_repository`'s four policy reads decode `d.policies` into a +//! throwaway `serde_json::Value` DOM and then call +//! `DrivePolicies::from_value(&raw)` (`Self::deserialize(&Value)`) — the +//! exact throwaway-DOM pattern ROUND23 §J1 removed for contacts, but left +//! on the drive-policy path (§J2 removed only the `from_value` clone). The +//! Value tree (a `Map` + boxed String key + `Value` node per policy field) +//! is walked once and dropped. AFTER decodes straight into the struct via +//! `serde_json::from_slice::` (what `sqlx::types::Json` +//! runs on the raw JSONB bytes) — no intermediate DOM. The lenient +//! `unwrap_or_default` fallback is preserved. +//! +//! Run: +//! RUSTFLAGS="-C target-cpu=x86-64-v3" \ +//! cargo run --release --features bench --example bench_round26_micro +//! Tunables (env): P1_ITERS (200000) + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::env; +use std::hint::black_box; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Instant; + +use oxicloud::domain::entities::drive::DrivePolicies; +use serde::Deserialize as _; + +static ALLOC_CALLS: AtomicU64 = AtomicU64::new(0); +static ALLOC_BYTES: AtomicU64 = AtomicU64::new(0); + +struct CountingAlloc; + +unsafe impl GlobalAlloc for CountingAlloc { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + ALLOC_BYTES.fetch_add(layout.size() as u64, Ordering::Relaxed); + unsafe { System.alloc(layout) } + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + unsafe { System.dealloc(ptr, layout) } + } + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + ALLOC_BYTES.fetch_add(new_size as u64, Ordering::Relaxed); + unsafe { System.realloc(ptr, layout, new_size) } + } + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + ALLOC_BYTES.fetch_add(layout.size() as u64, Ordering::Relaxed); + unsafe { System.alloc_zeroed(layout) } + } +} + +#[global_allocator] +static GLOBAL: CountingAlloc = CountingAlloc; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +#[derive(Clone, Copy)] +struct Measure { + ns: f64, + allocs: f64, + bytes: f64, +} + +fn measure(iters: u64, mut f: impl FnMut() -> T) -> Measure { + black_box(f()); + ALLOC_CALLS.store(0, Ordering::Relaxed); + ALLOC_BYTES.store(0, Ordering::Relaxed); + let start = Instant::now(); + for _ in 0..iters { + black_box(f()); + } + let ns = start.elapsed().as_nanos() as f64 / iters as f64; + Measure { + ns, + allocs: ALLOC_CALLS.load(Ordering::Relaxed) as f64 / iters as f64, + bytes: ALLOC_BYTES.load(Ordering::Relaxed) as f64 / iters as f64, + } +} + +fn report(tag: &str, before: Measure, after: Measure) { + println!("## {tag}"); + println!("| arm | ns/op | allocs/op | bytes/op |"); + println!( + "| BEFORE | {:>12.1} | {:>11.2} | {:>11.0} |", + before.ns, before.allocs, before.bytes + ); + println!( + "| AFTER | {:>12.1} | {:>11.2} | {:>11.0} |", + after.ns, after.allocs, after.bytes + ); + println!( + "# {:.2}x wall · {:.2} fewer allocs/op · {:.0} fewer bytes/op\n", + before.ns / after.ns.max(0.0001), + before.allocs - after.allocs, + before.bytes - after.bytes + ); +} + +fn gate(tag: &str, metric: &str, before: f64, after: f64) { + if after >= before { + eprintln!("GATE FAIL [{tag}] {metric}: AFTER {after} !< BEFORE {before} — rollback"); + std::process::exit(1); + } +} + +// ── [P1] drive-policy JSONB decode: Value DOM + from_value vs from_slice ─── +fn section_p1() { + let iters: u64 = env_or("P1_ITERS", 200_000); + // A realistically-populated policies bag (several fields set); the column is + // `jsonb NOT NULL DEFAULT '{}'`, and `#[serde(default)]` fills the rest. + let json: &[u8] = br#"{"forbid_sharing":true,"forbid_public_links":true,"include_in_photo_index":true,"read_only":false,"forbid_cross_drive_move":true}"#; + + // Equivalence: both arms yield the identical DrivePolicies. + let before_val: serde_json::Value = serde_json::from_slice(json).unwrap(); + let before = DrivePolicies::deserialize(&before_val).unwrap_or_default(); + let after = serde_json::from_slice::(json).unwrap_or_default(); + assert_eq!(before, after, "P1 decoded policies differ"); + + let b = measure(iters, || { + // BEFORE: raw JSONB → full serde_json::Value DOM → deserialize(&Value). + let v: serde_json::Value = serde_json::from_slice(black_box(json)).unwrap(); + DrivePolicies::deserialize(&v).unwrap_or_default() + }); + let a = measure(iters, || { + // AFTER: raw JSONB → from_slice:: (what sqlx Json does). + serde_json::from_slice::(black_box(json)).unwrap_or_default() + }); + report( + "[P1] drive-policy JSONB decode (Value DOM vs from_slice)", + b, + a, + ); + gate("P1", "allocs/op", b.allocs, a.allocs); +} + +fn main() { + println!("# Round-26 micro alloc pack\n"); + section_p1(); + println!("All Round-26 micro sections passed their gate."); +} diff --git a/examples/bench_round27_micro.rs b/examples/bench_round27_micro.rs new file mode 100644 index 00000000..5bf05ec2 --- /dev/null +++ b/examples/bench_round27_micro.rs @@ -0,0 +1,206 @@ +//! Round-27 CPU/alloc micro-pack (no Postgres). +//! +//! Same rule as ROUND2–26: BEFORE (replica of the shipped-before shape) vs AFTER +//! (replica of the shipped-after shape, which the source is then made to match), +//! with a value-equivalence gate and a `GATE FAIL … rollback` `exit(1)` if the +//! AFTER arm fails to beat BEFORE. +//! +//! [H1] The NextCloud PROPFIND page loops build `oc:id` as a fresh `String` +//! per child (`format_oc_id(id, svc)` = `format!("{:08}{}", id, instance)`), +//! then pass `oc_id.as_deref()` into `write_{file,folder}_response`. The +//! sibling per-row costs (href, etag, dates) were already reduced to a +//! reused buffer / borrowed events (ROUND19/20); oc:id was the last +//! per-row String. AFTER computes it into one `oc_buf` reused across the +//! page via `format_oc_id_into` — 1 String/row → 0 (amortized). +//! +//! [P2] `contact_pg_repository::{create,update}_contact` build a throwaway +//! `serde_json::Value` per JSONB column (`serde_json::to_value(&dtos)`) +//! and bind that — the Value tree is serialized to JSONB bytes at encode +//! time and dropped. AFTER binds `sqlx::types::Json(&dtos)`, whose +//! `Encode` runs `serde_json::to_writer` straight into the JSONB buffer, +//! skipping the intermediate DOM (the write-side twin of ROUND23 §J1). +//! +//! Run: +//! RUSTFLAGS="-C target-cpu=x86-64-v3" \ +//! cargo run --release --features bench --example bench_round27_micro +//! Tunables (env): H1_ROWS (500), P2_ITERS (100000) + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::env; +use std::fmt::Write as _; +use std::hint::black_box; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Instant; + +use serde::Serialize; + +static ALLOC_CALLS: AtomicU64 = AtomicU64::new(0); + +struct CountingAlloc; + +unsafe impl GlobalAlloc for CountingAlloc { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + unsafe { System.alloc(layout) } + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + unsafe { System.dealloc(ptr, layout) } + } + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + unsafe { System.realloc(ptr, layout, new_size) } + } + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + unsafe { System.alloc_zeroed(layout) } + } +} + +#[global_allocator] +static GLOBAL: CountingAlloc = CountingAlloc; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +fn measure(iters: u64, mut f: impl FnMut()) -> (f64, f64) { + f(); + ALLOC_CALLS.store(0, Ordering::Relaxed); + let start = Instant::now(); + for _ in 0..iters { + f(); + } + let ns = start.elapsed().as_nanos() as f64 / iters as f64; + let allocs = ALLOC_CALLS.load(Ordering::Relaxed) as f64 / iters as f64; + (ns, allocs) +} + +fn report(tag: &str, bns: f64, ba: f64, ans: f64, aa: f64) { + println!("## {tag}"); + println!("| arm | ns/op | allocs/op |"); + println!("| BEFORE | {bns:>9.1} | {ba:>9.2} |"); + println!("| AFTER | {ans:>9.1} | {aa:>9.2} |"); + println!( + "# {:.2}x wall · {:.2} fewer allocs/op\n", + bns / ans.max(0.0001), + ba - aa + ); +} + +fn gate(tag: &str, before: f64, after: f64) { + if after >= before { + eprintln!("GATE FAIL [{tag}] allocs/op: AFTER {after} !< BEFORE {before} — rollback"); + std::process::exit(1); + } +} + +// ── [H1] oc:id per-row String vs reused buffer ─────────────────────────────── +fn format_oc_id(id: i64, instance: &str) -> String { + format!("{id:08}{instance}") +} +fn format_oc_id_into(out: &mut String, id: i64, instance: &str) { + out.clear(); + let _ = write!(out, "{id:08}"); + out.push_str(instance); +} + +fn section_h1() { + let rows: usize = env_or("H1_ROWS", 500); + let instance = "ocnca"; + + // Equivalence: the reused-buffer output matches the per-row String byte-for-byte. + for id in [0i64, 7, 12345, 99_999_999] { + let mut buf = String::new(); + format_oc_id_into(&mut buf, id, instance); + assert_eq!(buf, format_oc_id(id, instance), "H1 oc:id differs"); + } + + let (bns, ba) = measure(2000, || { + // BEFORE: one String per row. + let mut sink = 0usize; + for i in 0..rows { + let s = format_oc_id(black_box(i as i64), instance); + sink += s.len(); + } + black_box(sink); + }); + let (ans, aa) = measure(2000, || { + // AFTER: one buffer reused across the page. + let mut oc_buf = String::new(); + let mut sink = 0usize; + for i in 0..rows { + format_oc_id_into(&mut oc_buf, black_box(i as i64), instance); + sink += oc_buf.len(); + } + black_box(sink); + }); + report( + &format!("[H1] PROPFIND oc:id ({rows} rows)"), + bns, + ba, + ans, + aa, + ); + gate("H1", ba, aa); +} + +// ── [P2] contact JSONB write: to_value DOM vs direct serialize (Json) ────── +#[derive(Serialize, serde::Deserialize, Clone, PartialEq, Debug)] +struct EmailDto { + email: String, + r#type: String, + is_primary: bool, +} + +fn section_p2() { + let iters: u64 = env_or("P2_ITERS", 100_000); + let dtos: Vec = (0..3) + .map(|i| EmailDto { + email: format!("user{i}@example.com"), + r#type: "home".into(), + is_primary: i == 0, + }) + .collect(); + + // Equivalence: the two serializations differ only in key ORDER — + // `serde_json::to_value` builds a (sorted) Map, direct serialize keeps struct + // order — but Postgres normalizes JSONB key order, so the STORED value and + // the read-back DTOs are identical (verified via psql: + // `'{...alpha...}'::jsonb = '{...struct...}'::jsonb` → t). Assert the + // semantic equivalence: both decode back to the same DTOs. + let via_dom = serde_json::to_vec(&serde_json::to_value(&dtos).unwrap()).unwrap(); + let direct = serde_json::to_vec(&dtos).unwrap(); + let from_dom: Vec = serde_json::from_slice(&via_dom).unwrap(); + let from_direct: Vec = serde_json::from_slice(&direct).unwrap(); + assert_eq!(from_dom, from_direct, "P2 decoded DTOs differ"); + + let (bns, ba) = measure(iters, || { + // BEFORE: build a serde_json::Value DOM, then serialize it (what + // `to_value(&dtos)` + binding the Value does). + let v = serde_json::to_value(black_box(&dtos)).unwrap(); + black_box(serde_json::to_vec(&v).unwrap()); + }); + let (ans, aa) = measure(iters, || { + // AFTER: serialize the DTOs straight to JSONB bytes (what + // `Json(&dtos)`'s Encode does via to_writer) — no intermediate DOM. + black_box(serde_json::to_vec(black_box(&dtos)).unwrap()); + }); + report( + "[P2] contact JSONB write (Value DOM vs direct serialize)", + bns, + ba, + ans, + aa, + ); + gate("P2", ba, aa); +} + +fn main() { + println!("# Round-27 micro alloc pack\n"); + section_h1(); + section_p2(); + println!("All Round-27 micro sections passed their gate."); +} diff --git a/src/application/services/delta_upload_service.rs b/src/application/services/delta_upload_service.rs index e2839ccb..1f42383d 100644 --- a/src/application/services/delta_upload_service.rs +++ b/src/application/services/delta_upload_service.rs @@ -446,8 +446,12 @@ impl DeltaUploadService { ct if ct.is_empty() => "application/octet-stream".to_string(), ct => ct, }; - let chunk_hashes: Vec = request.chunks.iter().map(|c| c.h.clone()).collect(); - let chunk_sizes: Vec = request.chunks.iter().map(|c| c.s).collect(); + // `request.chunks` is owned and dead after this line (only + // `request.file_hash` is read below), so move the hashes out instead of + // cloning each 64-char hash a third time — the distinct set and the + // verification tuple already materialized it twice (benches/ROUND25.md §M2). + let (chunk_hashes, chunk_sizes): (Vec, Vec) = + request.chunks.into_iter().map(|c| (c.h, c.s)).unzip(); let attached = self .dedup .attach_manifest( @@ -549,7 +553,10 @@ impl DeltaUploadService { self.max_chunk_count() ))); } - let mut distinct_seen = HashSet::new(); + // foldhash::quality::RandomState — a fast, per-instance random-seeded + // hasher, DoS-safe for these attacker-controlled client hashes (up to + // max_chunk_count() of them per request) — benches/ROUND26.md §G1. + let mut distinct_seen: HashSet<&str, foldhash::quality::RandomState> = HashSet::default(); for hash in &request.hashes { if !is_valid_hash(hash) { return Err(DomainError::validation_error( @@ -680,7 +687,9 @@ fn sanitize_file_name(name: &str) -> Result { /// Distinct hashes in first-occurrence order. fn distinct_hashes(chunks: &[ChunkRef]) -> Vec { - let mut seen = HashSet::new(); + // foldhash::quality::RandomState — fast, per-instance random-seeded and thus + // DoS-safe for these attacker-controlled client hashes (benches/ROUND26.md §G1). + let mut seen: HashSet<&str, foldhash::quality::RandomState> = HashSet::default(); chunks .iter() .filter(|c| seen.insert(c.h.as_str())) diff --git a/src/infrastructure/adapters/music_storage_adapter.rs b/src/infrastructure/adapters/music_storage_adapter.rs index f8720e17..129f07a5 100644 --- a/src/infrastructure/adapters/music_storage_adapter.rs +++ b/src/infrastructure/adapters/music_storage_adapter.rs @@ -140,19 +140,18 @@ impl MusicStoragePort for MusicStorageAdapter { limit: i64, offset: i64, ) -> Result, DomainError> { + // One `LEFT JOIN … GROUP BY` instead of 1 listing + N per-playlist + // `COUNT(*)` round-trips (up to 101 at limit=100) — benches/ROUND25.md §Q1. let playlists = self .playlist_repository - .list_public_playlists(limit, offset) + .list_public_playlists_with_counts(limit, offset) .await?; - let mut result = Vec::new(); - for playlist in playlists { - let dto = PlaylistDto::from(playlist); - let track_count = self - .get_track_count(&uuid::Uuid::parse_str(&dto.id).unwrap()) - .await?; - result.push(dto.with_track_info(track_count, 0)); - } - Ok(result) + Ok(playlists + .into_iter() + .map(|(playlist, track_count)| { + PlaylistDto::from(playlist).with_track_info(track_count, 0) + }) + .collect()) } async fn user_has_access(&self, playlist_id: &str, user_id: Uuid) -> Result { diff --git a/src/infrastructure/repositories/pg/contact_pg_repository.rs b/src/infrastructure/repositories/pg/contact_pg_repository.rs index d6ab1e5a..da2e6be6 100644 --- a/src/infrastructure/repositories/pg/contact_pg_repository.rs +++ b/src/infrastructure/repositories/pg/contact_pg_repository.rs @@ -1,5 +1,4 @@ use chrono::Utc; -use serde_json::Value as JsonValue; use sqlx::{PgPool, Row, types::Uuid}; use std::sync::Arc; @@ -21,8 +20,28 @@ impl ContactPgRepository { Self { pool } } - /// Maps a database row to a Contact domain entity + /// Maps a database row to a Contact domain entity (reads the `vcard` column). fn row_to_contact(row: &sqlx::postgres::PgRow) -> Result { + Self::row_to_contact_with_vcard(row, row.get("vcard")) + } + + /// Maps a row whose SELECT omitted the `vcard` column — used by the REST + /// listings (paginated / search / by-group) whose `ContactDto` drops vcard + /// anyway, so the multi-KB vCard TEXT (which can embed a base64 PHOTO) is + /// never SELECTed, shipped over the wire, or allocated (benches/ROUND25.md + /// §Q2). The domain `Contact` keeps an empty vcard; these paths never + /// re-emit it. Do NOT use for CardDAV sync / whole-book export, which need + /// the round-trip vCard. + fn row_to_contact_lite(row: &sqlx::postgres::PgRow) -> Result { + Self::row_to_contact_with_vcard(row, String::new()) + } + + /// Shared row → `Contact` mapper; `vcard` is supplied by the caller so the + /// TEXT column can be omitted from listings that don't consume it. + fn row_to_contact_with_vcard( + row: &sqlx::postgres::PgRow, + vcard: String, + ) -> Result { // Decode each JSONB column straight into its typed Vec via // `sqlx::types::Json` (a single `serde_json::from_slice` pass over // the raw JSONB bytes) instead of `row.get::` + @@ -63,7 +82,7 @@ impl ContactPgRepository { row.get::, _>("photo_url"), row.get("birthday"), row.get("anniversary"), - row.get("vcard"), + vcard, row.get("etag"), row.get("created_at"), row.get("updated_at"), @@ -78,10 +97,6 @@ impl ContactRepository for ContactPgRepository { let phone_dtos = phones_to_persistence(contact.phone()); let address_dtos = addresses_to_persistence(contact.address()); - let email_json = serde_json::to_value(&email_dtos).unwrap_or(JsonValue::Null); - let phone_json = serde_json::to_value(&phone_dtos).unwrap_or(JsonValue::Null); - let address_json = serde_json::to_value(&address_dtos).unwrap_or(JsonValue::Null); - let row = sqlx::query( r#" INSERT INTO carddav.contacts ( @@ -106,9 +121,9 @@ impl ContactRepository for ContactPgRepository { .bind(contact.first_name_owned()) .bind(contact.last_name_owned()) .bind(contact.nickname_owned()) - .bind(email_json) - .bind(phone_json) - .bind(address_json) + .bind(sqlx::types::Json(&email_dtos)) + .bind(sqlx::types::Json(&phone_dtos)) + .bind(sqlx::types::Json(&address_dtos)) .bind(contact.organization_owned()) .bind(contact.title_owned()) .bind(contact.notes_owned()) @@ -133,10 +148,6 @@ impl ContactRepository for ContactPgRepository { let phone_dtos = phones_to_persistence(contact.phone()); let address_dtos = addresses_to_persistence(contact.address()); - let email_json = serde_json::to_value(&email_dtos).unwrap_or(JsonValue::Null); - let phone_json = serde_json::to_value(&phone_dtos).unwrap_or(JsonValue::Null); - let address_json = serde_json::to_value(&address_dtos).unwrap_or(JsonValue::Null); - // Create a clone of the contact with the updated timestamp let mut updated_contact = contact.clone(); updated_contact.set_updated_at(now); @@ -172,9 +183,9 @@ impl ContactRepository for ContactPgRepository { .bind(updated_contact.first_name_owned()) .bind(updated_contact.last_name_owned()) .bind(updated_contact.nickname_owned()) - .bind(email_json) - .bind(phone_json) - .bind(address_json) + .bind(sqlx::types::Json(&email_dtos)) + .bind(sqlx::types::Json(&phone_dtos)) + .bind(sqlx::types::Json(&address_dtos)) .bind(updated_contact.organization_owned()) .bind(updated_contact.title_owned()) .bind(updated_contact.notes_owned()) @@ -366,7 +377,7 @@ impl ContactRepository for ContactPgRepository { SELECT id, address_book_id, uid, full_name, first_name, last_name, nickname, email, phone, address, organization, title, notes, photo_url, - birthday, anniversary, vcard, etag, created_at, updated_at + birthday, anniversary, etag, created_at, updated_at FROM carddav.contacts WHERE address_book_id = $1 ORDER BY full_name, first_name, last_name @@ -387,7 +398,7 @@ impl ContactRepository for ContactPgRepository { let mut contacts = Vec::with_capacity(rows.len()); for row in &rows { - contacts.push(Self::row_to_contact(row)?); + contacts.push(Self::row_to_contact_lite(row)?); } Ok(contacts) } @@ -429,7 +440,7 @@ impl ContactRepository for ContactPgRepository { SELECT c.id, c.address_book_id, c.uid, c.full_name, c.first_name, c.last_name, c.nickname, c.email, c.phone, c.address, c.organization, c.title, c.notes, c.photo_url, - c.birthday, c.anniversary, c.vcard, c.etag, c.created_at, c.updated_at + c.birthday, c.anniversary, c.etag, c.created_at, c.updated_at FROM carddav.contacts c INNER JOIN carddav.group_memberships m ON c.id = m.contact_id WHERE m.group_id = $1 @@ -445,7 +456,7 @@ impl ContactRepository for ContactPgRepository { let mut contacts = Vec::with_capacity(rows.len()); for row in &rows { - contacts.push(Self::row_to_contact(row)?); + contacts.push(Self::row_to_contact_lite(row)?); } Ok(contacts) } @@ -462,9 +473,9 @@ impl ContactRepository for ContactPgRepository { SELECT id, address_book_id, uid, full_name, first_name, last_name, nickname, email, phone, address, organization, title, notes, photo_url, - birthday, anniversary, vcard, etag, created_at, updated_at + birthday, anniversary, etag, created_at, updated_at FROM carddav.contacts - WHERE address_book_id = $1 + WHERE address_book_id = $1 AND ( full_name ILIKE $2 OR first_name ILIKE $2 @@ -485,7 +496,7 @@ impl ContactRepository for ContactPgRepository { let mut contacts = Vec::with_capacity(rows.len()); for row in &rows { - contacts.push(Self::row_to_contact(row)?); + contacts.push(Self::row_to_contact_lite(row)?); } Ok(contacts) } diff --git a/src/infrastructure/repositories/pg/drive_pg_repository.rs b/src/infrastructure/repositories/pg/drive_pg_repository.rs index 0e5c57de..e7f51ede 100644 --- a/src/infrastructure/repositories/pg/drive_pg_repository.rs +++ b/src/infrastructure/repositories/pg/drive_pg_repository.rs @@ -20,6 +20,19 @@ use crate::domain::repositories::drive_repository::{ DriveRepository, DriveRepositoryError, DriveWithRootName, }; +/// Decode a `d.policies` JSONB column straight into `DrivePolicies` via +/// `sqlx::types::Json` — a single `serde_json::from_slice` over the raw JSONB +/// bytes — instead of fetching a throwaway `serde_json::Value` DOM and walking it +/// once with `DrivePolicies::from_value`. The §J1 pattern (ROUND23) applied to +/// the drive-policy path §J2 left behind (benches/ROUND26.md §P1). The lenient +/// `unwrap_or_default` fallback (a malformed bag decodes to all-false rather than +/// erroring the read) is preserved exactly. +fn policies_from_row(row: &sqlx::postgres::PgRow) -> crate::domain::entities::drive::DrivePolicies { + row.try_get::, _>("policies") + .map(|j| j.0) + .unwrap_or_default() +} + /// `default_drive_cache` TTL. The default-drive → root-folder binding is /// nearly immutable (changes only on provisioning / drive deletion / /// policy edits — all of which invalidate explicitly below), yet it is @@ -706,7 +719,7 @@ impl DriveRepository for DrivePgRepository { &self, file_id: Uuid, ) -> Result { - let row: Option<(serde_json::Value,)> = sqlx::query_as( + let row = sqlx::query( "SELECT d.policies \ FROM storage.drives d \ JOIN storage.files f ON f.drive_id = d.id \ @@ -715,20 +728,16 @@ impl DriveRepository for DrivePgRepository { .bind(file_id) .fetch_optional(self.pool.as_ref()) .await - .map_err(|e| Self::map_sqlx_err("get_policies_for_file", e))?; - let raw = row - .ok_or_else(|| DriveRepositoryError::NotFound(file_id.to_string()))? - .0; - Ok(crate::domain::entities::drive::DrivePolicies::from_value( - &raw, - )) + .map_err(|e| Self::map_sqlx_err("get_policies_for_file", e))? + .ok_or_else(|| DriveRepositoryError::NotFound(file_id.to_string()))?; + Ok(policies_from_row(&row)) } async fn get_policies_for_folder( &self, folder_id: Uuid, ) -> Result { - let row: Option<(serde_json::Value,)> = sqlx::query_as( + let row = sqlx::query( "SELECT d.policies \ FROM storage.drives d \ JOIN storage.folders fo ON fo.drive_id = d.id \ @@ -737,20 +746,16 @@ impl DriveRepository for DrivePgRepository { .bind(folder_id) .fetch_optional(self.pool.as_ref()) .await - .map_err(|e| Self::map_sqlx_err("get_policies_for_folder", e))?; - let raw = row - .ok_or_else(|| DriveRepositoryError::NotFound(folder_id.to_string()))? - .0; - Ok(crate::domain::entities::drive::DrivePolicies::from_value( - &raw, - )) + .map_err(|e| Self::map_sqlx_err("get_policies_for_folder", e))? + .ok_or_else(|| DriveRepositoryError::NotFound(folder_id.to_string()))?; + Ok(policies_from_row(&row)) } async fn get_drive_id_and_policies_for_file( &self, file_id: Uuid, ) -> Result<(Uuid, crate::domain::entities::drive::DrivePolicies), DriveRepositoryError> { - let row: Option<(Uuid, serde_json::Value)> = sqlx::query_as( + let row = sqlx::query( "SELECT d.id, d.policies \ FROM storage.drives d \ JOIN storage.files f ON f.drive_id = d.id \ @@ -759,20 +764,19 @@ impl DriveRepository for DrivePgRepository { .bind(file_id) .fetch_optional(self.pool.as_ref()) .await - .map_err(|e| Self::map_sqlx_err("get_drive_id_and_policies_for_file", e))?; - let (drive_id, raw) = - row.ok_or_else(|| DriveRepositoryError::NotFound(file_id.to_string()))?; - Ok(( - drive_id, - crate::domain::entities::drive::DrivePolicies::from_value(&raw), - )) + .map_err(|e| Self::map_sqlx_err("get_drive_id_and_policies_for_file", e))? + .ok_or_else(|| DriveRepositoryError::NotFound(file_id.to_string()))?; + let drive_id: Uuid = row + .try_get("id") + .map_err(|e| Self::map_sqlx_err("get_drive_id_and_policies_for_file", e))?; + Ok((drive_id, policies_from_row(&row))) } async fn get_drive_id_and_policies_for_folder( &self, folder_id: Uuid, ) -> Result<(Uuid, crate::domain::entities::drive::DrivePolicies), DriveRepositoryError> { - let row: Option<(Uuid, serde_json::Value)> = sqlx::query_as( + let row = sqlx::query( "SELECT d.id, d.policies \ FROM storage.drives d \ JOIN storage.folders fo ON fo.drive_id = d.id \ @@ -781,13 +785,12 @@ impl DriveRepository for DrivePgRepository { .bind(folder_id) .fetch_optional(self.pool.as_ref()) .await - .map_err(|e| Self::map_sqlx_err("get_drive_id_and_policies_for_folder", e))?; - let (drive_id, raw) = - row.ok_or_else(|| DriveRepositoryError::NotFound(folder_id.to_string()))?; - Ok(( - drive_id, - crate::domain::entities::drive::DrivePolicies::from_value(&raw), - )) + .map_err(|e| Self::map_sqlx_err("get_drive_id_and_policies_for_folder", e))? + .ok_or_else(|| DriveRepositoryError::NotFound(folder_id.to_string()))?; + let drive_id: Uuid = row + .try_get("id") + .map_err(|e| Self::map_sqlx_err("get_drive_id_and_policies_for_folder", e))?; + Ok((drive_id, policies_from_row(&row))) } async fn drive_id_for_folder(&self, folder_id: Uuid) -> Result { diff --git a/src/infrastructure/repositories/pg/playlist_pg_repository.rs b/src/infrastructure/repositories/pg/playlist_pg_repository.rs index d0f5f95d..97659fdb 100644 --- a/src/infrastructure/repositories/pg/playlist_pg_repository.rs +++ b/src/infrastructure/repositories/pg/playlist_pg_repository.rs @@ -22,6 +22,21 @@ struct PlaylistRow { updated_at: DateTime, } +/// A public playlist row carrying its aggregated track count, produced by the +/// single `LEFT JOIN … GROUP BY` that replaces the per-playlist `COUNT(*)` N+1. +#[derive(FromRow)] +struct PublicPlaylistCountRow { + id: Uuid, + name: String, + description: Option, + owner_id: Uuid, + is_public: bool, + cover_file_id: Option, + created_at: DateTime, + updated_at: DateTime, + track_count: i64, +} + #[derive(FromRow)] struct PlaylistItemRow { id: Uuid, @@ -79,6 +94,52 @@ impl PlaylistPgRepository { pub fn pool(&self) -> &PgPool { &self.pool } + + /// Public playlists together with their track counts in a **single** + /// round-trip. Replaces the adapter's 1 + N shape (one listing SELECT then + /// one `SELECT COUNT(*) FROM audio.playlist_items` per returned playlist — + /// up to 101 round-trips at `limit = 100`) with one `LEFT JOIN … GROUP BY`, + /// backed by `idx_playlist_items_playlist_id` (benches/ROUND25.md §Q1). + pub async fn list_public_playlists_with_counts( + &self, + limit: i64, + offset: i64, + ) -> PlaylistRepositoryResult> { + let rows = sqlx::query_as::<_, PublicPlaylistCountRow>( + "SELECT p.id, p.name, p.description, p.owner_id, p.is_public, p.cover_file_id, \ + p.created_at, p.updated_at, COUNT(pi.id) AS track_count \ + FROM audio.playlists p \ + LEFT JOIN audio.playlist_items pi ON pi.playlist_id = p.id \ + WHERE p.is_public = TRUE \ + GROUP BY p.id \ + ORDER BY p.updated_at DESC LIMIT $1 OFFSET $2", + ) + .bind(limit) + .bind(offset) + .fetch_all(&*self.pool) + .await + .map_err(|e| { + DomainError::database_error(format!("Failed to list public playlists: {}", e)) + })?; + + rows.into_iter() + .map(|row| { + let track_count = row.track_count; + Playlist::with_id( + row.id, + row.name, + row.description, + row.owner_id, + row.is_public, + row.cover_file_id, + row.created_at, + row.updated_at, + ) + .map(|p| (p, track_count)) + .map_err(|e| DomainError::new(ErrorKind::InternalError, "Playlist", e.to_string())) + }) + .collect() + } } impl PlaylistRepository for PlaylistPgRepository { diff --git a/src/infrastructure/services/cached_blob_backend.rs b/src/infrastructure/services/cached_blob_backend.rs index 36bec900..a38220be 100644 --- a/src/infrastructure/services/cached_blob_backend.rs +++ b/src/infrastructure/services/cached_blob_backend.rs @@ -119,10 +119,21 @@ impl BlobStorageBackend for CachedBlobBackend { Box::pin(async move { inner.initialize().await?; - // Create cache dir structure (256 prefix dirs) + // Create the cache dir AND its 256 {00..ff} shard dirs up front + // (mirroring LocalBlobBackend::initialize), so the write paths never + // pay a per-chunk `create_dir_all` on an already-existing shard — a + // ~45 µs mkdirat(EEXIST)+stat+blocking-dispatch removed per cache + // write on cached-remote deployments (benches/ROUND26.md §D1). fs::create_dir_all(&cache_dir).await.map_err(|e| { DomainError::internal_error("BlobCache", format!("mkdir cache_dir: {e}")) })?; + for prefix in &crate::infrastructure::services::local_blob_backend::HEX_PREFIXES { + fs::create_dir_all(cache_dir.join(prefix)) + .await + .map_err(|e| { + DomainError::internal_error("BlobCache", format!("mkdir cache shard: {e}")) + })?; + } // Scan existing cache to rebuild index. Collect entries WITHOUT // holding the index lock — a large cache directory walk must not @@ -411,10 +422,9 @@ impl CachedBlobBackend { /// index deliberately skipped the eviction sweep on this path, letting /// write bursts overshoot the budget until the next read-miss insert). async fn cache_bytes_write_through(&self, hash: String, data: &Bytes) { + // The shard dir was created at initialize() — no per-write create_dir_all + // (benches/ROUND26.md §D1). let dest = self.cached_path(&hash); - if let Some(parent) = dest.parent() { - let _ = fs::create_dir_all(parent).await; - } let _ = fs::write(&dest, data).await; let data_len = data.len() as u64; self.index.insert(hash, CacheEntry { size: data_len }); @@ -451,12 +461,8 @@ impl CachedBlobBackend { } async fn insert_into_cache(&self, hash: &str, source_path: &Path) -> Result<(), DomainError> { + // Shard dir pre-created at initialize() (benches/ROUND26.md §D1). let dest = self.cached_path(hash); - if let Some(parent) = dest.parent() { - fs::create_dir_all(parent).await.map_err(|e| { - DomainError::internal_error("BlobCache", format!("mkdir failed: {e}")) - })?; - } let size = fs::metadata(source_path) .await @@ -476,12 +482,8 @@ impl CachedBlobBackend { async fn fetch_and_cache(&self, hash: &str) -> Result { let stream = self.inner.get_blob_stream(hash).await?; + // Shard dir pre-created at initialize() (benches/ROUND26.md §D1). let dest = self.cached_path(hash); - if let Some(parent) = dest.parent() { - fs::create_dir_all(parent).await.map_err(|e| { - DomainError::internal_error("BlobCache", format!("mkdir failed: {e}")) - })?; - } // Unique temp name: even if two fetches for one hash ever race // (e.g. across processes sharing a cache dir), each writes its own diff --git a/src/infrastructure/services/encrypted_blob_backend.rs b/src/infrastructure/services/encrypted_blob_backend.rs index d8d0cb73..272d7806 100644 --- a/src/infrastructure/services/encrypted_blob_backend.rs +++ b/src/infrastructure/services/encrypted_blob_backend.rs @@ -86,7 +86,7 @@ impl EncryptedBlobBackend { /// Encrypt `data` into the on-disk layout: `[12-byte nonce][ciphertext + tag]`. /// -/// Single output buffer, mirroring the read side's `decrypt_in_place`: +/// Single output buffer, mirroring the read side's in-place detached decrypt: /// the payload is copied exactly once and encrypted in place with the tag /// appended. The old shape let `cipher.encrypt` allocate a full ciphertext /// `Vec` and then copied it a second time behind the nonce — one extra @@ -106,22 +106,39 @@ fn encrypt_bytes(cipher: &Aes256Gcm, data: &[u8]) -> Result /// Decrypt the on-disk layout `[nonce][ciphertext + tag]` **in place**. /// -/// Consumes the encrypted buffer and reuses it for the plaintext, so peak -/// RAM is one buffer — not ciphertext + plaintext side by side (which for -/// legacy whole-file blobs would double a multi-hundred-MB allocation). +/// Reuses the encrypted buffer for the plaintext, so peak RAM is one buffer — +/// not ciphertext + plaintext side by side (which for legacy whole-file blobs +/// would double a multi-hundred-MB allocation). The nonce and 16-byte GCM tag +/// are lifted to the stack, the ciphertext body is decrypted in place via the +/// detached API (mirroring the encrypt side's `encrypt_in_place_detached`), and +/// the plaintext is returned as a zero-copy `Bytes::slice` past the nonce. +/// +/// The prior shape did `encrypted.split_off(NONCE_SIZE)`, which allocated a +/// fresh `Vec` and memcpy'd the entire ciphertext (up to a whole legacy blob) +/// on every decrypted read — one full-payload allocation + copy the doc comment +/// above claimed did not happen (benches/ROUND25.md §M1; ROUND11 §15 fixed only +/// the encrypt side). Output plaintext is byte-identical. fn decrypt_bytes(cipher: &Aes256Gcm, mut encrypted: Vec) -> Result { - if encrypted.len() < NONCE_SIZE { + let len = encrypted.len(); + if len < NONCE_SIZE + TAG_SIZE { return Err(DomainError::internal_error( "Encryption", - "encrypted blob too short (missing nonce)", + "encrypted blob too short (missing nonce/tag)", )); } - let mut ciphertext = encrypted.split_off(NONCE_SIZE); // `encrypted` keeps the nonce - let nonce = Nonce::from_slice(&encrypted); + // Nonce (first 12 bytes) and GCM tag (last 16 bytes) copied to the stack so + // the middle can be borrowed mutably for in-place decryption. + let mut nonce_buf = [0u8; NONCE_SIZE]; + nonce_buf.copy_from_slice(&encrypted[..NONCE_SIZE]); + let nonce = Nonce::from_slice(&nonce_buf); + let tag = aes_gcm::aead::Tag::::clone_from_slice(&encrypted[len - TAG_SIZE..]); cipher - .decrypt_in_place(nonce, b"", &mut ciphertext) + .decrypt_in_place_detached(nonce, b"", &mut encrypted[NONCE_SIZE..len - TAG_SIZE], &tag) .map_err(|e| DomainError::internal_error("Encryption", format!("decrypt failed: {e}")))?; - Ok(Bytes::from(ciphertext)) + // Plaintext now lives at `encrypted[NONCE_SIZE..len - TAG_SIZE]`; drop the + // tag and hand out a refcounted view past the nonce — no copy, no new alloc. + encrypted.truncate(len - TAG_SIZE); + Ok(Bytes::from(encrypted).slice(NONCE_SIZE..)) } /// Run a crypto closure inline for small payloads, on the blocking pool for diff --git a/src/infrastructure/services/local_blob_backend.rs b/src/infrastructure/services/local_blob_backend.rs index 40f932be..556c5b19 100644 --- a/src/infrastructure/services/local_blob_backend.rs +++ b/src/infrastructure/services/local_blob_backend.rs @@ -164,7 +164,7 @@ pub async fn write_blob_bytes_for_bench( } /// Compile-time lookup table for the 256 two-digit lowercase hex prefixes ("00"…"ff"). -static HEX_PREFIXES: [&str; 256] = [ +pub(crate) static HEX_PREFIXES: [&str; 256] = [ "00", "01", "02", "03", "04", "05", "06", "07", "08", "09", "0a", "0b", "0c", "0d", "0e", "0f", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "1a", "1b", "1c", "1d", "1e", "1f", "20", "21", "22", "23", "24", "25", "26", "27", "28", "29", "2a", "2b", "2c", "2d", "2e", "2f", diff --git a/src/interfaces/api/handlers/folder_handler.rs b/src/interfaces/api/handlers/folder_handler.rs index 5956adab..726b27b7 100644 --- a/src/interfaces/api/handlers/folder_handler.rs +++ b/src/interfaces/api/handlers/folder_handler.rs @@ -4,7 +4,6 @@ use axum::{ http::{Response, StatusCode, header}, response::IntoResponse, }; -use std::collections::HashMap; use std::sync::Arc; use crate::application::dtos::display_helpers::{ @@ -210,7 +209,6 @@ impl FolderHandler { State(state): State>, auth_user: AuthUser, Path(id): Path, - Query(_params): Query>, ) -> impl IntoResponse { tracing::info!("Downloading folder as ZIP: {}", id); @@ -421,9 +419,12 @@ pub async fn download_folder_zip( state: State>, auth_user: AuthUser, path: Path, - query: Query>, ) -> impl IntoResponse { - FolderHandler::download_folder_zip_impl(state, auth_user, path, query).await + // No `Query` extractor: the handler reads only the path `id`. axum ignores + // any query string when no extractor is present, so the response is + // byte-identical while a per-request HashMap + owned key/value Strings are + // no longer parsed and dropped (benches/ROUND25.md §M3). + FolderHandler::download_folder_zip_impl(state, auth_user, path).await } // ── GET /api/folders/{id}/resources ───────────────────────────────────────── diff --git a/src/interfaces/nextcloud/report_handler.rs b/src/interfaces/nextcloud/report_handler.rs index 4f2cfdcd..5d1c6a76 100644 --- a/src/interfaces/nextcloud/report_handler.rs +++ b/src/interfaces/nextcloud/report_handler.rs @@ -24,7 +24,8 @@ use crate::interfaces::api::handlers::webdav_handler::{ }; use crate::interfaces::errors::AppError; use crate::interfaces::nextcloud::webdav_handler::{ - batch_resolve_ids, format_oc_id, nc_href, nc_id_of, write_file_response, write_folder_response, + batch_resolve_ids, format_oc_id_into, nc_href, nc_id_of, write_file_response, + write_folder_response, }; /// Handle WebDAV REPORT and SEARCH methods for Nextcloud compatibility. @@ -174,6 +175,8 @@ async fn handle_filter_files( // per type, not 2N round-trips). Hrefs use `url_user` so the // multi-drive `~{drive}` form is echoed back to the client; // owner-id stays canonical via `&user.username`. + // One oc:id buffer reused across both emit loops (benches/ROUND27.md §H1). + let mut oc_buf = String::new(); for file in &files { // Skip favorites that live outside the caller's chroot // (other-drive favorites); reachable via REST if needed. @@ -188,13 +191,19 @@ async fn handle_filter_files( }; let href = nc_href(url_user, subpath); let fid = nc_id_of(&file_id_map, &file.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; let dead = dead_props_for(&file.id, &file_deads); write_file_response( &mut xml, file, &href, - (fid, oc_id.as_deref()), + (fid, oc_id), &user.username, &favorite_ids, dead, @@ -214,13 +223,19 @@ async fn handle_filter_files( }; let href = format!("{}/", nc_href(url_user, subpath)); let fid = nc_id_of(&folder_id_map, &folder.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; let dead = dead_props_for(&folder.id, &folder_deads); write_folder_response( &mut xml, folder, &href, - (fid, oc_id.as_deref()), + (fid, oc_id), &user.username, &favorite_ids, // REPORT results are a flat filter/search listing, not a @@ -317,6 +332,8 @@ async fn handle_search( let folder_deads = folders_dead_props_map(&state.webdav_dead_props, &folders).await; // Files. + // One oc:id buffer reused across both emit loops (benches/ROUND27.md §H1). + let mut oc_buf = String::new(); for file in &files { let Some(subpath) = strip_home_prefix(chroot, &file.path, home_prefix) else { tracing::debug!( @@ -329,13 +346,19 @@ async fn handle_search( }; let href = nc_href(url_user, subpath); let fid = nc_id_of(&file_id_map, &file.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; let dead = dead_props_for(&file.id, &file_deads); write_file_response( &mut xml, file, &href, - (fid, oc_id.as_deref()), + (fid, oc_id), &user.username, &favorite_ids, dead, @@ -356,13 +379,19 @@ async fn handle_search( }; let href = format!("{}/", nc_href(url_user, subpath)); let fid = nc_id_of(&folder_id_map, &folder.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; let dead = dead_props_for(&folder.id, &folder_deads); write_folder_response( &mut xml, folder, &href, - (fid, oc_id.as_deref()), + (fid, oc_id), &user.username, &favorite_ids, // REPORT results are a flat filter/search listing, not a diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index 2cb8ef28..ea8abfbd 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -1610,8 +1610,10 @@ fn build_nc_streaming_propfind( { let mut xml = Writer::new(&mut chunk); // One href buffer reused across the page instead of a fresh - // format! String per child (benches/ROUND19.md §M6). + // format! String per child (benches/ROUND19.md §M6); likewise + // one oc:id buffer (benches/ROUND27.md §H1). let mut href = String::new(); + let mut oc_buf = String::new(); for file in batch.iter() { let dead = dead_props_for(&file.id, &file_deads); // Only the name varies per row — the encoded @@ -1622,8 +1624,14 @@ fn build_nc_streaming_propfind( href.push_str(&child_href_prefix); href.push_str(&urlencoding::encode(&file.name)); let fid = nc_id_of(&file_id_map, &file.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); - write_file_response(&mut xml, file, &href, (fid, oc_id.as_deref()), &username, &favs, dead) + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; + write_file_response(&mut xml, file, &href, (fid, oc_id), &username, &favs, dead) .map_err(std::io::Error::other)?; } } @@ -1675,8 +1683,10 @@ fn build_nc_streaming_propfind( let mut chunk = Vec::with_capacity(batch.len() * 1024); { let mut xml = Writer::new(&mut chunk); - // One href buffer reused across the page (benches/ROUND19.md §M6). + // One href buffer reused across the page (benches/ROUND19.md + // §M6); likewise one oc:id buffer (benches/ROUND27.md §H1). let mut href = String::new(); + let mut oc_buf = String::new(); for sf in batch.iter() { let dead = dead_props_for(&sf.id, &sub_deads); // Collections carry the trailing slash; prefix @@ -1686,8 +1696,14 @@ fn build_nc_streaming_propfind( href.push_str(&urlencoding::encode(&sf.name)); href.push('/'); let fid = nc_id_of(&sub_id_map, &sf.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); - write_folder_response(&mut xml, sf, &href, (fid, oc_id.as_deref()), &username, &favs, quota, dead) + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; + write_folder_response(&mut xml, sf, &href, (fid, oc_id), &username, &favs, quota, dead) .map_err(std::io::Error::other)?; } } @@ -2062,6 +2078,17 @@ pub fn format_oc_id(id: i64, svc: Option<&Arc>) -> Strin } } +/// Write `oc:id` (`{:08}{instance_id}`) into a caller-provided buffer reused +/// across a PROPFIND/REPORT page — the 0-alloc form of [`format_oc_id`] for the +/// emit loops, replacing a fresh `String` per child (benches/ROUND27.md §H1). +/// Output is byte-identical to `format_oc_id`. +pub fn format_oc_id_into(out: &mut String, id: i64, svc: Option<&Arc>) { + use std::fmt::Write as _; + out.clear(); + let _ = write!(out, "{id:08}"); + out.push_str(svc.map(|s| s.instance_id()).unwrap_or("ocnca")); +} + #[cfg(test)] mod tests { use super::*;