Files
Oxicloud/benches/ROUND21.md
T
Claude 77f13ac643 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 &quot; 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
2026-07-20 08:48:42 +00:00

226 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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 `"` → `&quot;`, 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 `&quot;` 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 `&quot;`, 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 `&quot;` 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.