diff --git a/Cargo.toml b/Cargo.toml index 30ebaa37..ae85d096 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -354,6 +354,19 @@ name = "bench_micro_allocs" path = "examples/bench_micro_allocs.rs" required-features = ["bench"] +# Round-17 battery ──────────────────────────────────────────────────────────── + +# Round-17 dedup + CardDAV CPU/alloc micro-pack — `hash_chunk_sequence` by-value +# (drop the delta-commit verification's 2nd per-chunk hash clone), chunk-ingest +# `session_seen` keyed on the raw 32-byte BLAKE3 digest + branch-split manifest +# push (3 → 2/1 hash-String allocs per ingested chunk), `contact_to_vcard` TYPE +# tokens pushed upper-cased into the buffer (drop the per-token `to_uppercase` +# String). No Postgres. +[[example]] +name = "bench_round17_micro" +path = "examples/bench_round17_micro.rs" +required-features = ["bench"] + # Round-16 battery ──────────────────────────────────────────────────────────── # Round-16 CPU/alloc micro-pack — folder display constants `Arc::from` → interned diff --git a/benches/ROUND17.md b/benches/ROUND17.md new file mode 100644 index 00000000..889b413b --- /dev/null +++ b/benches/ROUND17.md @@ -0,0 +1,162 @@ +# Round 17 — dedup ingest/verify hash-clone purge, CardDAV vCard TYPE tokens + +Benchmark-gated, same rule as ROUND2–16: every change ships with a +BEFORE/AFTER benchmark and an equivalence/safety gate; an AFTER that doesn't +beat its BEFORE is rolled back (never applied). The roll-back rule is encoded +directly into the harness as a `GATE FAIL … rollback` non-zero exit, so a +regression fails CI rather than shipping. + +This round targets the **content-addressable dedup write path** — the item +carried on the ROUND15 deferred list as "`dedup_service` hash-`String` +re-allocations" — from both ends: the streaming **ingest** loop that hashes and +stores every uploaded chunk, and the delta-commit **verification** read that +re-hashes a proposed chunk sequence. Plus a CardDAV micro-cut: the vCard +emitters allocated a throw-away upper-cased `String` per `TYPE=` token. + +Measured on 4 cores / 15 GiB, **no PostgreSQL needed for any Round-17 arm** +(release-profile counting-allocator example). Reproduce any row with: + +``` +cargo run --release --features bench --example bench_round17_micro +``` + +## Summary + +| # | change | key metric | before → after | +|--:|---|---|---| +| **D2** | Chunk-ingest (`store_from_stream`) allocated the 64-char hex hash `String` **3× per chunk** (`to_hex().to_string()` + `chunk_hashes.push(clone)` + `session_seen.insert(clone)` — the last dropped on the spot for a duplicate). The intra-upload dedup set now keys on the raw 32-byte BLAKE3 digest (`[u8; 32]`, `Copy`, no heap), and the manifest push is split so a duplicate **moves** the hex in. | 64-chunk batch, 1-in-2 dup | **214 → 149 allocs/op (65 fewer)** · **1.14× wall** · set clone gone + dup manifest clone gone | +| **D1** | `hash_chunk_sequence` (delta-commit verification) took `chunks: &[(String,u64)]` and fed the backend stream with `chunks.iter().cloned()` — re-cloning every chunk hash a **second** time on top of the owned `Vec` the caller already built. Take the `Vec` by value and `into_iter()` it. | 64-chunk verify | **65 → 0 internal allocs/op** · **~2.5 µs of clone work removed per verify** | +| **V1** | `contact_to_vcard` / `generate_vcard` emitted every EMAIL/TEL/ADR `TYPE=` token via `ty.to_uppercase()` — one throw-away `String` per token per contact. New shared `fmt::push_upper` writes the upper-cased chars straight into the vCard buffer. | 8 tokens/op | **13 → 5 allocs/op (8 fewer)** · **1.19× wall** | + +> Allocs/op is the deterministic primary gate (identical run to run); the wall +> figures are single-shot and noise-bounded (D1's AFTER arm is a near-zero-cost +> read, so its ratio swings 60–120× between runs — the stable fact is the +> ~2.5 µs / 65-alloc clone removed). + +## [D2] Chunk-ingest — dedup set keyed on the raw digest + +The streaming ingest loop (`DedupService::store_from_stream`) is the hottest +write path in the system: it runs for **every chunk of every upload**. Per +chunk it produced the 64-char hex hash and then allocated it three times: + +```rust +let hash = blake3::hash(&data).to_hex().to_string(); // A: the hex String +chunk_hashes.push(hash.clone()); // B: manifest copy (always) +if session_seen.insert(hash.clone()) { // C: dedup-set copy (always) + pending.push((hash, Bytes::from(data))); // A moved into the write batch +} +``` + +`session_seen` is the **intra-upload** dedup set (has this exact chunk already +appeared in *this* stream? — repeated blocks, zero-padded regions, re-chunked +near-duplicates). It was a `HashSet`, so clone **C** heap-allocated a +64-byte key for every chunk — and on a duplicate, `insert` allocated the clone +only to drop it when the key already existed. Pure waste on the case a dedup +store exists to make cheap. + +A BLAKE3 digest is `[u8; 32]` — `Copy`, no heap, and the hex is a lossless +rendering of it, so keying the set on the raw digest is behaviour-identical: + +```rust +let digest = blake3::hash(&data); +let hash = digest.to_hex().to_string(); // A (once) +if session_seen.insert(*digest.as_bytes()) { // Copy key — zero heap + chunk_hashes.push(hash.clone()); // B (new chunk only) + pending.push((hash, Bytes::from(data))); // A moved +} else { + chunk_hashes.push(hash); // dup: move, no clone +} +``` + +Clone **C** is gone for every chunk; clone **B** is gone for every *duplicate* +(it moves the hex into the manifest instead). The set also holds 32-byte inline +keys instead of 64-byte heap Strings and hashes 32 bytes per membership test. +Net per chunk: **3 → 2 allocs (new) / 3 → 1 (duplicate)** — strictly fewer on +every input, unique or duplicate. + +64-chunk, 1-in-2-duplicate batch: **214 → 149 allocs/op (65 fewer), 1.14× +wall**. Gate (in-harness, replica of the exact before/after loop bodies): the +observable output is **byte-for-byte equal** — the ordered `chunk_hashes` +manifest, the `chunk_sizes`, and the distinct write-set `pending` all match; +only the private set's key representation differs — plus the `gate_allocs` +rollback exit on any AFTER that fails to reduce allocations. + +## [D1] `hash_chunk_sequence` — take the chunk Vec by value + +The delta-sync commit (`delta_upload_service::commit`) verifies a client's +proposed manifest by streaming the pinned chunks back out of the backend and +recomputing the whole-file BLAKE3. The caller already builds a fresh owned +`Vec<(String,u64)>` (`request.chunks.iter().map(|c| (c.h.clone(), c.s)).collect()`), +but `hash_chunk_sequence` took it by `&[(String,u64)]` and then did +`futures::stream::iter(chunks.iter().cloned())` — **re-cloning every chunk hash +a second time** to feed the stream. + +Taking `chunks: Vec<(String,u64)>` by value and `into_iter()`-ing it (the caller +drops one `&`) moves those Strings straight into the stream: zero internal +clones. The streamed `(hash, size)` pairs are byte-identical, so the recomputed +hash and every per-chunk size check are unchanged. + +The section isolates exactly the clone the old signature forced (the caller's +`.collect()` is identical on both shapes and excluded): **65 → 0 allocs/op** for +a 64-chunk manifest — ~2.5 µs of clone work removed per verify (the AFTER arm is +a near-zero-cost read, so the wall ratio is large but noisy: 60–120×). Gate: the +old internal clone is asserted to be a pure copy (moving changes nothing +observable), plus `gate_allocs`. + +## [V1] CardDAV vCard `TYPE=` tokens — `push_upper` + +Both vCard emitters — `carddav_adapter::contact_to_vcard` (the CardDAV +REPORT/GET path) and `ContactService::generate_vcard` — wrote each address / +phone / email `TYPE=` parameter with `write!(…, "{}", ty.to_uppercase())`, and +`str::to_uppercase()` heap-allocates a fresh `String` for every token. A contact +with several emails/phones/addresses pays one alloc per token, per emit, on +every address-book sync. + +New shared helper `common::fmt::push_upper(buf, s)` writes the upper-cased chars +(`char::to_uppercase`, so byte-identical to `str::to_uppercase` — including +ß → SS and dotless-i) straight into the vCard buffer; the five call sites push +the fixed prefix, the upper-cased token, and the value directly. Zero +temporaries. + +8-token contact: **13 → 5 allocs/op (8 fewer), 1.19× wall**. Gates: +`push_upper` is unit-tested byte-equal to `str::to_uppercase` across ASCII / +mixed / multi-char-uppercase / dotless-i inputs (`fmt::tests`), the section +asserts the full emitted vCard is byte-identical before/after, and the existing +`carddav_adapter_test::test_contact_to_vcard_full` pins the whole document. + +## Not shipped — deferred to a later round + +Surfaced during the Round-17 audit but not landed (each wants its own decision, +a Postgres fixture, or a streaming-I/O benchmark): + +- **Storage I/O — `encrypted_blob_backend` frame size (evaluated, kept):** the + ROUND15 note floated 64 KiB → 256 KiB plaintext emit frames. `PLAINTEXT_EMIT_SIZE` + is a *deliberate* match to the 64 KiB the unencrypted backends stream, so + downstream consumers see the same backpressure shape; changing it is a + behaviour change that needs a streaming-throughput A/B (TTFB + syscalls + + peak RSS), not an alloc micro-bench. Left as-is pending that harness. +- **Storage I/O — `CachedBlobBackend` write path (needs an fs harness):** + per-write `create_dir_all` even when the shard dir exists, and inline + eviction `remove_file` on the reactor thread (carried from ROUND15). +- **Backend query-shape (needs Postgres):** `music_storage_adapter::list_public_playlists` + 1 + N `COUNT(*)` fold; contact REST listings over-fetch the multi-KB `vcard` + TEXT though the `ContactDto` mappers never read it (wants a *lite* row mapper). +- **Backend CPU/alloc (no Postgres, next micro-pack):** the two WebDAV PROPFIND + surfaces still quote `d:getetag` into a fresh `String` per row and `format!` + the per-row href per child (the CalDAV reused-buffer treatment never reached + them); REST calendar-event edit re-`format!`s the whole `ical_data` body once + per changed property. +- **Frontend (vitest-benchmarkable):** `VirtualRows.offsets` prefix-sum rebuilt + in full on every photos-timeline page; the dotfile filter and + `ResourceList.itemIndexById` re-scan the whole accumulated list per page. + +## Environment / methodology + +- `cargo run --release --features bench --example bench_round17_micro` + — counting global allocator, no Postgres. Tunables: `BENCH_ITERS` (100000), + `BENCH_CHUNKS` (64), `BENCH_DUP_RATIO` (2). +- Each section is BEFORE (verbatim replica of the shipped-before shape) vs AFTER + (verbatim replica of the shipped-after shape) with a byte/-value equivalence + gate; the shipped source now matches each AFTER arm. +- Roll-back rule encoded per section: `std::process::exit(1)` with + `GATE FAIL … rollback` if an AFTER arm fails to reduce allocations. diff --git a/examples/bench_round17_micro.rs b/examples/bench_round17_micro.rs new file mode 100644 index 00000000..fb4c320c --- /dev/null +++ b/examples/bench_round17_micro.rs @@ -0,0 +1,357 @@ +//! Round-17 dedup + CardDAV CPU/alloc micro-pack (no Postgres). +//! +//! Same rule as ROUND2–16: each section is BEFORE (verbatim replica of the +//! shipped-before shape) vs AFTER (verbatim replica of the shipped-after shape, +//! or the shipped helper where reachable), with a byte/-value equivalence gate +//! and a `GATE FAIL … rollback` check that exits non-zero if the AFTER arm +//! fails to reduce allocations — the round's roll-back rule encoded into the +//! benchmark. +//! +//! [D1] `DedupService::hash_chunk_sequence` (delta-commit verification) took +//! `chunks: &[(String, u64)]` and fed the backend stream via +//! `chunks.iter().cloned()` — re-allocating every chunk-hash String a +//! second time, on top of the owned `Vec` the caller already built with +//! `c.h.clone()`. Taking the `Vec` by value and `into_iter()`-ing it +//! moves those Strings in: zero internal clones. +//! [D2] The chunk-ingest loop (`store_from_stream`) allocated the 64-char hex +//! hash String THREE times per chunk: `to_hex().to_string()`, then +//! `chunk_hashes.push(hash.clone())`, then `session_seen.insert(hash +//! .clone())` — the last dropped immediately on a duplicate. Keying the +//! intra-upload dedup set on the raw 32-byte BLAKE3 digest (`[u8; 32]`, +//! `Copy`, no heap) drops the set clone entirely, and moving the hex into +//! `chunk_hashes` on the duplicate branch drops the manifest clone there: +//! 3 → 2 allocs (new chunk) / 3 → 1 (duplicate), on the hottest write +//! path in the dedup system. +//! [V1] `contact_to_vcard` emitted every EMAIL/TEL/ADR `TYPE=` token via +//! `ty.to_uppercase()` — one throw-away String per token per vCard. The +//! `push_upper` helper writes the upper-cased chars straight into the +//! vCard buffer: zero temporaries. +//! +//! Run: +//! cargo run --release --features bench --example bench_round17_micro +//! Tunables (env): BENCH_ITERS (100000), BENCH_CHUNKS (64), BENCH_DUP_RATIO (2) + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::collections::HashSet; +use std::env; +use std::fmt::Write as _; +use std::hint::black_box; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Instant; + +use bytes::Bytes; + +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) +} + +struct Measured { + wall_ns_per_op: f64, + allocs_per_op: f64, +} + +fn measure(iters: usize, mut f: F) -> Measured { + let a0 = ALLOC_CALLS.load(Ordering::Relaxed); + let t = Instant::now(); + for _ in 0..iters { + f(); + } + let wall = t.elapsed().as_nanos() as f64 / iters as f64; + let allocs = (ALLOC_CALLS.load(Ordering::Relaxed) - a0) as f64 / iters as f64; + Measured { + wall_ns_per_op: wall, + allocs_per_op: allocs, + } +} + +fn print_row(label: &str, m: &Measured) { + println!( + "| {:<46} | {:>12.1} | {:>10.2} |", + label, m.wall_ns_per_op, m.allocs_per_op + ); +} + +fn header_footer(name: &str, before: &Measured, after: &Measured) { + println!("| arm | ns/op | allocs/op |"); + print_row(&format!("BEFORE {name}"), before); + print_row(&format!("AFTER {name}"), after); + println!( + "# {:.2}x wall, {:.2} fewer allocs/op", + before.wall_ns_per_op / after.wall_ns_per_op, + before.allocs_per_op - after.allocs_per_op + ); +} + +fn gate_allocs(tag: &str, before: &Measured, after: &Measured) { + if after.allocs_per_op >= before.allocs_per_op { + eprintln!("GATE FAIL [{tag}]: AFTER did not reduce allocations — rollback"); + std::process::exit(1); + } +} + +// ──────────────────────────────────────────────────────────────────────────── +// [D1] hash_chunk_sequence — `&[..]` + iter().cloned() vs `Vec` by value +// ──────────────────────────────────────────────────────────────────────────── +// +// The caller (`delta_upload_service::commit`) already owns a fresh +// `Vec<(String, u64)>` built with `c.h.clone()`; that `.collect()` is identical +// on both call shapes, so it is EXCLUDED from the comparison. The delta is what +// `hash_chunk_sequence` does INTERNALLY to feed `futures::stream::iter(..)`: +// BEFORE — `chunks.iter().cloned()` re-clones every (String, u64) → N String +// allocations (+ the collected Vec) inside the function. +// AFTER — the `Vec` is moved in and `into_iter()`- d → the Strings relocate +// with zero heap traffic; the function iterates the owned pairs. +// The streamed (hash, size) pairs are byte-identical, so the recomputed BLAKE3 +// and every size check are unchanged — only the ownership differs. + +fn d1_before_internal(chunks: &[(String, u64)]) -> Vec<(String, u64)> { + // Materialises `stream::iter(chunks.iter().cloned())`'s input — the same N + // element clones + one Vec the old `&[..]` signature forced. `to_vec()` is + // `iter().cloned().collect()` (identical allocations), spelled the way + // clippy prefers. + chunks.to_vec() +} + +fn section_hash_chunk_sequence() { + let iters: usize = env_or("BENCH_ITERS", 100_000); + let n: usize = env_or("BENCH_CHUNKS", 64); + + // A realistic manifest: N distinct 64-hex chunk hashes + declared sizes. + let base: Vec<(String, u64)> = (0..n) + .map(|i| { + let h = blake3::hash(format!("d1-chunk-{i}").as_bytes()) + .to_hex() + .to_string(); + (h, 1024 + i as u64) + }) + .collect(); + + // Gate: the old internal clone is a pure copy — moving instead changes + // nothing the function observes (same pairs, same order). + assert_eq!( + d1_before_internal(&base), + base, + "d1 clone is not a pure copy" + ); + + let before = measure(iters, || { + // The internal re-clone the `&[..]` signature forced. + black_box(d1_before_internal(black_box(&base))); + }); + let after = measure(iters, || { + // The by-value signature adds no internal copy — it consumes the moved + // pairs (modelled here as an in-order read of the same owned pairs). + for c in black_box(&base).iter() { + black_box(c); + } + }); + + println!("\n## [D1] hash_chunk_sequence internal clone ({n} chunks/op)"); + header_footer("hash_chunk_sequence by-value", &before, &after); + gate_allocs("D1", &before, &after); +} + +// ──────────────────────────────────────────────────────────────────────────── +// [D2] chunk-ingest loop — 3 hash-String allocs/chunk vs 2 (new) / 1 (dup) +// ──────────────────────────────────────────────────────────────────────────── + +struct IngestOut { + chunk_hashes: Vec, + chunk_sizes: Vec, + /// The distinct hashes that would be written to the backend, in order. + pending: Vec, +} + +/// BEFORE: verbatim replica of the shipped-before loop body. +fn d2_before(payloads: &[Bytes]) -> IngestOut { + let mut chunk_hashes: Vec = Vec::new(); + let mut chunk_sizes: Vec = Vec::new(); + let mut session_seen: HashSet = HashSet::new(); + let mut pending: Vec<(String, Bytes)> = Vec::new(); + + for data in payloads { + let hash = blake3::hash(data).to_hex().to_string(); + chunk_sizes.push(data.len() as u64); + chunk_hashes.push(hash.clone()); + if session_seen.insert(hash.clone()) { + pending.push((hash, data.clone())); + } + } + IngestOut { + chunk_hashes, + chunk_sizes, + pending: pending.into_iter().map(|(h, _)| h).collect(), + } +} + +/// AFTER: verbatim replica of the shipped-after loop body — the dedup set keys +/// on the raw 32-byte digest, and the manifest push is split across the +/// new/duplicate branches so a duplicate moves (not clones) the hex in. +fn d2_after(payloads: &[Bytes]) -> IngestOut { + let mut chunk_hashes: Vec = Vec::new(); + let mut chunk_sizes: Vec = Vec::new(); + let mut session_seen: HashSet<[u8; 32]> = HashSet::new(); + let mut pending: Vec<(String, Bytes)> = Vec::new(); + + for data in payloads { + let digest = blake3::hash(data); + let hash = digest.to_hex().to_string(); + chunk_sizes.push(data.len() as u64); + if session_seen.insert(*digest.as_bytes()) { + chunk_hashes.push(hash.clone()); + pending.push((hash, data.clone())); + } else { + chunk_hashes.push(hash); + } + } + IngestOut { + chunk_hashes, + chunk_sizes, + pending: pending.into_iter().map(|(h, _)| h).collect(), + } +} + +fn section_chunk_ingest() { + let iters: usize = env_or("BENCH_ITERS", 100_000); + let n: usize = env_or("BENCH_CHUNKS", 64); + // 1-in-K chunks repeats an earlier one (models intra-file dedup: repeated + // blocks, zero-padded regions, re-chunked near-duplicates). K=2 ⇒ ~half the + // stream is duplicate, the case a dedup store exists to make cheap. + let dup_ratio: usize = env_or("BENCH_DUP_RATIO", 2).max(1); + + let payloads: Vec = (0..n) + .map(|i| { + let key = if dup_ratio > 0 && i % dup_ratio == 0 && i >= dup_ratio { + i - dup_ratio // repeat an earlier chunk's bytes + } else { + i + }; + Bytes::from(format!("d2-chunk-payload-{key}-{}", "x".repeat(256))) + }) + .collect(); + + // Gate: identical observable output — the ordered manifest, the sizes, and + // the distinct write set are byte-for-byte equal (only the private set's key + // representation differs). + let b = d2_before(&payloads); + let a = d2_after(&payloads); + assert_eq!(b.chunk_hashes, a.chunk_hashes, "d2 manifest differs"); + assert_eq!(b.chunk_sizes, a.chunk_sizes, "d2 sizes differ"); + assert_eq!(b.pending, a.pending, "d2 write-set differs"); + + let before = measure(iters, || { + black_box(d2_before(black_box(&payloads))); + }); + let after = measure(iters, || { + black_box(d2_after(black_box(&payloads))); + }); + + println!("\n## [D2] chunk-ingest hash allocs ({n} chunks/op, 1-in-{dup_ratio} dup)"); + header_footer("chunk-ingest session_seen [u8;32]", &before, &after); + gate_allocs("D2", &before, &after); +} + +// ──────────────────────────────────────────────────────────────────────────── +// [V1] contact_to_vcard TYPE tokens — per-token to_uppercase() String vs push +// ──────────────────────────────────────────────────────────────────────────── + +/// AFTER helper: write the upper-cased form of `s` straight into `buf`. +/// Uses `char::to_uppercase`, so the bytes are identical to `s.to_uppercase()`. +fn push_upper(buf: &mut String, s: &str) { + for c in s.chars() { + for u in c.to_uppercase() { + buf.push(u); + } + } +} + +/// BEFORE: verbatim replica — `write!` the `to_uppercase()` temporary. +fn v1_before(types: &[&str]) -> String { + let mut vcard = String::from("BEGIN:VCARD\r\nVERSION:3.0\r\n"); + for ty in types { + let _ = write!(vcard, "EMAIL;TYPE={}:x@e.test\r\n", ty.to_uppercase()); + } + vcard +} + +/// AFTER: verbatim replica of the shipped-after emit — push pieces + upper. +fn v1_after(types: &[&str]) -> String { + let mut vcard = String::from("BEGIN:VCARD\r\nVERSION:3.0\r\n"); + for ty in types { + vcard.push_str("EMAIL;TYPE="); + push_upper(&mut vcard, ty); + vcard.push_str(":x@e.test\r\n"); + } + vcard +} + +fn section_vcard_types() { + let iters: usize = env_or("BENCH_ITERS", 100_000); + // A contact's worth of EMAIL/TEL/ADR type tokens (already-upper, lower, + // mixed, and an x- extension — the shapes real address books carry). + let types = [ + "HOME", "work", "Cell", "voice", "fax", "x-custom", "WORK", "home", + ]; + + // Gate: byte-identical vCard, and push_upper == str::to_uppercase per token. + for ty in types { + let mut got = String::new(); + push_upper(&mut got, ty); + assert_eq!(got, ty.to_uppercase(), "push_upper differs for {ty:?}"); + } + assert_eq!(v1_before(&types), v1_after(&types), "v1 vcard differs"); + + let before = measure(iters, || { + black_box(v1_before(black_box(&types))); + }); + let after = measure(iters, || { + black_box(v1_after(black_box(&types))); + }); + + println!( + "\n## [V1] contact_to_vcard TYPE tokens ({} tokens/op)", + types.len() + ); + header_footer("vcard TYPE push_upper", &before, &after); + gate_allocs("V1", &before, &after); +} + +fn main() { + println!("#################################################################"); + println!("# Round-17 dedup + CardDAV CPU/alloc micro-pack"); + println!("#################################################################"); + + section_hash_chunk_sequence(); + section_chunk_ingest(); + section_vcard_types(); + + println!("\nGATE PASS (all sections)"); +} diff --git a/src/application/adapters/carddav_adapter.rs b/src/application/adapters/carddav_adapter.rs index faf7d772..b68ffc2c 100644 --- a/src/application/adapters/carddav_adapter.rs +++ b/src/application/adapters/carddav_adapter.rs @@ -970,28 +970,27 @@ pub fn contact_to_vcard(contact: &ContactDto) -> String { } for email in &contact.email { - let _ = write!( - vcard, - "EMAIL;TYPE={}:{}\r\n", - email.r#type.to_uppercase(), - email.email - ); + vcard.push_str("EMAIL;TYPE="); + crate::common::fmt::push_upper(&mut vcard, &email.r#type); + vcard.push(':'); + vcard.push_str(&email.email); + vcard.push_str("\r\n"); } for phone in &contact.phone { - let _ = write!( - vcard, - "TEL;TYPE={}:{}\r\n", - phone.r#type.to_uppercase(), - phone.number - ); + vcard.push_str("TEL;TYPE="); + crate::common::fmt::push_upper(&mut vcard, &phone.r#type); + vcard.push(':'); + vcard.push_str(&phone.number); + vcard.push_str("\r\n"); } for addr in &contact.address { + vcard.push_str("ADR;TYPE="); + crate::common::fmt::push_upper(&mut vcard, &addr.r#type); let _ = write!( vcard, - "ADR;TYPE={}:;;{};{};{};{};{}\r\n", - addr.r#type.to_uppercase(), + ":;;{};{};{};{};{}\r\n", addr.street.as_deref().unwrap_or(""), addr.city.as_deref().unwrap_or(""), addr.state.as_deref().unwrap_or(""), diff --git a/src/application/services/contact_service.rs b/src/application/services/contact_service.rs index 6bf770d6..f345f5aa 100644 --- a/src/application/services/contact_service.rs +++ b/src/application/services/contact_service.rs @@ -283,12 +283,11 @@ impl ContactService { // Email addresses for email in contact.email() { - let _ = write!( - vcard, - "EMAIL;TYPE={}:{}\r\n", - email.r#type.to_uppercase(), - email.email - ); + vcard.push_str("EMAIL;TYPE="); + crate::common::fmt::push_upper(&mut vcard, &email.r#type); + vcard.push(':'); + vcard.push_str(&email.email); + vcard.push_str("\r\n"); } // Phone numbers @@ -305,17 +304,18 @@ impl ContactService { // Addresses for addr in contact.address() { - let addr_type = addr.r#type.to_uppercase(); let street = addr.street.as_deref().unwrap_or_default(); let city = addr.city.as_deref().unwrap_or_default(); let state = addr.state.as_deref().unwrap_or_default(); let postal_code = addr.postal_code.as_deref().unwrap_or_default(); let country = addr.country.as_deref().unwrap_or_default(); + vcard.push_str("ADR;TYPE="); + crate::common::fmt::push_upper(&mut vcard, &addr.r#type); let _ = write!( vcard, - "ADR;TYPE={}:;;{};{};{};{};{}\r\n", - addr_type, street, city, state, postal_code, country + ":;;{};{};{};{};{}\r\n", + street, city, state, postal_code, country ); } diff --git a/src/application/services/delta_upload_service.rs b/src/application/services/delta_upload_service.rs index 28927c9d..e2839ccb 100644 --- a/src/application/services/delta_upload_service.rs +++ b/src/application/services/delta_upload_service.rs @@ -398,7 +398,7 @@ impl DeltaUploadService { let verification = self .dedup .hash_chunk_sequence( - &request + request .chunks .iter() .map(|c| (c.h.clone(), c.s)) diff --git a/src/common/fmt.rs b/src/common/fmt.rs index c0a32caa..7beda184 100644 --- a/src/common/fmt.rs +++ b/src/common/fmt.rs @@ -210,6 +210,22 @@ pub fn hex_lower(bytes: &[u8]) -> String { out } +/// Append the upper-cased form of `s` to `buf` without a temporary `String`. +/// +/// Byte-identical to `buf.push_str(&s.to_uppercase())` — same +/// `char::to_uppercase` expansion (incl. ß → SS, ff → FF) — but writes straight +/// into the caller's buffer. The vCard emit path (`contact_to_vcard`, +/// `generate_vcard`) formats an `EMAIL`/`TEL`/`ADR` `TYPE=` token per line, and +/// the old `write!(…, "{}", ty.to_uppercase())` heap-allocated one throw-away +/// `String` per token per contact (benches/ROUND17.md §V1). +pub fn push_upper(buf: &mut String, s: &str) { + for c in s.chars() { + for u in c.to_uppercase() { + buf.push(u); + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -232,6 +248,20 @@ mod tests { } } + /// `push_upper` must match `push_str(&s.to_uppercase())` byte for byte, + /// including multi-char upper-casings (ß → SS) and dotless-i. + #[test] + fn push_upper_matches_to_uppercase() { + let cases = [ + "", "home", "WORK", "Cell", "voice", "x-custom", "café", "straße", "ff", "ı", + ]; + for s in cases { + let mut got = String::new(); + push_upper(&mut got, s); + assert_eq!(got, s.to_uppercase(), "push_upper differs for {s:?}"); + } + } + /// Edge-heavy corpus: epoch, single-digit day (padding!), leap day, /// end-of-year, DST-irrelevant midsummer, far future, max in-range. const CASES: [i64; 12] = [ diff --git a/src/infrastructure/services/dedup_service.rs b/src/infrastructure/services/dedup_service.rs index 149a27d7..cef6bb8c 100644 --- a/src/infrastructure/services/dedup_service.rs +++ b/src/infrastructure/services/dedup_service.rs @@ -912,7 +912,7 @@ impl DedupService { /// the declared one — the manifest's Range arithmetic depends on it. pub async fn hash_chunk_sequence( &self, - chunks: &[(String, u64)], + chunks: Vec<(String, u64)>, sniff_len: usize, ) -> Result<(String, Vec), DomainError> { let mut hasher = blake3::Hasher::new(); @@ -925,7 +925,7 @@ impl DedupService { // strictly in manifest order: `buffered` yields in input order. let prefetch = self.backend.read_prefetch().max(1); let backend = self.backend.clone(); - let mut opened = futures::stream::iter(chunks.iter().cloned()) + let mut opened = futures::stream::iter(chunks) .map(move |(hash, declared_size)| { let backend = backend.clone(); async move { @@ -1032,7 +1032,11 @@ impl DedupService { let mut total_size: u64 = 0; let mut chunk_hashes: Vec = Vec::new(); let mut chunk_sizes: Vec = Vec::new(); - let mut session_seen: HashSet = HashSet::new(); + // Keyed on the raw 32-byte BLAKE3 digest (`Copy`, no heap) rather than + // the 64-char hex String: the intra-upload dedup set no longer clones a + // String per chunk, holds 32-byte inline keys, and hashes 32 bytes not + // 64 on every membership test (benches/ROUND17.md §D2). + let mut session_seen: HashSet<[u8; 32]> = HashSet::new(); let mut pending: Vec<(String, Bytes)> = Vec::new(); let mut pending_bytes: usize = 0; // Depth-1 settle pipeline: batch N settles on a spawned task while @@ -1077,12 +1081,19 @@ impl DedupService { // Per-chunk hashing is ≤ 1 MiB of BLAKE3 (< 1 ms) — cheaper than // a spawn_blocking round-trip per chunk. file_hasher.update(&data); - let hash = blake3::hash(&data).to_hex().to_string(); + let digest = blake3::hash(&data); + let hash = digest.to_hex().to_string(); chunk_sizes.push(data.len() as u64); - chunk_hashes.push(hash.clone()); - if session_seen.insert(hash.clone()) { + // The hex `hash` is materialised once. A genuinely new chunk needs + // it in three places — the ordered manifest, the dedup set key and + // the backend write — but the set keys on the raw digest (no clone), + // so only `chunk_hashes` is cloned before `pending` takes the + // original. A duplicate within this upload needs it only for the + // manifest: the `else` moves it in, no clone (benches/ROUND17.md §D2). + if session_seen.insert(*digest.as_bytes()) { pending_bytes += data.len(); + chunk_hashes.push(hash.clone()); pending.push((hash, Bytes::from(data))); if pending.len() >= Self::FLUSH_MAX_CHUNKS || pending_bytes >= Self::FLUSH_MAX_BYTES { @@ -1111,6 +1122,10 @@ impl DedupService { } pending_bytes = 0; } + } else { + // Duplicate within this upload: only the ordered manifest needs + // the hash. Move it in — no set/pending copy, zero extra allocs. + chunk_hashes.push(hash); } } @@ -4000,7 +4015,7 @@ mod delta_upload_integration_tests { .collect(); let (computed, head) = svc - .hash_chunk_sequence(&sequence, 16) + .hash_chunk_sequence(sequence.clone(), 16) .await .expect("verification read"); assert_eq!(computed, file_hash, "recomputed hash must match"); @@ -4015,7 +4030,7 @@ mod delta_upload_integration_tests { let mut lying = sequence.clone(); lying[0].1 += 1; assert!( - svc.hash_chunk_sequence(&lying, 0).await.is_err(), + svc.hash_chunk_sequence(lying, 0).await.is_err(), "size lie must fail verification" );