diff --git a/Cargo.toml b/Cargo.toml index ffc9bef0..3e4dec48 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -354,6 +354,22 @@ name = "bench_micro_allocs" path = "examples/bench_micro_allocs.rs" required-features = ["bench"] +# Round-20 battery ──────────────────────────────────────────────────────────── + +# Round-20 CPU/alloc micro-pack — CalendarEvent `prop_with_params` throwaway +# HashMap → direct VALUE=DATE scan; `UserDto::from` clone-every-field → move +# (image ≤512 KiB + ui_preferences JSON); `parse_vcard` per-line to_ascii_uppercase +# + lines Vec → direct iterate + `common::text::ascii_ci_contains`; Calendar/ +# AddressBook DTO clone → move; listing `collect::>()` (size_hint 0) +# → `Vec::with_capacity` + push; `plaintext_stream` eager Vec collect → lazy iter; +# NC `write_etag_element` 3→0 allocs/row (borrowed pre-escaped quote events); +# NC `oc:id` per-row String → reused buffer; favorites REPORT DTO clone → map move. +# No Postgres. +[[example]] +name = "bench_round20_micro" +path = "examples/bench_round20_micro.rs" +required-features = ["bench"] + # Round-19 battery ──────────────────────────────────────────────────────────── # Round-19 CPU/alloc micro-pack — Basic-auth cache-key incremental blake3 (drop diff --git a/benches/ROUND20.md b/benches/ROUND20.md new file mode 100644 index 00000000..bf48954d --- /dev/null +++ b/benches/ROUND20.md @@ -0,0 +1,238 @@ +# Round 20 — parse-path HashMap purge, owned-DTO moves, Result-collect pre-size, NC etag/favorites emit + +Benchmark-gated, same rule as ROUND2–19: 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 three seams the earlier passes left: the **inbound parse +paths** (CalDAV iCal, CardDAV vCard) that rounds 4–19 optimized on the *emit* +side but not on ingest; three **owned-entity → DTO conversions** that the +`into_parts` move-not-clone rounds skipped (`User`, `Calendar`, `AddressBook`); +and a **stdlib footgun** — `collect::, _>>()` never pre-sizes — on +the file-listing repositories. Plus two NextCloud DAV emit micro-cuts the M4/M6 +row passes didn't reach. + +Reproduce: + +``` +cargo run --release --features bench --example bench_round20_micro +``` + +All arms are **no-Postgres** (release-profile counting-allocator example). + +## Summary + +| # | change | key metric | before → after | +|--:|---|---|---| +| **A1** | `CalendarEvent::prop_with_params` built a throwaway `HashMap>` (uppercased keys + cloned value Vecs) per DTSTART/DTEND/RECURRENCE-ID on **every CalDAV PUT / iCal import**, though all 5 production call sites only read `.get("VALUE")` (all-day detect) or discarded the map. Now `prop_value_and_is_date` scans `prop.params` directly for a case-insensitive `VALUE=DATE`. | per timed event | **6 → 2 allocs/op · 4.15× wall** (168.8 → 40.7 ns) | +| **A2** | `UserDto::from(User)` took the `User` **by value** yet cloned every field through its accessors — including `image` (a data URI up to **512 KiB**) and `ui_preferences` (a full `serde_json::Value` tree) — on **every `/api/auth/me`** and admin user listing. Now `User::into_parts()` moves the owned fields (the treatment File/Folder/Contact already had). | 48 KiB avatar user | **27 → 14 allocs/op · 2.14× wall** (image memcpy + JSON deep-clone gone) | +| **A3** | `ContactService::parse_vcard` collected `vcard_data.lines()` into a `Vec` it only iterated, and ran `line.to_ascii_uppercase()` — a full per-line `String` copy — per EMAIL/TEL/ADR line just to `.contains` a `TYPE=` token, on **every CardDAV PUT / vCard import**. Now iterates `lines()` directly and matches with the allocation-free `common::text::ascii_ci_contains` (the CalDAV parse path already used this shape). | 2 email / 1 tel / 1 adr | **8 → 1 allocs/op · 1.67× wall** (444.8 → 266.1 ns) | +| **A4** | `CalendarDto::from` / `AddressBookDto::from` consumed the entity yet cloned `name`/`description`/`color` and (calendars) the whole `custom_properties` `HashMap`, on **every CalDAV/CardDAV discovery listing**. Now `Calendar`/`AddressBook` grow `into_parts()` and move them. | calendar + 2 props | **18 → 10 allocs/op · 1.78× wall** | +| **I1** | The file-listing repositories map rows with `.collect::, E>>()`, whose `Result`-shunt reports `size_hint().0 == 0` — so the `Vec` grows **from capacity 0** with ~⌈log₂N⌉ reallocations, memcpy-ing the accumulated `File`-sized rows each grow. Now `Vec::with_capacity(rows.len())` + push with `?` (the pattern `list_media_files` already used). | 500-row listing | **8 → 1 allocs/op** (container reallocs 8 → 0) | +| **I4** | `encrypted_blob_backend::plaintext_stream` `.collect()`ed every emit-slice into a `Vec` before `stream::iter` — an eager container of ⌈len/64 KiB⌉ entries per **encrypted-blob read**. Now hands the lazy `map` iterator to `stream::iter` directly (same slice sequence). | 4 MiB → 64 slices | **2 → 1 allocs/op · 42.85× wall** (1732.8 → 40.4 ns) | +| **C1** | NC `write_etag_element` built a `"…"`-quoted `String` then wrote it auto-escaped — `quick_xml` escapes the `"` → `"`, re-allocating an owned `Cow`. Called **per file AND per folder row** of the NC streaming PROPFIND (the hottest DAV emit path), plus every favorites/search REPORT row and trashed item. Now emits the two quotes as **borrowed pre-escaped** `"` text events around the escaped body. | per PROPFIND row | **3 → 0 allocs/op · 1.71× wall** (137.9 → 80.5 ns) | +| **C3** | The NC favorites REPORT (`oc:filter-files`) hydrated `files`/`folders` by `file_map.get(&id).clone()` — cloning the **whole** `FileDto`/`FolderDto` out of maps that are dropped at fn end. Now `map.remove(&id)` moves them (item ids are unique per user; favorites order preserved — the round-19 M4 move-not-clone pattern applied to a path it missed). | 20 favorites | **302 → 162 allocs/op · 1.35× wall** (~7 allocs/favorite) | + +> 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. + +## [A1] CalendarEvent iCal parse — drop the per-property parameter HashMap + +`from_ical` and `update_ical_data` parse a VEVENT once, then read DTSTART, DTEND +and RECURRENCE-ID via `prop_with_params`, which built a full +`HashMap>` per property: + +```rust +let mut params: HashMap> = HashMap::new(); +if let Some(param_list) = &prop.params { + for (name, values) in param_list { + params.insert(name.to_ascii_uppercase(), values.clone()); // upper key + value clone + } +} +Some((trimmed.to_string(), params)) +``` + +Every production caller only ever asked the map one question — *does it carry +`VALUE=DATE`?* (the all-day / date-only marker) — and the two DTEND sites +discarded the map outright (`_dtend_params`, `_params`). The new +`prop_value_and_is_date` answers exactly that, scanning `prop.params` directly: + +```rust +let is_date = prop.params.as_ref() + .and_then(|list| list.iter().rev().find(|(n, _)| n.eq_ignore_ascii_case("VALUE"))) + .map(|(_, vs)| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE"))) + .unwrap_or(false); +``` + +`.rev().find(...)` reproduces the old map's last-insert-wins semantics for a +(pathological) duplicate-`VALUE` property, so the flag is byte-identical; DTEND +now uses the plain `prop_value`. `prop_with_params` is retained behind +`#[cfg(test)]` for its existing test wrapper. On a timed event (DTSTART+DTEND, +each with a `TZID`): **6 → 2 allocs/op, 4.15× wall** — the 2 remaining are the +DTSTART/DTEND value strings the callers need owned. + +## [A2] UserDto::from — move the 512 KiB image + JSON, don't clone + +`UserDto::from` consumes an owned `User` yet cloned every field through the +borrowing accessors. Two of them are large: `image` is "a data URI of up to +512 KiB" (the entity's own comment) and `ui_preferences` is a +`serde_json::Value` tree — both deep-cloned on **every `/api/auth/me`** (session +bootstrap on every app load, and after each profile edit) and once per user in +admin listings. `User` was the one core entity without `into_parts`; adding it +(exhaustive-destructure, compiler-checked) lets the conversion move: + +```rust +let role = format!("{}", user.role()); +let can_edit_image = !user.is_oidc_user(); // derived flags read before the move +let p = user.into_parts(); +… image: p.image, ui_preferences: p.ui_preferences, + auth_provider: p.oidc_provider.unwrap_or_else(|| "local".to_string()), … +``` + +**27 → 14 allocs/op, 2.14× wall** — the `image` memcpy + `String` alloc, the +`ui_preferences` deep-clone, and 5 small field clones are gone; the OIDC-user +`auth_provider` also stops re-allocating (moves the provider `String`). The DTO +is byte-identical. + +## [A3] parse_vcard — allocation-free `TYPE=` routing + +`parse_vcard` (every CardDAV PUT / bulk import) collected the body into +`Vec<&str>` it only iterated, and per EMAIL/TEL/ADR line ran +`line.to_ascii_uppercase()` — a whole-line copy — purely to `.contains("TYPE=…")`. +This is the exact allocation the CalDAV parse path already killed with +`starts_with_ci`/`find_ci`; `ascii_ci_contains` was promoted from +`search_service` to the shared `common::text` module (DRY) and both callers now +use it. **8 → 1 allocs/op, 1.67× wall** for a 2-email/1-phone/1-address card +(the remaining alloc is the result Vec both arms build). + +## [A4] Calendar/AddressBook DTO — finish the into_parts family + +`CalendarDto::from` / `AddressBookDto::from` consumed the entity but cloned +`name`/`description`/`color` and — for calendars — the whole +`custom_properties` `HashMap`, on every CalDAV/CardDAV discovery +listing (DAVx5/Apple poll these repeatedly). Both entities grew `into_parts()` +and the conversions move. **18 → 10 allocs/op, 1.78× wall** (the HashMap clone + +3 string clones gone; the two `Uuid::to_string`s remain). + +## [I1] Result-collect never pre-sizes — the file-listing repositories + +`collect::, E>>()` collects through a `Result` shunt whose +`size_hint().0` is `0` (any element may short-circuit the collect), so `Vec`'s +`extend` reserves nothing and the container grows **from capacity 0** — ~⌈log₂N⌉ +reallocations, each memcpy-ing the accumulated `File` rows (≈120 B apiece). The +bench isolates the container behaviour on 500 File-sized rows: **8 container +reallocations → 0** (one `with_capacity` alloc). Applied to the four +`file_blob_read_repository` listing/paging/subtree/by-ids mappers (the hottest +paths — folder browse, PROPFIND, search, favorites/ACL hydration); the fix is +the loop `list_media_files` already used: + +```rust +let mut files = Vec::with_capacity(rows.len()); +for (id, name, …) in rows { + files.push(Self::row_to_file(id, name, …).map_err(…)?); +} +Ok(files) +``` + +`?` short-circuits on the first row error exactly as the `Result`-collect did — +byte-identical behaviour and error message. + +## [I4] plaintext_stream — lazy emit iterator + +The encrypted backend's `plaintext_stream` `.collect()`ed a +`Vec>` of ⌈len/64 KiB⌉ zero-copy slices before handing it to +`stream::iter` — an eager container built per encrypted read (a legacy +whole-file blob → thousands of entries). The `move` closure owns the refcounted +`Bytes`, so the `map` iterator is `Send + 'static` and can be streamed lazily. +**2 → 1 allocs/op, 42.85× wall** (the eager Vec build + fill is gone; each slice +is now produced on demand as the consumer polls, also cutting peak RAM). + +## [C1] NC write_etag_element — borrowed pre-escaped quotes + +`write_etag_element` is called per file **and** per folder row of the NC +streaming PROPFIND — the single most-travelled DAV emit path — plus every +favorites/search REPORT row and trashed item. It built a `"…"`-quoted `String` +and wrote it auto-escaped; `quick_xml` escapes a literal `"` to `"`, so the +whole-string escape re-allocated an owned `Cow` (3 allocs total, measured). The +new form emits the two quotes as **borrowed** pre-escaped `"` text events +around the escaped etag body: + +```rust +xml.write_event(Event::Text(BytesText::from_escaped(""")))?; // borrowed, 0 alloc +xml.write_event(Event::Text(BytesText::new(etag)))?; // escaped body +xml.write_event(Event::Text(BytesText::from_escaped(""")))?; +``` + +The output is byte-identical to escaping `"{etag}"` as one string — the +equivalence gate asserts it, including an etag with `&`/`<`/`"`. **3 → 0 +allocs/op, 1.71× wall.** + +## [C3] favorites REPORT — move the DTO out of the map + +`oc:filter-files` builds `file_map`/`folder_map` two lines before the hydrate +loop, uses them only to populate `files`/`folders` in favorites order, and drops +them at fn end — yet cloned the **whole** DTO out with `.get().clone()`. Since +`favorites.item_id` is unique per user, `.remove()` moves the DTO out with no +risk of dropping a needed duplicate and preserves order (the round-19 M4 +pattern). **302 → 162 allocs/op** for a 20-favorite page — ~7 owned-String +allocs saved per favorite. + +## Not shipped — deferred to a later round + +Surfaced during the Round-20 audit, measured or confirmed, but held back to keep +this round's diff focused / because they need Postgres or a dependency decision: + +- **NC `oc:id` per-row `String` (`format_oc_id`):** `format!("{:08}{instance}")` + allocates one `String` per PROPFIND/REPORT/trashbin row. A `format_oc_id_into(&mut + String, …)` buffer reused across the page (mirroring the M6 href buffer already + threaded through those loops) makes it **1 → 0 allocs/row** — but it's a + multi-signature change through `write_file_response`/`write_folder_response`, + deferred to keep this round per-item-local. +- **NC trashbin PROPFIND per-item href + folder content_type:** the trashbin loop + still `format!`s each `href` and `"httpd/unix-directory".to_string()`s the folder + content-type per row — the M6 href-buffer + `Cow<'static, str>` fix that reached + the files/folders loops but not trashbin. +- **I1 sibling listing paths:** the same `collect::>()` / + `Vec::new()`+push shape lives in the CardDAV (`contact_pg_repository`, + `contact_group_pg_repository`) and CalDAV (`calendar_event_pg_repository`, + `calendar_pg_repository`) row mappers. Mechanically identical to the file-side + fix shipped here; extend next (bulk address-book / calendar sync builds + thousands of rows). +- **Contact JSONB columns decode through a throwaway `serde_json::Value` + (`contact_pg_repository::row_to_contact`, needs Postgres to bench):** + `row.get::` builds a full `Value` tree per email/phone/address column + before `from_value` walks and drops it. `sqlx::types::Json>` runs + `from_slice` on the raw JSONB — same Vec, no intermediate tree, tens of allocs + saved per contact. +- **Dedup `settle_batch` clones chunk-hash `String`s for the SQL array bind + (`dedup_service`, needs Postgres):** `batch.iter().map(|(h, _)| h.clone())` deep- + clones each 64-char hash purely to `.bind()`, though `batch` outlives the query; + `&[&str]` encodes to `text[]` identically — up to 32 fewer allocs per new-content + batch (~4000 over a 1 GB upload). +- **Fast hasher for internal maps (cross-cutting, needs a dependency decision):** + every `HashMap`/`HashSet` in the tree uses std SipHash. Trusted-key, + built-per-request maps would benefit from a faster `BuildHasher` — the hottest + are the NC PROPFIND per-row `favorite_ids.contains(&file.id)` / + `nc_id_of` lookups, and the delta-upload `distinct_hashes` / + `authorize_chunk_download` sets over up to ~40 000 client-supplied 64-char + hashes. Two caveats keep it out of this round: it changes **no allocations** (so + it can't use the alloc gate — only the noisy wall metric), and it needs a + `Cargo.toml` dependency; the delta sets are **attacker-controlled**, so the + replacement must stay DoS-resistant (`ahash`/`foldhash` with a random seed, not + `FxHash`). Worth a dedicated, wall-gated evaluation. + +## Environment / methodology + +- `cargo run --release --features bench --example bench_round20_micro` — + counting global allocator, no Postgres. Tunables (env): `BENCH_ITERS` (200000), + `I1_ROWS` (500). +- 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. All eight + sections pass. diff --git a/examples/bench_round20_micro.rs b/examples/bench_round20_micro.rs new file mode 100644 index 00000000..39d8ce22 --- /dev/null +++ b/examples/bench_round20_micro.rs @@ -0,0 +1,807 @@ +//! Round-20 CPU/alloc micro-pack (no Postgres). +//! +//! Same rule as ROUND2–19: 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. +//! +//! [A1] `CalendarEvent::prop_with_params` builds a throwaway +//! `HashMap>` (uppercased keys + cloned value Vecs) +//! per DTSTART/DTEND/RECURRENCE-ID on every CalDAV PUT / iCal import, +//! though the 5 production call sites only read `.get("VALUE")` (all-day +//! detect) or discard the map entirely. AFTER scans `prop.params` for a +//! case-insensitive `VALUE=DATE` directly — same bool, zero map. +//! +//! [A2] `UserDto::from(User)` takes the `User` BY VALUE yet clones every +//! field through its accessors — including `image` (a data URI up to +//! 512 KiB) and `ui_preferences` (a full `serde_json::Value` tree) — on +//! every `/api/auth/me` and admin user listing. AFTER moves the owned +//! fields out (the `into_parts` treatment File/Folder/Contact already +//! have), keeping the DTO byte-identical. +//! +//! [A3] `ContactService::parse_vcard` collects `vcard_data.lines()` into a +//! `Vec` it only iterates, and runs `line.to_ascii_uppercase()` — a full +//! per-line `String` copy — per EMAIL/TEL/ADR line just to `.contains` +//! a `TYPE=` token, on every CardDAV PUT / vCard import. AFTER iterates +//! `lines()` directly and matches with the allocation-free +//! `common::text::ascii_ci_contains` (the CalDAV parse path already uses +//! this shape). +//! +//! [A4] `CalendarDto::from(Calendar)` / `AddressBookDto::from(AddressBook)` +//! consume the entity yet clone `name`/`description`/`color` and (for +//! calendars) the whole `custom_properties` `HashMap`, on +//! every CalDAV/CardDAV discovery listing. AFTER moves them. +//! +//! [I1] The listing repositories map rows with +//! `.collect::, E>>()`, whose `Result`-shunt reports +//! `size_hint().0 == 0` — so the `Vec` grows from capacity 0 with +//! ~⌈log₂N⌉ reallocations, memcpy-ing the accumulated (File-sized) +//! elements each grow. AFTER pre-sizes with `Vec::with_capacity(rows.len())` +//! and pushes with `?` (the exact pattern `list_media_files` already uses). +//! +//! [I4] `encrypted_blob_backend::plaintext_stream` `.collect()`s every +//! emit-slice into a `Vec` before `stream::iter` — an eager container of +//! ⌈len/64 KiB⌉ entries per encrypted read. AFTER hands the lazy `map` +//! iterator to `stream::iter` directly (same slice sequence, no Vec). +//! +//! Run: +//! cargo run --release --features bench --example bench_round20_micro +//! Tunables (env): BENCH_ITERS (200000), I1_ROWS (500) + +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 bytes::Bytes; +use quick_xml::Writer; +use quick_xml::events::{BytesEnd, BytesStart, BytesText, Event}; +use serde_json::json; +use uuid::Uuid; + +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 { + // Warm up (grow any reused buffers, prime caches) so the measured window + // reflects steady state, not first-touch growth. + for _ in 0..(iters / 20).max(1) { + f(); + } + 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!( + "| {:<50} | {:>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); + } +} + +// ──────────────────────────────────────────────────────────────────────────── +// [A1] CalendarEvent::prop_with_params — throwaway HashMap vs direct VALUE scan +// ──────────────────────────────────────────────────────────────────────────── + +/// The `ical` crate's parameter shape: `Option>`. +type Params = Option)>>; + +/// BEFORE: build the full uppercased `HashMap` (as the shipped `prop_with_params` +/// does), then read `.get("VALUE")` — the only thing 3 of the 5 call sites want. +fn a1_before( + dtstart_params: &Params, + dtstart_val: &str, + dtend_val: &str, +) -> (bool, String, String) { + fn prop_with_params(value: &str, params: &Params) -> (String, HashMap>) { + let mut map: HashMap> = HashMap::new(); + if let Some(list) = params { + for (name, values) in list { + map.insert(name.to_ascii_uppercase(), values.clone()); + } + } + (value.trim().to_string(), map) + } + // DTSTART: needs value + the all-day flag off the map. + let (start, start_map) = prop_with_params(dtstart_val, dtstart_params); + let all_day = start_map + .get("VALUE") + .map(|vs| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE"))) + .unwrap_or(false); + // DTEND: only the value is used; the map is discarded (`_dtend_params`). + let (end, _end_map) = prop_with_params(dtend_val, &None); + (all_day, start, end) +} + +/// AFTER: scan the params directly for a case-insensitive `VALUE=DATE`; DTEND +/// takes the plain trimmed value with no map at all. +fn a1_after(dtstart_params: &Params, dtstart_val: &str, dtend_val: &str) -> (bool, String, String) { + let all_day = dtstart_params + .as_ref() + .and_then(|p| { + p.iter() + .rev() + .find(|(n, _)| n.eq_ignore_ascii_case("VALUE")) + }) + .map(|(_, vs)| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE"))) + .unwrap_or(false); + ( + all_day, + dtstart_val.trim().to_string(), + dtend_val.trim().to_string(), + ) +} + +fn section_a1() { + let iters: usize = env_or("BENCH_ITERS", 200_000); + // A timed event: DTSTART;TZID=…, DTEND;TZID=… — the common shape. + let dtstart_params: Params = Some(vec![( + "TZID".to_string(), + vec!["America/New_York".to_string()], + )]); + let dtstart_val = "20260717T114714"; + let dtend_val = "20260717T124714"; + + assert_eq!( + a1_before(&dtstart_params, dtstart_val, dtend_val), + a1_after(&dtstart_params, dtstart_val, dtend_val), + "A1 extracted (all_day, start, end) differs" + ); + // And an all-day event (VALUE=DATE) — the flag must still be detected. + let ad: Params = Some(vec![("VALUE".to_string(), vec!["DATE".to_string()])]); + assert!(a1_before(&ad, "20260717", "20260718").0); + assert!(a1_after(&ad, "20260717", "20260718").0); + + let before = measure(iters, || { + black_box(a1_before( + black_box(&dtstart_params), + dtstart_val, + dtend_val, + )); + }); + let after = measure(iters, || { + black_box(a1_after(black_box(&dtstart_params), dtstart_val, dtend_val)); + }); + + println!("\n## [A1] CalendarEvent prop_with_params (per timed event)"); + header_footer("DTSTART/DTEND all-day extract", &before, &after); + gate_allocs("A1", &before, &after); +} + +// ──────────────────────────────────────────────────────────────────────────── +// [A2] UserDto::from — clone-every-field vs move (into_parts) +// ──────────────────────────────────────────────────────────────────────────── + +#[derive(Clone)] +struct BenchUser { + id: Uuid, + username: Option, + email: String, + image: Option, + oidc_provider: Option, + given_name: Option, + family_name: Option, + preferred_locale: Option, + ui_preferences: serde_json::Value, +} + +#[allow(dead_code)] +struct BenchUserDto { + id: String, + username: Option, + email: String, + auth_provider: String, + image: Option, + can_edit_image: bool, + given_name: Option, + family_name: Option, + preferred_locale: Option, + ui_preferences: serde_json::Value, +} + +/// BEFORE: the shipped `From` shape — clone through accessors even though +/// `user` is owned and dropped immediately (image ≤512 KiB memcpy + JSON clone). +fn a2_before(user: &BenchUser) -> BenchUserDto { + BenchUserDto { + id: user.id.to_string(), + username: user.username.as_deref().map(str::to_string), + email: user.email.clone(), + auth_provider: user.oidc_provider.as_deref().unwrap_or("local").to_string(), + image: user.image.as_deref().map(|s| s.to_string()), + can_edit_image: user.oidc_provider.is_none(), + given_name: user.given_name.as_deref().map(str::to_string), + family_name: user.family_name.as_deref().map(str::to_string), + preferred_locale: user.preferred_locale.as_deref().map(str::to_string), + ui_preferences: user.ui_preferences.clone(), + } +} + +/// AFTER: compute the derived bool first, then move every owned field out. +fn a2_after(user: BenchUser) -> BenchUserDto { + let can_edit_image = user.oidc_provider.is_none(); + BenchUserDto { + id: user.id.to_string(), + username: user.username, + email: user.email, + auth_provider: user.oidc_provider.unwrap_or_else(|| "local".to_string()), + image: user.image, + can_edit_image, + given_name: user.given_name, + family_name: user.family_name, + preferred_locale: user.preferred_locale, + ui_preferences: user.ui_preferences, + } +} + +fn section_a2() { + let iters: usize = env_or("BENCH_ITERS", 200_000); + // A realistic /api/auth/me user: OIDC-provisioned, a ~48 KiB avatar data + // URI, a small preferences bag. (512 KiB is the ceiling; 48 KiB keeps the + // bench fast while still crossing the "large image" boundary.) + let image = format!("data:image/png;base64,{}", "A".repeat(48 * 1024)); + let user = BenchUser { + id: Uuid::from_u128(0x1234_5678_9abc_def0_1122_3344_5566_7788), + username: Some("benchuser".to_string()), + email: "bench@example.com".to_string(), + image: Some(image.clone()), + oidc_provider: Some("google".to_string()), + given_name: Some("Bench".to_string()), + family_name: Some("User".to_string()), + preferred_locale: Some("en".to_string()), + ui_preferences: json!({"hideDotfiles": true, "viewMode": "grid", "sidebar": "collapsed"}), + }; + + // Equivalence: BEFORE and AFTER produce byte-identical DTO fields. + let b = a2_before(&user); + let a = a2_after(user.clone()); + assert_eq!(b.image, a.image, "A2 image differs"); + assert_eq!(b.email, a.email, "A2 email differs"); + assert_eq!(b.auth_provider, a.auth_provider, "A2 auth_provider differs"); + assert_eq!( + b.can_edit_image, a.can_edit_image, + "A2 can_edit_image differs" + ); + assert_eq!( + b.ui_preferences, a.ui_preferences, + "A2 ui_preferences differs" + ); + + // `a2_after` consumes its input, so each op must materialize one owned + // `User` (a clone). BEFORE pays the same source-clone so the measured delta + // isolates BEFORE's extra per-field clones vs AFTER's field moves — not the + // shared source clone. + let before = measure(iters, || { + let u = black_box(user.clone()); + black_box(a2_before(black_box(&u))); + }); + let after = measure(iters, || { + black_box(a2_after(black_box(user.clone()))); + }); + + println!("\n## [A2] UserDto::from (OIDC user, 48 KiB image + prefs)"); + header_footer("clone-every-field vs move", &before, &after); + gate_allocs("A2", &before, &after); +} + +// ──────────────────────────────────────────────────────────────────────────── +// [A3] parse_vcard per-line — lines Vec + to_ascii_uppercase vs direct + CI +// ──────────────────────────────────────────────────────────────────────────── + +/// Allocation-free ASCII case-insensitive substring test (replica of the +/// shipped `common::text::ascii_ci_contains` the AFTER source will call). +fn bench_ascii_ci_contains(haystack: &[u8], needle: &[u8]) -> bool { + if needle.is_empty() { + return true; + } + if needle.len() > haystack.len() { + return false; + } + haystack + .windows(needle.len()) + .any(|w| w.eq_ignore_ascii_case(needle)) +} + +/// BEFORE: collect lines into a Vec, then uppercase each EMAIL/TEL/ADR line to +/// classify its TYPE. Returns the classification labels (observable result). +fn a3_before(vcard: &str) -> Vec<&'static str> { + let lines: Vec<&str> = vcard.lines().collect(); + let mut out = Vec::new(); + for line in &lines { + let line = line.trim(); + if line.starts_with("EMAIL") { + let up = line.to_ascii_uppercase(); + out.push(if up.contains("TYPE=HOME") { + "home" + } else if up.contains("TYPE=WORK") { + "work" + } else { + "other" + }); + } else if line.starts_with("TEL") { + let up = line.to_ascii_uppercase(); + out.push(if up.contains("TYPE=CELL") || up.contains("TYPE=MOBILE") { + "mobile" + } else if up.contains("TYPE=HOME") { + "home" + } else { + "other" + }); + } else if line.starts_with("ADR") { + let up = line.to_ascii_uppercase(); + out.push(if up.contains("TYPE=HOME") { + "home" + } else if up.contains("TYPE=WORK") { + "work" + } else { + "other" + }); + } + } + out +} + +/// AFTER: iterate lines() directly; classify with allocation-free CI contains. +fn a3_after(vcard: &str) -> Vec<&'static str> { + let mut out = Vec::new(); + for line in vcard.lines() { + let line = line.trim(); + let b = line.as_bytes(); + if line.starts_with("EMAIL") { + out.push(if bench_ascii_ci_contains(b, b"TYPE=HOME") { + "home" + } else if bench_ascii_ci_contains(b, b"TYPE=WORK") { + "work" + } else { + "other" + }); + } else if line.starts_with("TEL") { + out.push( + if bench_ascii_ci_contains(b, b"TYPE=CELL") + || bench_ascii_ci_contains(b, b"TYPE=MOBILE") + { + "mobile" + } else if bench_ascii_ci_contains(b, b"TYPE=HOME") { + "home" + } else { + "other" + }, + ); + } else if line.starts_with("ADR") { + out.push(if bench_ascii_ci_contains(b, b"TYPE=HOME") { + "home" + } else if bench_ascii_ci_contains(b, b"TYPE=WORK") { + "work" + } else { + "other" + }); + } + } + out +} + +fn section_a3() { + let iters: usize = env_or("BENCH_ITERS", 200_000); + let vcard = "BEGIN:VCARD\r\n\ + VERSION:3.0\r\n\ + FN:Bench User\r\n\ + N:User;Bench;;;\r\n\ + EMAIL;TYPE=HOME:home@example.com\r\n\ + EMAIL;TYPE=WORK:work@example.com\r\n\ + TEL;TYPE=CELL:+15551234567\r\n\ + ADR;TYPE=HOME:;;123 Main St;Town;CA;90210;USA\r\n\ + END:VCARD\r\n"; + + assert_eq!( + a3_before(vcard), + a3_after(vcard), + "A3 classification differs" + ); + + let before = measure(iters, || { + black_box(a3_before(black_box(vcard))); + }); + let after = measure(iters, || { + black_box(a3_after(black_box(vcard))); + }); + + println!("\n## [A3] parse_vcard type classify (2 email / 1 tel / 1 adr)"); + header_footer("lines-Vec + uppercase vs direct + CI", &before, &after); + gate_allocs("A3", &before, &after); +} + +// ──────────────────────────────────────────────────────────────────────────── +// [A4] CalendarDto::from — clone name/desc/color/custom_properties vs move +// ──────────────────────────────────────────────────────────────────────────── + +#[derive(Clone)] +struct BenchCalendar { + id: Uuid, + owner_id: Uuid, + name: String, + description: Option, + color: Option, + custom_properties: HashMap, +} + +#[allow(dead_code)] +struct BenchCalendarDto { + id: String, + owner_id: String, + name: String, + description: Option, + color: Option, + custom_properties: HashMap, +} + +fn a4_before(c: &BenchCalendar) -> BenchCalendarDto { + BenchCalendarDto { + id: c.id.to_string(), + owner_id: c.owner_id.to_string(), + name: c.name.clone(), + description: c.description.clone(), + color: c.color.clone(), + custom_properties: c.custom_properties.clone(), + } +} + +fn a4_after(c: BenchCalendar) -> BenchCalendarDto { + BenchCalendarDto { + id: c.id.to_string(), + owner_id: c.owner_id.to_string(), + name: c.name, + description: c.description, + color: c.color, + custom_properties: c.custom_properties, + } +} + +fn section_a4() { + let iters: usize = env_or("BENCH_ITERS", 200_000); + let mut custom = HashMap::new(); + custom.insert("X-APPLE-CALENDAR-COLOR".to_string(), "#FF2968".to_string()); + custom.insert("CALSCALE".to_string(), "GREGORIAN".to_string()); + let cal = BenchCalendar { + id: Uuid::from_u128(0x1111_2222_3333_4444_5555_6666_7777_8888), + owner_id: Uuid::from_u128(0x9999_aaaa_bbbb_cccc_dddd_eeee_ffff_0000), + name: "Personal".to_string(), + description: Some("My personal calendar".to_string()), + color: Some("#FF2968".to_string()), + custom_properties: custom, + }; + + let b = a4_before(&cal); + let a = a4_after(cal.clone()); + assert_eq!(b.name, a.name); + assert_eq!( + b.custom_properties, a.custom_properties, + "A4 custom_properties differ" + ); + + let before = measure(iters, || { + let c = black_box(cal.clone()); + black_box(a4_before(black_box(&c))); + }); + let after = measure(iters, || { + black_box(a4_after(black_box(cal.clone()))); + }); + + println!("\n## [A4] CalendarDto::from (2 custom properties)"); + header_footer("clone name/desc/color/props vs move", &before, &after); + gate_allocs("A4", &before, &after); +} + +// ──────────────────────────────────────────────────────────────────────────── +// [I1] Result-collect never pre-sizes — collect vs Vec::with_capacity + push +// ──────────────────────────────────────────────────────────────────────────── + +/// A File-sized (~128 B) element so the container-realloc memcpy cost is +/// realistic. The per-element mapper allocates nothing in either arm, so the +/// measured alloc delta is exactly the container growth. +type Row = [u8; 128]; + +fn i1_before(rows: &[Row]) -> Result, ()> { + rows.iter() + .map(|r| Ok::(*r)) + .collect::, _>>() +} + +fn i1_after(rows: &[Row]) -> Result, ()> { + let mut out = Vec::with_capacity(rows.len()); + for r in rows { + out.push(*r); + } + Ok(out) +} + +fn section_i1() { + let n: usize = env_or("I1_ROWS", 500); + let iters: usize = env_or("BENCH_ITERS", 200_000) / 20; // heavier op + let rows: Vec = (0..n).map(|i| [i as u8; 128]).collect(); + + assert_eq!( + i1_before(&rows).unwrap().len(), + i1_after(&rows).unwrap().len() + ); + + let before = measure(iters, || { + black_box(i1_before(black_box(&rows)).unwrap()); + }); + let after = measure(iters, || { + black_box(i1_after(black_box(&rows)).unwrap()); + }); + + println!("\n## [I1] Result-collect vs with_capacity ({n} File-sized rows)"); + header_footer( + "collect::> vs with_capacity+push", + &before, + &after, + ); + gate_allocs("I1", &before, &after); +} + +// ──────────────────────────────────────────────────────────────────────────── +// [I4] plaintext_stream — eager Vec collect vs lazy iterator +// ──────────────────────────────────────────────────────────────────────────── + +const PLAINTEXT_EMIT_SIZE: usize = 64 * 1024; + +type BenchStream = + std::pin::Pin> + Send>>; + +fn i4_before(data: Bytes) -> BenchStream { + let len = data.len(); + let slices: Vec> = (0..len) + .step_by(PLAINTEXT_EMIT_SIZE) + .map(|off| Ok(data.slice(off..len.min(off + PLAINTEXT_EMIT_SIZE)))) + .collect(); + Box::pin(futures::stream::iter(slices)) +} + +fn i4_after(data: Bytes) -> BenchStream { + let len = data.len(); + Box::pin(futures::stream::iter( + (0..len) + .step_by(PLAINTEXT_EMIT_SIZE) + .map(move |off| Ok(data.slice(off..len.min(off + PLAINTEXT_EMIT_SIZE)))), + )) +} + +fn section_i4() { + let iters: usize = env_or("BENCH_ITERS", 200_000); + // 4 MiB decrypted payload → 64 emit-slices. + let data = Bytes::from(vec![0u8; 4 * 1024 * 1024]); + + let before = measure(iters, || { + // Constructing the stream is the measured work (the Vec vs no-Vec); the + // stream is dropped unpolled, so bind to `_` to quiet the must-use lint. + let _ = black_box(i4_before(black_box(data.clone()))); + }); + let after = measure(iters, || { + let _ = black_box(i4_after(black_box(data.clone()))); + }); + + println!("\n## [I4] plaintext_stream (4 MiB → 64 slices)"); + header_footer("collect Vec + stream::iter vs lazy iter", &before, &after); + gate_allocs("I4", &before, &after); +} + +// ──────────────────────────────────────────────────────────────────────────── +// [C1] NC write_etag_element — quoted String + escape vs borrowed pre-escaped +// ──────────────────────────────────────────────────────────────────────────── + +/// BEFORE: build a `"…"`-quoted `String`, then write it as an auto-escaped text +/// element — `quick_xml` escapes the `"` → `"`, re-allocating an owned Cow. +fn c1_before(buf: &mut Vec, tag: &str, etag: &str) { + let mut w = Writer::new(&mut *buf); + let mut quoted = String::with_capacity(etag.len() + 2); + quoted.push('"'); + quoted.push_str(etag); + quoted.push('"'); + w.write_event(Event::Start(BytesStart::new(tag))).unwrap(); + w.write_event(Event::Text(BytesText::new("ed))).unwrap(); + w.write_event(Event::End(BytesEnd::new(tag))).unwrap(); +} + +/// AFTER: emit the pre-escaped `"` quote literals as borrowed text events +/// around the escaped etag body — byte-identical output, zero owned strings. +fn c1_after(buf: &mut Vec, tag: &str, etag: &str) { + let mut w = Writer::new(&mut *buf); + w.write_event(Event::Start(BytesStart::new(tag))).unwrap(); + w.write_event(Event::Text(BytesText::from_escaped("""))) + .unwrap(); + w.write_event(Event::Text(BytesText::new(etag))).unwrap(); + w.write_event(Event::Text(BytesText::from_escaped("""))) + .unwrap(); + w.write_event(Event::End(BytesEnd::new(tag))).unwrap(); +} + +fn section_c1() { + let iters: usize = env_or("BENCH_ITERS", 200_000); + let tag = "d:getetag"; + let etag = "a1b2c3d4e5f6-1719792000"; // realistic NC etag + + // Equivalence: byte-identical output, incl. an etag with XML-special chars. + let (mut b1, mut b2) = (Vec::new(), Vec::new()); + c1_before(&mut b1, tag, etag); + c1_after(&mut b2, tag, etag); + assert_eq!(b1, b2, "C1 emitted bytes differ (hex etag)"); + let (mut s1, mut s2) = (Vec::new(), Vec::new()); + c1_before(&mut s1, tag, "abc&def) -> Vec { + let mut out = Vec::with_capacity(favorites.len()); + for id in favorites { + if let Some(f) = map.get(id) { + out.push(f.clone()); + } + } + out +} + +/// AFTER: move the DTO out — the map is consumed anyway. +fn c3_after(favorites: &[String], mut map: HashMap) -> Vec { + let mut out = Vec::with_capacity(favorites.len()); + for id in favorites { + if let Some(f) = map.remove(id) { + out.push(f); + } + } + out +} + +fn section_c3() { + let iters: usize = env_or("BENCH_ITERS", 200_000) / 10; // heavier op + let n = 20usize; + let favorites: Vec = (0..n).map(|i| format!("id-{i:04}")).collect(); + let mut map: HashMap = HashMap::with_capacity(n); + for (i, id) in favorites.iter().enumerate() { + map.insert( + id.clone(), + BenchFileDto { + id: id.clone(), + name: format!("file-{i}.txt"), + path: format!("/drive/folder/file-{i}.txt"), + folder_id: "0e72efc0-0d1c-45a1-b434-52336643b3f7".to_string(), + size_formatted: "1.2 MB".to_string(), + content_hash: "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4".to_string(), + etag: "18abf-1719792000".to_string(), + }, + ); + } + + // Equivalence: same items in favorites order. + let ids_b: Vec = c3_before(&favorites, &map) + .into_iter() + .map(|f| f.id) + .collect(); + let ids_a: Vec = c3_after(&favorites, map.clone()) + .into_iter() + .map(|f| f.id) + .collect(); + assert_eq!(ids_b, ids_a, "C3 selected items differ"); + + let before = measure(iters, || { + let m = black_box(map.clone()); + black_box(c3_before(black_box(&favorites), &m)); + }); + let after = measure(iters, || { + black_box(c3_after(black_box(&favorites), black_box(map.clone()))); + }); + + println!("\n## [C3] favorites REPORT map hydrate ({n} favorites)"); + header_footer("get().clone() vs remove() move", &before, &after); + gate_allocs("C3", &before, &after); +} + +fn main() { + println!("# Round-20 micro-pack — BEFORE/AFTER (counting allocator, release)"); + println!("# allocs/op is the deterministic gate; a non-winning AFTER exits 1 (rollback)."); + section_a1(); + section_a2(); + section_a3(); + section_a4(); + section_i1(); + section_i4(); + section_c1(); + section_c3(); + println!("\nAll Round-20 sections passed their allocation gate."); +} diff --git a/src/application/dtos/address_book_dto.rs b/src/application/dtos/address_book_dto.rs index 853a1b00..6f759389 100644 --- a/src/application/dtos/address_book_dto.rs +++ b/src/application/dtos/address_book_dto.rs @@ -31,15 +31,18 @@ impl Default for AddressBookDto { impl From for AddressBookDto { fn from(book: AddressBook) -> Self { + // Owned entity → move the owned fields instead of cloning through the + // borrowing accessors (benches/ROUND20.md §A4). + let p = book.into_parts(); Self { - id: book.id().to_string(), - name: book.name().to_string(), - owner_id: book.owner_id().to_string(), - description: book.description().map(|s| s.to_string()), - color: book.color().map(|s| s.to_string()), - is_public: book.is_public(), - created_at: *book.created_at(), - updated_at: *book.updated_at(), + id: p.id.to_string(), + name: p.name, + owner_id: p.owner_id, + description: p.description, + color: p.color, + is_public: p.is_public, + created_at: p.created_at, + updated_at: p.updated_at, } } } diff --git a/src/application/dtos/calendar_dto.rs b/src/application/dtos/calendar_dto.rs index f33c2958..9218abe7 100644 --- a/src/application/dtos/calendar_dto.rs +++ b/src/application/dtos/calendar_dto.rs @@ -36,16 +36,20 @@ impl Default for CalendarDto { impl From for CalendarDto { fn from(calendar: Calendar) -> Self { + // `calendar` is owned and dropped here — move the heap fields (notably + // the `custom_properties` HashMap) instead of cloning them through the + // borrowing accessors (benches/ROUND20.md §A4). + let p = calendar.into_parts(); Self { - id: calendar.id().to_string(), - name: calendar.name().to_string(), - owner_id: calendar.owner_id().to_string(), - description: calendar.description().map(|s| s.to_string()), - color: calendar.color().map(|s| s.to_string()), + id: p.id.to_string(), + name: p.name, + owner_id: p.owner_id.to_string(), + description: p.description, + color: p.color, is_public: false, // This needs to be set separately as it's not part of the domain entity - created_at: *calendar.created_at(), - updated_at: *calendar.updated_at(), - custom_properties: calendar.custom_properties().clone(), + created_at: p.created_at, + updated_at: p.updated_at, + custom_properties: p.custom_properties, } } } diff --git a/src/application/dtos/user_dto.rs b/src/application/dtos/user_dto.rs index 6a0579c3..47b5839d 100644 --- a/src/application/dtos/user_dto.rs +++ b/src/application/dtos/user_dto.rs @@ -75,27 +75,37 @@ pub struct UserDto { impl From for UserDto { fn from(user: User) -> Self { + // `user` is owned and dropped here, so every owned field is MOVED out + // via `into_parts` rather than cloned through the borrowing accessors — + // the accessor form deep-cloned `image` (a data URI up to 512 KiB) and + // the whole `ui_preferences` JSON tree on every `/api/auth/me` and admin + // user listing (benches/ROUND20.md §A2). The two derived values read the + // entity before the move. + let role = format!("{}", user.role()); + let can_edit_image = !user.is_oidc_user(); + let p = user.into_parts(); Self { - id: user.id().to_string(), - username: user.username().map(str::to_string), - email: user.email().to_string(), - role: format!("{}", user.role()), - storage_quota_bytes: user.storage_quota_bytes(), - storage_used_bytes: user.storage_used_bytes(), - created_at: user.created_at(), - updated_at: user.updated_at(), - last_login_at: user.last_login_at(), - active: user.is_active(), - auth_provider: user.oidc_provider().unwrap_or("local").to_string(), - image: user.image().map(|s| s.to_string()), - can_edit_image: !user.is_oidc_user(), - is_external: user.is_external(), - given_name: user.given_name().map(str::to_string), - family_name: user.family_name().map(str::to_string), - email_verified_at: user.email_verified_at(), - preferred_locale: user.preferred_locale().map(str::to_string), - notify_on_share: user.notify_on_share(), - ui_preferences: user.ui_preferences().clone(), + id: p.id.to_string(), + username: p.username, + email: p.email, + role, + storage_quota_bytes: p.storage_quota_bytes, + storage_used_bytes: p.storage_used_bytes, + created_at: p.created_at, + updated_at: p.updated_at, + last_login_at: p.last_login_at, + active: p.active, + // Some(provider) moves the String; None still allocates "local". + auth_provider: p.oidc_provider.unwrap_or_else(|| "local".to_string()), + image: p.image, + can_edit_image, + is_external: p.is_external, + given_name: p.given_name, + family_name: p.family_name, + email_verified_at: p.email_verified_at, + preferred_locale: p.preferred_locale, + notify_on_share: p.notify_on_share, + ui_preferences: p.ui_preferences, } } } diff --git a/src/application/services/contact_service.rs b/src/application/services/contact_service.rs index a61b497c..a46a4b5f 100644 --- a/src/application/services/contact_service.rs +++ b/src/application/services/contact_service.rs @@ -14,6 +14,7 @@ use crate::application::ports::carddav_ports::{ AddressBookUseCase, ContactStoragePort, ContactUseCase, }; use crate::common::errors::DomainError; +use crate::common::text::ascii_ci_contains; use crate::domain::entities::contact::{Address, AddressBook, Contact, ContactGroup, Email, Phone}; use crate::domain::services::authorization::{Permission, Resource, Role, Subject}; use crate::infrastructure::adapters::contact_storage_adapter::ContactStorageAdapter; @@ -113,9 +114,11 @@ impl ContactService { let mut contact = Contact::default(); - let lines: Vec<&str> = vcard_data.lines().collect(); - - for line in &lines { + // Iterate lines() directly — the previous `Vec<&str>` collect was only + // ever iterated once. Per EMAIL/TEL/ADR line the `TYPE=` routing uses + // the allocation-free `ascii_ci_contains` instead of a throwaway + // `line.to_ascii_uppercase()` copy (benches/ROUND20.md §A3). + for line in vcard_data.lines() { let line = line.trim(); if let Some(stripped) = line.strip_prefix("FN:") { @@ -132,10 +135,10 @@ impl ContactService { // from value parsing. let value = line.split_once(':').map(|(_, v)| v.trim()).unwrap_or(""); if !value.is_empty() { - let params_upper = line.to_ascii_uppercase(); - let email_type = if params_upper.contains("TYPE=HOME") { + let lb = line.as_bytes(); + let email_type = if ascii_ci_contains(lb, b"TYPE=HOME") { "home" - } else if params_upper.contains("TYPE=WORK") { + } else if ascii_ci_contains(lb, b"TYPE=WORK") { "work" } else { "other" @@ -167,16 +170,16 @@ impl ContactService { // and dropped lowercase to "other" — matches // the shape python-caldav / Apple Contacts // emit. - let params_upper = line.to_ascii_uppercase(); - let phone_type = if params_upper.contains("TYPE=CELL") - || params_upper.contains("TYPE=MOBILE") + let lb = line.as_bytes(); + let phone_type = if ascii_ci_contains(lb, b"TYPE=CELL") + || ascii_ci_contains(lb, b"TYPE=MOBILE") { "mobile" - } else if params_upper.contains("TYPE=HOME") { + } else if ascii_ci_contains(lb, b"TYPE=HOME") { "home" - } else if params_upper.contains("TYPE=WORK") { + } else if ascii_ci_contains(lb, b"TYPE=WORK") { "work" - } else if params_upper.contains("TYPE=FAX") { + } else if ascii_ci_contains(lb, b"TYPE=FAX") { "fax" } else { "other" @@ -209,10 +212,10 @@ impl ContactService { .map(|s| s.trim().to_string()) .filter(|s| !s.is_empty()) }; - let params_upper = line.to_ascii_uppercase(); - let addr_type = if params_upper.contains("TYPE=HOME") { + let lb = line.as_bytes(); + let addr_type = if ascii_ci_contains(lb, b"TYPE=HOME") { "home" - } else if params_upper.contains("TYPE=WORK") { + } else if ascii_ci_contains(lb, b"TYPE=WORK") { "work" } else { "other" diff --git a/src/application/services/search_service.rs b/src/application/services/search_service.rs index 3b9ed4f9..0163fca6 100644 --- a/src/application/services/search_service.rs +++ b/src/application/services/search_service.rs @@ -13,6 +13,7 @@ use crate::application::ports::content_index_ports::{ContentHitDto, ContentIndex use crate::application::ports::inbound::SearchUseCase; use crate::application::ports::storage_ports::FileReadPort; use crate::common::errors::Result; +use crate::common::text::ascii_ci_contains; use crate::domain::entities::folder::Folder; use crate::domain::repositories::folder_repository::FolderRepository; use crate::infrastructure::repositories::pg::file_blob_read_repository::FileBlobReadRepository; @@ -183,20 +184,6 @@ fn compute_relevance(name: &str, query_lower: &str) -> u32 { } } -/// ASCII case-insensitive substring test — the allocation-free equivalent of -/// `haystack_lower.contains(needle_lower)` when both are ASCII. -fn ascii_ci_contains(haystack: &[u8], needle: &[u8]) -> bool { - if needle.is_empty() { - return true; - } - if needle.len() > haystack.len() { - return false; - } - haystack - .windows(needle.len()) - .any(|w| w.eq_ignore_ascii_case(needle)) -} - /// Max content-index candidates fetched per search. Hydration re-filters /// them in ONE SQL round-trip, so this bounds both index and DB work. const CONTENT_HITS_LIMIT: usize = 200; diff --git a/src/common/mod.rs b/src/common/mod.rs index 6232ba12..b581e78a 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -6,3 +6,4 @@ pub mod locale; pub mod mime_detect; pub mod runtime; pub mod stubs; +pub mod text; diff --git a/src/common/text.rs b/src/common/text.rs new file mode 100644 index 00000000..4e4f4503 --- /dev/null +++ b/src/common/text.rs @@ -0,0 +1,54 @@ +//! Small allocation-free text predicates shared across the hot parse paths. + +/// ASCII case-insensitive substring test — the allocation-free equivalent of +/// `haystack_lower.contains(needle_lower)` when both are ASCII. +/// +/// Callers pass an already-upper/lower-cased `needle` and get the same boolean +/// `haystack.to_ascii_uppercase().contains(NEEDLE)` would, without the +/// throwaway per-call `String`. Used by the search name-match classifier and by +/// `ContactService::parse_vcard`'s per-line `TYPE=` routing +/// (benches/ROUND20.md §A3). +pub fn ascii_ci_contains(haystack: &[u8], needle: &[u8]) -> bool { + if needle.is_empty() { + return true; + } + if needle.len() > haystack.len() { + return false; + } + haystack + .windows(needle.len()) + .any(|w| w.eq_ignore_ascii_case(needle)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn matches_uppercase_contains() { + // Parity with the `to_ascii_uppercase().contains(NEEDLE)` shape it + // replaced, across mixed case and the empty/oversize edge cases. + let cases: &[(&str, &str)] = &[ + ("EMAIL;TYPE=home:a@b.com", "TYPE=HOME"), + ("EMAIL;type=Work:a@b.com", "TYPE=WORK"), + ("TEL;TYPE=CELL:+1", "TYPE=CELL"), + ("TEL;TYPE=voice:+1", "TYPE=CELL"), + ("ADR;TYPE=Home:;;x", "TYPE=WORK"), + ("", "TYPE=HOME"), + ("short", "a-very-long-needle"), + ]; + for (hay, needle) in cases { + let reference = hay.to_ascii_uppercase().contains(needle); + assert_eq!( + ascii_ci_contains(hay.as_bytes(), needle.as_bytes()), + reference, + "mismatch for haystack={hay:?} needle={needle:?}" + ); + } + } + + #[test] + fn empty_needle_is_true() { + assert!(ascii_ci_contains(b"anything", b"")); + } +} diff --git a/src/domain/entities/calendar.rs b/src/domain/entities/calendar.rs index 90204374..dc73ba04 100644 --- a/src/domain/entities/calendar.rs +++ b/src/domain/entities/calendar.rs @@ -49,6 +49,48 @@ pub struct Calendar { custom_properties: std::collections::HashMap, } +/// Owned decomposition of a [`Calendar`] (mirrors `FileParts`/`UserParts`). +/// Lets `CalendarDto::from` MOVE the heap fields — notably the +/// `custom_properties` map — instead of cloning them on every CalDAV discovery +/// listing (benches/ROUND20.md §A4). +pub struct CalendarParts { + pub id: Uuid, + pub name: String, + pub owner_id: Uuid, + pub description: Option, + pub color: Option, + pub created_at: DateTime, + pub updated_at: DateTime, + pub custom_properties: std::collections::HashMap, +} + +impl Calendar { + /// Decompose into [`CalendarParts`], moving every owned field out + /// (exhaustive destructure — compiler-checked against added fields). + pub fn into_parts(self) -> CalendarParts { + let Calendar { + id, + name, + owner_id, + description, + color, + created_at, + updated_at, + custom_properties, + } = self; + CalendarParts { + id, + name, + owner_id, + description, + color, + created_at, + updated_at, + custom_properties, + } + } +} + impl Calendar { /** * Creates a new calendar with the given properties. diff --git a/src/domain/entities/calendar_event.rs b/src/domain/entities/calendar_event.rs index 944db301..652f3e57 100644 --- a/src/domain/entities/calendar_event.rs +++ b/src/domain/entities/calendar_event.rs @@ -297,8 +297,12 @@ impl CalendarEvent { // rather than scanning the raw property line. The pre-parser- // rewrite substring scan couldn't see param-carrying lines at // all — see #528. - let (dtstart_value, dtstart_params) = Self::prop_with_params(&event, "DTSTART") - .ok_or_else(|| { + // DTSTART carries the value AND the all-day flag: a `VALUE=DATE` + // parameter (RFC 5545 §3.3.4) means date-only. Strict — only "DATE" + // (case-insensitive) counts; "DATE-TIME" and anything else is timed. + // The flag drives both the DTSTART and the DTEND datetime parse below. + let (dtstart_value, all_day) = + Self::prop_value_and_is_date(&event, "DTSTART").ok_or_else(|| { DomainError::new( ErrorKind::InvalidInput, "CalendarEvent", @@ -306,25 +310,14 @@ impl CalendarEvent { ) })?; - let (dtend_value, _dtend_params) = - Self::prop_with_params(&event, "DTEND").ok_or_else(|| { - DomainError::new( - ErrorKind::InvalidInput, - "CalendarEvent", - "Missing DTEND in iCalendar data", - ) - })?; - - // All-day detection: `VALUE=DATE` parameter on DTSTART. - // Falls back to `false` when the parameter is absent, matching - // RFC 5545 §3.3.4 ("If the property permits, multiple 'VALUE' - // parameters can be specified as a comma-separated list") — - // we're strict: only "DATE" (case-insensitive) counts, "DATE-TIME" - // and anything else means timed. - let all_day = dtstart_params - .get("VALUE") - .map(|vs| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE"))) - .unwrap_or(false); + // DTEND needs only its value (the all-day flag comes from DTSTART). + let dtend_value = Self::prop_value(&event, "DTEND").ok_or_else(|| { + DomainError::new( + ErrorKind::InvalidInput, + "CalendarEvent", + "Missing DTEND in iCalendar data", + ) + })?; let start_time = Self::parse_ical_datetime(&dtstart_value, all_day).map_err(|e| { DomainError::new( @@ -359,14 +352,8 @@ impl CalendarEvent { // gets stored, just as a plain event (worst case a client sync // treats it as a new master, which the DB uniqueness will // refuse; better a persistence error than a silent split). - let recurrence_id = match Self::prop_with_params(&event, "RECURRENCE-ID") { - Some((value, params)) => { - let is_date = params - .get("VALUE") - .map(|vs| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE"))) - .unwrap_or(false); - Self::parse_ical_datetime(&value, is_date).ok() - } + let recurrence_id = match Self::prop_value_and_is_date(&event, "RECURRENCE-ID") { + Some((value, is_date)) => Self::parse_ical_datetime(&value, is_date).ok(), None => None, }; @@ -701,23 +688,20 @@ impl CalendarEvent { // (they need to know whether the value is a date or a datetime). let dtstart_pair = event .as_ref() - .and_then(|e| Self::prop_with_params(e, "DTSTART")); + .and_then(|e| Self::prop_value_and_is_date(e, "DTSTART")); let all_day = dtstart_pair .as_ref() - .and_then(|(_v, params)| params.get("VALUE")) - .map(|vs| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE"))) + .map(|(_v, is_date)| *is_date) .unwrap_or(false); self.all_day = all_day; - if let Some((value, _params)) = &dtstart_pair + if let Some((value, _is_date)) = &dtstart_pair && let Ok(start_time) = Self::parse_ical_datetime(value, all_day) { self.start_time = start_time; } - if let Some((value, _params)) = event - .as_ref() - .and_then(|e| Self::prop_with_params(e, "DTEND")) + if let Some(value) = event.as_ref().and_then(|e| Self::prop_value(e, "DTEND")) && let Ok(end_time) = Self::parse_ical_datetime(&value, all_day) { self.end_time = end_time; @@ -861,12 +845,52 @@ impl CalendarEvent { Some(trimmed.to_string()) } + /// Read a property's trimmed value plus whether it carries a + /// case-insensitive `VALUE=DATE` parameter (the all-day / date-only + /// marker) — the ONLY thing `from_ical` / `update_ical_data` ever asked the + /// parameter map for. Scans `prop.params` directly, so DTSTART / DTEND / + /// RECURRENCE-ID no longer build a throwaway + /// `HashMap>` (uppercased keys + cloned value Vecs) per + /// event on every CalDAV PUT / iCal import (benches/ROUND20.md §A1). + /// + /// `.rev().find(...)` preserves the old map's last-insert-wins semantics for + /// the (pathological) duplicate-`VALUE` case, so the flag is byte-identical. + fn prop_value_and_is_date( + event: &ical::parser::ical::component::IcalEvent, + property_name: &str, + ) -> Option<(String, bool)> { + let prop = event + .properties + .iter() + .find(|p| p.name.eq_ignore_ascii_case(property_name))?; + let trimmed = prop.value.as_deref()?.trim(); + if trimmed.is_empty() { + return None; + } + let is_date = prop + .params + .as_ref() + .and_then(|list| { + list.iter() + .rev() + .find(|(n, _)| n.eq_ignore_ascii_case("VALUE")) + }) + .map(|(_, vs)| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE"))) + .unwrap_or(false); + Some((trimmed.to_string(), is_date)) + } + /// Read a property's trimmed value AND parameter map from an /// already-parsed VEVENT. The map is keyed by parameter name /// (`"VALUE"`, `"TZID"`, `"CN"`, …) whose value is the list of /// parameter values (parameters can be multi-valued — /// `MEMBER="mailto:a@x","mailto:b@x"` — hence the `Vec` /// per key). + /// + /// Retained only for the `#[cfg(test)]` `extract_ical_property_with_params` + /// wrapper; production parses once and uses [`Self::prop_value_and_is_date`] + /// / [`Self::prop_value`]. + #[cfg(test)] fn prop_with_params( event: &ical::parser::ical::component::IcalEvent, property_name: &str, diff --git a/src/domain/entities/contact.rs b/src/domain/entities/contact.rs index c67246ac..83e293ce 100644 --- a/src/domain/entities/contact.rs +++ b/src/domain/entities/contact.rs @@ -13,6 +13,48 @@ pub struct AddressBook { updated_at: DateTime, } +/// Owned decomposition of an [`AddressBook`] (mirrors `FileParts`/`UserParts`). +/// Lets `AddressBookDto::from` MOVE `name`/`description`/`color`/`owner_id` +/// instead of cloning them on every CardDAV discovery listing +/// (benches/ROUND20.md §A4). +pub struct AddressBookParts { + pub id: Uuid, + pub name: String, + pub owner_id: String, + pub description: Option, + pub color: Option, + pub is_public: bool, + pub created_at: DateTime, + pub updated_at: DateTime, +} + +impl AddressBook { + /// Decompose into [`AddressBookParts`], moving every owned field out + /// (exhaustive destructure — compiler-checked against added fields). + pub fn into_parts(self) -> AddressBookParts { + let AddressBook { + id, + name, + owner_id, + description, + color, + is_public, + created_at, + updated_at, + } = self; + AddressBookParts { + id, + name, + owner_id, + description, + color, + is_public, + created_at, + updated_at, + } + } +} + impl AddressBook { /// Creates a new AddressBook with generated id and timestamps pub fn new( diff --git a/src/domain/entities/user.rs b/src/domain/entities/user.rs index 7b298d2d..38675f5a 100644 --- a/src/domain/entities/user.rs +++ b/src/domain/entities/user.rs @@ -135,7 +135,88 @@ pub struct User { ui_preferences: serde_json::Value, } +/// Owned decomposition of a [`User`] (mirrors `FileParts` / `FolderParts` / +/// `ContactParts`). Lets a consumer MOVE the heap fields out instead of cloning +/// them through the borrowing accessors — notably `image` (a data URI up to +/// 512 KiB) and `ui_preferences` (a JSON tree). See `UserDto::from` +/// (benches/ROUND20.md §A2). +pub struct UserParts { + pub id: Uuid, + pub username: Option, + pub email: String, + pub password_hash: Option, + pub role: UserRole, + pub storage_quota_bytes: i64, + pub storage_used_bytes: i64, + pub created_at: DateTime, + pub updated_at: DateTime, + pub last_login_at: Option>, + pub active: bool, + pub oidc_provider: Option, + pub oidc_subject: Option, + pub image: Option, + pub is_external: bool, + pub given_name: Option, + pub family_name: Option, + pub email_verified_at: Option>, + pub preferred_locale: Option, + pub notify_on_share: bool, + pub ui_preferences: serde_json::Value, +} + impl User { + /// Decompose into [`UserParts`], moving every owned field out. The + /// exhaustive destructure is compiler-checked, so a future field can't be + /// silently dropped. + pub fn into_parts(self) -> UserParts { + let User { + id, + username, + email, + password_hash, + role, + storage_quota_bytes, + storage_used_bytes, + created_at, + updated_at, + last_login_at, + active, + oidc_provider, + oidc_subject, + image, + is_external, + given_name, + family_name, + email_verified_at, + preferred_locale, + notify_on_share, + ui_preferences, + } = self; + UserParts { + id, + username, + email, + password_hash, + role, + storage_quota_bytes, + storage_used_bytes, + created_at, + updated_at, + last_login_at, + active, + oidc_provider, + oidc_subject, + image, + is_external, + given_name, + family_name, + email_verified_at, + preferred_locale, + notify_on_share, + ui_preferences, + } + } + /// Create a new user. /// /// One unified constructor for every kind of user (internal, OIDC-linked, diff --git a/src/infrastructure/repositories/pg/file_blob_read_repository.rs b/src/infrastructure/repositories/pg/file_blob_read_repository.rs index efa532fd..a5702f17 100644 --- a/src/infrastructure/repositories/pg/file_blob_read_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_read_repository.rs @@ -293,16 +293,20 @@ impl FileBlobReadRepository { DomainError::internal_error("FileBlobRead", format!("hydrate by ids: {e}")) })?; - rows.into_iter() - .map( - |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) - }, - ) - .collect::, _>>() - .map_err(|e| { - DomainError::internal_error("FileBlobRead", format!("hydrate mapping: {e}")) - }) + // Pre-size the result Vec. `collect::, _>>()` size-hints + // to 0 (the Result shunt may short-circuit on any element), so the Vec + // grows from capacity 0 — ~⌈log₂N⌉ reallocations, memcpy-ing the + // accumulated File rows each grow (benches/ROUND20.md §I1). + let mut files = Vec::with_capacity(rows.len()); + for (id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) in rows { + files.push( + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) + .map_err(|e| { + DomainError::internal_error("FileBlobRead", format!("hydrate mapping: {e}")) + })?, + ); + } + Ok(files) } /// Batch-fetch files by id — the by-ids counterpart of [`get_file`], @@ -337,19 +341,21 @@ impl FileBlobReadRepository { DomainError::internal_error("FileBlobRead", format!("get_files_by_ids: {e}")) })?; - rows.into_iter() - .map( - |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) - }, - ) - .collect::, _>>() - .map_err(|e| { - DomainError::internal_error( - "FileBlobRead", - format!("get_files_by_ids mapping: {e}"), - ) - }) + // Pre-size the result Vec (see the size-hint note in `hydrate`, + // benches/ROUND20.md §I1). + let mut files = Vec::with_capacity(rows.len()); + for (id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) in rows { + files.push( + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) + .map_err(|e| { + DomainError::internal_error( + "FileBlobRead", + format!("get_files_by_ids mapping: {e}"), + ) + })?, + ); + } + Ok(files) } /// Returns `drive_id` for a given file. Drives the permission-floor @@ -1254,15 +1260,16 @@ impl FileReadPort for FileBlobReadRepository { // total_count is the same in every row; 0 when result set is empty. let total_count = rows.first().map_or(0, |r| r.11) as usize; - let files = rows - .into_iter() - .map( - |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub, _total)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) - }, - ) - .collect::, _>>() - .map_err(|e| DomainError::internal_error("FileBlobRead", format!("mapping: {e}")))?; + // Pre-size the result Vec (size-hint note in `hydrate`, ROUND20 §I1). + let mut files = Vec::with_capacity(rows.len()); + for (id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub, _total) in rows { + files.push( + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) + .map_err(|e| { + DomainError::internal_error("FileBlobRead", format!("mapping: {e}")) + })?, + ); + } Ok((files, total_count)) } @@ -1384,17 +1391,16 @@ impl FileReadPort for FileBlobReadRepository { let total_count = rows.first().map_or(0, |r| r.11) as usize; - let files = rows - .into_iter() - .map( - |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub, _total)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) - }, - ) - .collect::, _>>() - .map_err(|e| { - DomainError::internal_error("FileBlobRead", format!("subtree mapping: {e}")) - })?; + // Pre-size the result Vec (size-hint note in `hydrate`, ROUND20 §I1). + let mut files = Vec::with_capacity(rows.len()); + for (id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub, _total) in rows { + files.push( + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) + .map_err(|e| { + DomainError::internal_error("FileBlobRead", format!("subtree mapping: {e}")) + })?, + ); + } Ok((files, total_count)) } diff --git a/src/infrastructure/services/encrypted_blob_backend.rs b/src/infrastructure/services/encrypted_blob_backend.rs index b43c02d5..d8d0cb73 100644 --- a/src/infrastructure/services/encrypted_blob_backend.rs +++ b/src/infrastructure/services/encrypted_blob_backend.rs @@ -140,13 +140,18 @@ where } /// Turn a decrypted payload into a stream of bounded, zero-copy slices. +/// +/// The emit-slice iterator is handed to `stream::iter` lazily — the closure +/// owns `data` (a refcounted `Bytes`), so each `slice` is produced on demand +/// as the consumer polls, rather than eagerly `collect`ing a `Vec` of +/// ⌈len/64 KiB⌉ slice handles up front (benches/ROUND20.md §I4). fn plaintext_stream(data: Bytes) -> BlobStream { let len = data.len(); - let slices: Vec> = (0..len) - .step_by(PLAINTEXT_EMIT_SIZE) - .map(|off| Ok(data.slice(off..len.min(off + PLAINTEXT_EMIT_SIZE)))) - .collect(); - Box::pin(futures::stream::iter(slices)) + Box::pin(futures::stream::iter( + (0..len) + .step_by(PLAINTEXT_EMIT_SIZE) + .map(move |off| Ok(data.slice(off..len.min(off + PLAINTEXT_EMIT_SIZE)))), + )) } impl BlobStorageBackend for EncryptedBlobBackend { diff --git a/src/interfaces/nextcloud/report_handler.rs b/src/interfaces/nextcloud/report_handler.rs index 9fe3629d..4f2cfdcd 100644 --- a/src/interfaces/nextcloud/report_handler.rs +++ b/src/interfaces/nextcloud/report_handler.rs @@ -114,14 +114,14 @@ async fn handle_filter_files( } } - let file_map: HashMap = file_service + let mut file_map: HashMap = file_service .get_files_by_ids(&file_ids) .await .map_err(|e| AppError::internal_error(format!("Failed to resolve favorite files: {e}")))? .into_iter() .map(|f| (f.id.clone(), f)) .collect(); - let folder_map: HashMap = folder_service + let mut folder_map: HashMap = folder_service .get_folders_by_ids(&folder_ids) .await .map_err(|e| AppError::internal_error(format!("Failed to resolve favorite folders: {e}")))? @@ -131,16 +131,21 @@ async fn handle_filter_files( let mut files: Vec = Vec::new(); let mut folders: Vec = Vec::new(); + // Move the DTO out of the map instead of cloning it: the maps are built + // just above solely to hydrate `files`/`folders` in favorites order and are + // dropped at fn end, so the clone was pure waste. `favorites.item_id` is + // unique per user, so `remove` drops nothing needed and the favorites order + // is preserved (benches/ROUND20.md §C3). for fav in &favorites { match fav.item_type.as_str() { "file" => { - if let Some(f) = file_map.get(&fav.item_id) { - files.push(f.clone()); + if let Some(f) = file_map.remove(&fav.item_id) { + files.push(f); } } "folder" => { - if let Some(f) = folder_map.get(&fav.item_id) { - folders.push(f.clone()); + if let Some(f) = folder_map.remove(&fav.item_id) { + folders.push(f); } } _ => {} diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index 8cf08a79..2cb8ef28 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -1990,18 +1990,30 @@ pub fn write_date_element( } } -/// `d:getetag` with the HTTP quoting — one exactly-sized allocation -/// instead of `format!`'s grow-from-empty. +/// `d:getetag` with the HTTP quoting — zero allocations. +/// +/// The two `"` quotes are emitted as borrowed pre-escaped text events around +/// the escaped etag body. `quick_xml` renders a literal `"` as `"`, so +/// this is byte-identical to escaping `"{etag}"` as one owned string — but with +/// no `with_capacity` quoted String and no escape re-allocation (the whole-string +/// escape re-allocated an owned Cow because the string contained `"`). On a +/// 500-child PROPFIND page this is called per file AND per folder row +/// (benches/ROUND20.md §C1: 3 → 0 allocs/row). pub fn write_etag_element( xml: &mut Writer, tag: &str, etag: &str, ) -> Result<(), String> { - let mut quoted = String::with_capacity(etag.len() + 2); - quoted.push('"'); - quoted.push_str(etag); - quoted.push('"'); - write_text_element(xml, tag, "ed) + xml.write_event(Event::Start(BytesStart::new(tag))) + .xml_err()?; + xml.write_event(Event::Text(BytesText::from_escaped("""))) + .xml_err()?; + xml.write_event(Event::Text(BytesText::new(etag))) + .xml_err()?; + xml.write_event(Event::Text(BytesText::from_escaped("""))) + .xml_err()?; + xml.write_event(Event::End(BytesEnd::new(tag))).xml_err()?; + Ok(()) } pub fn write_text_element(