perf: round 21 — CalDAV/CardDAV row-mapper pre-size, dedup hash-bind & digest-key dedup, CardDAV etag/BDAY emit, NC trashbin content-type
Round 21 of the benchmark-gated perf sweep. Six behaviour-preserving, allocation-reducing changes, each with a BEFORE/AFTER counting-allocator section in examples/bench_round21_micro.rs and a byte/-value equivalence gate; all six pass their deterministic alloc gate (a non-winning AFTER exits 1 = rollback). - R1: pre-size the 16 CalDAV/CardDAV row-mapper Vecs (+1 HashMap) with Vec::with_capacity(rows.len()) — the ROUND20 §I1 file-side pattern extended to the calendar/contact repos it deferred. 7 → 1 allocs/op. - R2: settle_batch binds a borrowed Vec<&str> instead of cloning every chunk hash into a Vec<String> (sqlx encodes &[&str] as text[] identically; favorites_pg_repository.rs:271 precedent). 33 → 1 allocs/op, 39x wall. - R3: store_loose_chunks keys its intra-request dedup set on the raw [u8;32] BLAKE3 digest and moves the hex on a duplicate (the ROUND17 §D2 pattern applied to the delta-upload sibling). 401 → 209 allocs/op. - R4: CardDAV getetag emits borrowed pre-escaped " quotes via a shared write_quoted_etag helper (ROUND20 §C1 pattern, all 4 CardDAV etag sites). 3 → 0 allocs/op. - R5: BDAY stamped via the new fmt::compact_date stack renderer instead of chrono's strftime interpreter (chrono fallback out of the 4-digit-year range; byte-identical, unit-tested vs chrono). 2 → 0 allocs/op, 10.5x wall. - R6: NC trashbin folder content-type via Cow::Borrowed instead of .to_string() on the constant (ROUND16 §M1 pattern). 1 → 0 allocs/op. See benches/ROUND21.md for the full write-up and the deferred-items list (HeaderMap-clone hot handlers, Query→typed-struct, WebDAV dead-props HashSet, and others surfaced by the audit that want their own validated pass). Validated: cargo fmt, cargo clippy --features bench --all-targets -D warnings, cargo test --lib --features test_utils (529 passed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gHVq5Wy2TzdWeSqtEmK6m
This commit is contained in:
Claude
@@ -0,0 +1,225 @@
|
||||
# Round 21 — CalDAV/CardDAV row-mapper pre-size, dedup hash-bind & digest-key dedup, CardDAV etag & BDAY emit, NC trashbin content-type
|
||||
|
||||
Benchmark-gated, same rule as ROUND2–20: every change ships with a BEFORE/AFTER
|
||||
benchmark and a byte/-value equivalence gate; an AFTER that doesn't beat its
|
||||
BEFORE is rolled back (never applied). The roll-back rule is encoded directly in
|
||||
the harness — a `GATE FAIL … rollback` non-zero exit if an AFTER arm fails to
|
||||
reduce allocations — so a regression fails CI rather than shipping.
|
||||
|
||||
This round drains the sibling seams the earlier passes explicitly deferred. The
|
||||
file-listing repositories got their result-`Vec` pre-sizing in ROUND20 §I1, but
|
||||
the **CalDAV/CardDAV row mappers** (bulk address-book / calendar sync builds
|
||||
thousands of rows) were left growing from capacity 0. The **streaming ingest**
|
||||
loop got its `[u8; 32]`-digest dedup key in ROUND17 §D2, but its **delta-upload
|
||||
sibling** `store_loose_chunks` kept a `HashSet<String>` and a double hex clone
|
||||
per frame. The **NextCloud** etag emitter got the borrowed-pre-escaped-quote
|
||||
treatment in ROUND20 §C1, but the **CardDAV** emitter still built a quoted
|
||||
`String`. And two dedup/DAV emit micro-cuts the earlier rounds named but held
|
||||
back: the `settle_batch` clone-to-bind and the `BDAY` strftime stamp.
|
||||
|
||||
Reproduce:
|
||||
|
||||
```
|
||||
cargo run --release --features bench --example bench_round21_micro
|
||||
```
|
||||
|
||||
All arms are **no-Postgres** (release-profile counting-allocator example).
|
||||
|
||||
## Summary
|
||||
|
||||
| # | change | key metric | before → after |
|
||||
|--:|---|---|---|
|
||||
| **R1** | The CalDAV/CardDAV row-mapping repositories (`calendar_event_pg_repository`, `calendar_pg_repository`, `contact_pg_repository`, `contact_group_pg_repository`) built their result `Vec` with `let mut v = Vec::new(); for row in rows { v.push(map(row)?) }` — growing from capacity 0 (~⌈log₂N⌉ reallocations, each memcpy-ing the accumulated rows) on **every CalDAV/CardDAV listing, multiget & bulk sync**. Now `Vec::with_capacity(rows.len())` (the ROUND20 §I1 file-side pattern extended to the 16 calendar/contact sites it deferred). Plus one `HashMap` (`get_calendar_properties`). | 200-row listing | **7 → 1 allocs/op** (6 fewer) |
|
||||
| **R2** | `DedupService::settle_batch` cloned every 64-char chunk hash into a `Vec<String>` purely to `.bind()` it to the pin `UPDATE … WHERE hash = ANY($1)`, on **every settle batch of every upload** (~128 batches for a 1 GB fully-unique upload). Now binds a borrowed `Vec<&str>` — sqlx encodes `&[&str]` to `text[]` identically (`favorites_pg_repository.rs:271` already does this). | 32-chunk batch | **33 → 1 allocs/op · 39.4× wall** |
|
||||
| **R3** | `DedupService::store_loose_chunks` — the delta-upload sibling of the ROUND17 §D2 ingest loop — kept an intra-request dedup `HashSet<String>` and cloned the hex hash **twice per frame** (into `received` and into the set; the set clone dropped on the spot for a duplicate). Now keys the set on the raw `[u8; 32]` BLAKE3 digest (`Copy`, no per-distinct-chunk heap key) and moves the hex into `received` on a duplicate. Runs **per frame** on delta/sync uploads (thousands of frames for a large changed file). | 128 frames, 50% dup | **401 → 209 allocs/op (192 fewer) · 1.50× wall** |
|
||||
| **R4** | `carddav_adapter::write_contact_response` built a `"…"`-quoted `String` for `getetag` then wrote it auto-escaped — `quick_xml` escapes the `"` → `"`, re-allocating an owned `Cow` — on **every contact of every CardDAV multiget/PROPFIND** (plus the per-address-book collection etag). Now emits the two quotes as borrowed pre-escaped `"` text events (the ROUND20 §C1 NextCloud pattern, via a shared `write_quoted_etag` helper covering all 4 CardDAV etag sites). | per-contact row | **3 → 0 allocs/op · 2.11× wall** |
|
||||
| **R5** | `contact_to_vcard` stamped `BDAY` via `write!(…, "{}", bday.format("%Y-%m-%d"))`, running chrono's strftime interpreter per **contact-with-birthday**. Now renders the fixed `YYYY-MM-DD` on the stack via the new `fmt::compact_date` (the date-only companion to the §V2 `REV` renderer), chrono fallback for out-of-range years. | per bday contact | **2 → 0 allocs/op · 10.51× wall** |
|
||||
| **R6** | The NextCloud trashbin PROPFIND row set `d:getcontenttype` for a folder to `"httpd/unix-directory".to_string()` — a heap `String` for a static constant, **per trashed folder row**. Now `Cow::Borrowed` (the ROUND16 §M1 `Cow<'static, str>` pattern); only the file branch (mime_guess) still owns its String. | per folder row | **1 → 0 allocs/op · 5.76× wall** |
|
||||
|
||||
> Allocs/op is the deterministic primary gate (identical run to run). Wall
|
||||
> figures are single-shot and noise-bounded. Every section carries a
|
||||
> byte/-value equivalence gate; the shipped source now matches each AFTER arm.
|
||||
|
||||
## [R1] CalDAV/CardDAV row-mapper container pre-size
|
||||
|
||||
`collect::<Result<Vec>>()` was ROUND20 §I1's target on the file side; the
|
||||
CalDAV/CardDAV repos use the equivalent `Vec::new()` + `for row in rows { … }`
|
||||
shape, which grows the container the same way — from capacity 0, reserving
|
||||
nothing, so `push` reallocates ~⌈log₂N⌉ times and memcpy-s the accumulated
|
||||
(Contact/Event-sized) rows on each grow. `rows` is a materialized `fetch_all`
|
||||
result, so `rows.len()` is exact:
|
||||
|
||||
```rust
|
||||
let mut events = Vec::with_capacity(rows.len());
|
||||
for row in rows {
|
||||
events.push(Self::row_to_event(row)?); // ? short-circuits identically
|
||||
}
|
||||
```
|
||||
|
||||
Applied to the 16 listing/multiget/paginated mappers across the four repos
|
||||
(`calendar_event` ×6, `calendar` ×2 + the `get_calendar_properties` HashMap,
|
||||
`contact` ×6, `contact_group` ×1). The `subject_group` and
|
||||
`nextcloud_object_id` sibling mappers already pre-sized (`with_capacity(rows.len())`),
|
||||
so they were left untouched. Byte-identical output; on a 200-row listing the
|
||||
container allocations drop from **7 → 1** (the growth-from-0 reallocations
|
||||
replaced by a single exact reserve).
|
||||
|
||||
## [R2] settle_batch — bind borrowed `&str`, don't clone
|
||||
|
||||
`settle_batch` runs once per flushed chunk batch of every upload. It built an
|
||||
owned `Vec<String>` of the batch's 64-char hashes only to `.bind()` it:
|
||||
|
||||
```rust
|
||||
let hashes: Vec<String> = batch.iter().map(|(h, _)| h.clone()).collect(); // N heap Strings
|
||||
// … .bind(&hashes) … WHERE hash = ANY($1) …
|
||||
```
|
||||
|
||||
`batch` outlives the query (it is consumed two statements later), so the hashes
|
||||
can be borrowed. sqlx encodes `&[&str]` to a PostgreSQL `text[]` identically to
|
||||
the owned `Vec<String>` (the pattern `favorites_pg_repository.rs:271` already
|
||||
uses, with the comment *"sqlx binds `&[&str]` as text[], so no per-id String is
|
||||
needed"*). The borrow is scoped in a block so it ends before `batch` is moved:
|
||||
|
||||
```rust
|
||||
let pinned: HashSet<String> = {
|
||||
let hashes: Vec<&str> = batch.iter().map(|(h, _)| h.as_str()).collect();
|
||||
sqlx::query_scalar::<_, String>("UPDATE … WHERE hash = ANY($1) RETURNING hash")
|
||||
.bind(&hashes).fetch_all(pool.as_ref()).await?.into_iter().collect()
|
||||
};
|
||||
```
|
||||
|
||||
Up to `FLUSH_MAX_CHUNKS` (=32) 64-byte `String` allocations removed per batch —
|
||||
~4000 over a 1 GB fully-unique upload — for one pointer-only `Vec`.
|
||||
|
||||
## [R3] store_loose_chunks — digest-keyed dedup set + move-on-duplicate
|
||||
|
||||
The delta-upload ingest (`store_loose_chunks`) is the sibling ROUND17 §D2 didn't
|
||||
reach. Per frame it allocated the 64-char hex hash and then cloned it twice:
|
||||
|
||||
```rust
|
||||
let hash = blake3::hash(&data).to_hex().to_string();
|
||||
received.push((hash.clone(), data.len() as u64)); // clone 1 (always)
|
||||
if seen.insert(hash.clone()) { // clone 2 (always; HashSet<String>)
|
||||
new_rows.push((hash, len));
|
||||
}
|
||||
```
|
||||
|
||||
`seen` is the **intra-request** dedup set (has this exact chunk already appeared
|
||||
in *this* delta stream? — re-chunked near-duplicates, zero-padded regions). Keyed
|
||||
on the raw 32-byte digest it needs no per-distinct-chunk `String`, and a
|
||||
duplicate frame **moves** the hex into `received` instead of cloning:
|
||||
|
||||
```rust
|
||||
let digest = blake3::hash(&data);
|
||||
let hash = digest.to_hex().to_string();
|
||||
let len = data.len();
|
||||
if seen.insert(*digest.as_bytes()) { // HashSet<[u8; 32]>, Copy key
|
||||
self.backend.put_blob_from_bytes_unsynced(&hash, data).await?;
|
||||
received.push((hash.clone(), len as u64));
|
||||
new_rows.push((hash, len as i64));
|
||||
} else {
|
||||
received.push((hash, len as u64)); // move, no clone
|
||||
}
|
||||
```
|
||||
|
||||
hex ↔ digest is bijective, so membership and the `received`/`new_rows`
|
||||
sequences are identical. On a 128-frame stream with 50 % intra-request dups the
|
||||
per-frame hash clones drop from 3 to ~1.5.
|
||||
|
||||
## [R4] CardDAV getetag — borrowed pre-escaped quotes
|
||||
|
||||
`write_contact_response` (per contact of every CardDAV multiget/PROPFIND) built
|
||||
a `"…"`-quoted `String` and wrote it auto-escaped; `quick_xml` escapes the `"`
|
||||
to `"`, so the whole-string escape re-allocated an owned `Cow`. The new
|
||||
shared `write_quoted_etag` helper emits the two quotes as **borrowed**
|
||||
pre-escaped `"` text events around the escaped etag body — byte-identical
|
||||
(the equivalence gate asserts it, including an etag with `&`/`<`/`"`), 0
|
||||
allocs/contact. Applied to all four CardDAV etag sites (2 per-contact + 2
|
||||
per-address-book collection), mirroring the NextCloud ROUND20 §C1 fix.
|
||||
|
||||
## [R5] BDAY — stack-rendered `%Y-%m-%d`
|
||||
|
||||
`contact_to_vcard` already stack-renders `REV` (ROUND19 §V2); `BDAY` still went
|
||||
through chrono's strftime interpreter (`bday.format("%Y-%m-%d")`). The new
|
||||
`fmt::compact_date(buf, year, month, day)` renders the fixed 10-byte
|
||||
`YYYY-MM-DD` with the same `push4`/`push2` LUT the other `fmt` helpers use, and
|
||||
returns `None` outside the 4-digit-year range (where chrono widens/sign-prefixes
|
||||
`%Y`) so the caller keeps the chrono path as fallback. Byte-identical for every
|
||||
representable birthday.
|
||||
|
||||
## [R6] NC trashbin folder content-type — borrowed constant
|
||||
|
||||
The trashbin PROPFIND folder branch `to_string()`-ed the static
|
||||
`"httpd/unix-directory"` per row. `Cow::Borrowed` for the folder constant (the
|
||||
file branch still owns its mime_guess String) drops that allocation per trashed
|
||||
folder row — the ROUND16 §M1 `Cow<'static, str>` pattern the trashbin loop
|
||||
missed.
|
||||
|
||||
## Not shipped — deferred to a later round
|
||||
|
||||
Surfaced by the Round-21 audit (three parallel sub-audits across the HTTP,
|
||||
storage/dedup and application/parse layers), verified against current source,
|
||||
but held back — each needs a signature/API decision, a Postgres fixture, or a
|
||||
gate the deterministic alloc-counter can't provide:
|
||||
|
||||
- **Hot GET handlers clone the whole request `HeaderMap`** (`file_handler`
|
||||
list/download/thumbnail, `photos_handler`, NC `preview`/`avatar`): axum's
|
||||
`HeaderMap` extractor does `parts.headers.clone()` (~2 allocs) purely to read
|
||||
1–3 headers — the exact cost `middleware/auth.rs` already eliminated (ROUND14
|
||||
§A4) but never propagated to the handlers. The fix takes `req: Request` last
|
||||
and reads `req.headers()` by borrow; it's a **multi-handler signature refactor**
|
||||
(each `_impl` + its wrapper + the route registration) that wants its own
|
||||
validated pass, same class as the ROUND19/20 multi-signature deferrals.
|
||||
- **`Query<HashMap<String,String>>` on the hot list/download paths** builds a
|
||||
`HashMap` + key `String` per request to read one param; a typed
|
||||
`Query<ListFilesQuery>` struct drops both (serde ignores unknown params). Same
|
||||
signature-surface reason as above; pairs naturally with the HeaderMap pass.
|
||||
- **Native WebDAV PROPFIND re-extracts the URI path** (`webdav_handler.rs:507`):
|
||||
`extract_webdav_path(req.uri())` re-runs a percent-decode + `String` alloc that
|
||||
the `path` parameter already holds at that point (the `:503` comment about the
|
||||
prefix is stale). One decode + alloc per PROPFIND — but removing it needs a
|
||||
careful href-equivalence proof across the chroot/scope resolution, so it wants
|
||||
a dedicated correctness check, not a perf banner.
|
||||
- **`music_service` public-playlist merge is O(owned·public)** (`Vec::any()` per
|
||||
public item): a `HashSet` makes it O(owned+public). Because `PlaylistDto.id` is
|
||||
a `String`, the set must own the ids (clone) — so the change trades N String
|
||||
comparisons for N String clones: a **wall win that ADDS allocations**, which
|
||||
the deterministic alloc gate can't score. Wants a wall-gated evaluation on the
|
||||
opt-in `include_public` path.
|
||||
- **WebDAV dead-props filter is O(N·D·R)** (`webdav_adapter.rs:616/705`): the
|
||||
loop-invariant requested-props list is re-scanned per dead prop per resource;
|
||||
a per-PROPFIND `HashSet<&QualifiedName>` makes it O(N·D). Only bites accounts
|
||||
that accumulate client-set custom props (macOS Finder) over large listings —
|
||||
and, like music_service, the HashSet build trades compares for an alloc, so
|
||||
it's wall-gated. Queued with a synthetic-dead-props bench.
|
||||
- **`verify_integrity` Phase 1 probes manifest chunks serially** while Phase 2
|
||||
is `buffer_unordered(16)` — on a remote backend that's O(total_chunks) serial
|
||||
HEADs. Background/admin path; needs a remote-backend fixture to show the win.
|
||||
- **`subject_group_service::remove_member` runs the same recursive-CTE
|
||||
`list_transitive_users(child_id)` twice** for a nested group removal (the
|
||||
intervening edge delete can't change the child's descendants). One DB
|
||||
round-trip halved; low frequency (admin), needs Postgres.
|
||||
- **`store_loose_chunks` final registration + `run_rollback` clone hashes to
|
||||
bind** (`dedup_service.rs:887/212`), and the **`contact_pg` JSONB columns
|
||||
decode through a throwaway `serde_json::Value`** — the R2/ROUND20 patterns
|
||||
applied to once-per-upload / per-contact-read sites; both need Postgres to
|
||||
bench end-to-end.
|
||||
- **`GzipCompressionService::{compress,decompress}_data` copy the whole buffer
|
||||
via `.to_vec()`** before `spawn_blocking` — forced by the `&[u8]` port
|
||||
signature; a `Bytes`-taking port lets an owning caller move. Port API change,
|
||||
gated (text > 50 KB), low heat.
|
||||
- **Fast hasher for trusted-key internal maps** (ROUND20 flag stands): needs a
|
||||
`Cargo.toml` dependency decision and must stay DoS-resistant for the
|
||||
attacker-controlled delta-hash sets — worth a dedicated, wall-gated pass.
|
||||
|
||||
## Environment / methodology
|
||||
|
||||
- `cargo run --release --features bench --example bench_round21_micro` —
|
||||
counting global allocator, no Postgres. Tunables (env): `BENCH_ITERS` (200000),
|
||||
`R1_ROWS` (200), `R3_FRAMES` (128).
|
||||
- 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; the shipped source now matches
|
||||
each AFTER arm.
|
||||
- Roll-back rule encoded per section: the harness `std::process::exit(1)`s with
|
||||
`GATE FAIL … rollback` if an AFTER arm fails to reduce allocations.
|
||||
Reference in New Issue
Block a user