From 9333037fc7f90037ebc574814801b1fa430235b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 01:49:37 +0000 Subject: [PATCH] perf(round28): extend the PROPFIND oc:id reused buffer (round27 H1) to the REPORT emit loops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit report_handler's four REPORT emit loops (the filter-files favorites REPORT and the search REPORT, each a file loop + a folder loop) shared the same per-row oc:id String that ROUND27 §H1 replaced in the two PROPFIND page loops. Apply the identical, already-validated transformation: hoist one oc_buf per handler (reused across both its loops) and compute the id into it via format_oc_id_into instead of a fresh format_oc_id String per child. 1 String/row -> 0 (amortized). The write_{file,folder}_response fns already take Option<&str>, so their signatures are unchanged and the emitted oc:id bytes are byte-identical. Same transformation benchmarked in ROUND27 §H1 (bench_round27_micro: 998 -> 0 per-row allocs, 2.16x wall), so no new bench. Verified: cargo fmt clean, cargo clippy --features bench -D warnings clean, cargo test --lib --features bench = 529 passed / 0 failed across 5 consecutive runs. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01L8gs91AhmazoxMsDcNk3KT --- benches/ROUND28.md | 75 ++++++++++++++++++++++ src/interfaces/nextcloud/report_handler.rs | 47 +++++++++++--- 2 files changed, 113 insertions(+), 9 deletions(-) create mode 100644 benches/ROUND28.md diff --git a/benches/ROUND28.md b/benches/ROUND28.md new file mode 100644 index 00000000..c09f59d1 --- /dev/null +++ b/benches/ROUND28.md @@ -0,0 +1,75 @@ +# Round 28 — extend the PROPFIND oc:id buffer (ROUND27 §H1) to the REPORT emit loops + +A small follow-through: ROUND27 §H1 replaced the per-row `oc:id` `String` with one +reused `oc_buf` in the two NextCloud **PROPFIND** page loops, but the four +**REPORT** emit loops (`report_handler`) shared the identical per-row-String +shape and were explicitly deferred there. This round applies the same validated +transformation to them. + +## The change + +`report_handler`'s two REPORT handlers (`filter-files` favorites REPORT and +`search` REPORT) each emit a file loop and a folder loop, and each row did: + +```rust +let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); // one String per row +… +write_{file,folder}_response(&mut xml, …, (fid, oc_id.as_deref()), …) +``` + +AFTER hoists one `oc_buf` per handler (reused across both its loops, beside the +same pattern the PROPFIND loops already use) and computes the id into it with +`format_oc_id_into` (added in ROUND27): + +```rust +let mut oc_buf = String::new(); // once per handler +… +let oc_id: Option<&str> = match fid { + Some(id) => { format_oc_id_into(&mut oc_buf, id, file_id_svc); Some(oc_buf.as_str()) } + None => None, +}; +write_{file,folder}_response(&mut xml, …, (fid, oc_id), …) +``` + +**1 String/row → 0** (amortized to one buffer per handler) across all four REPORT +loops. The `write_*_response` functions already take `Option<&str>`, so their +signatures are unchanged and the emitted `oc:id` bytes are byte-identical. + +## Benchmark + +This is the **same** transformation validated in ROUND27 §H1 +(`bench_round27_micro`): a per-row `format_oc_id` String vs one reused buffer via +`format_oc_id_into`, byte-identical output. §H1 measured it on a 500-row page: + +| arm | ns/op | allocs/op | +|--------|---------:|----------:| +| BEFORE | 34 185.3 | 1 000.00 | +| AFTER | 14 484.9 | 2.00 | + +**998 → 0 per-row allocs, 2.16–2.36× wall.** ROUND28 applies that proven change +to four more instances of the identical pattern (the REPORT loops), so no new +benchmark is needed — the §H1 gate is the evidence. REPORT/search is lower-traffic +than PROPFIND, so the aggregate impact is smaller, but it removes the last per-row +`oc:id` allocation from the NC emit surface. + +## Not shipped — carried forward + +- **`format_oc_id_into` for the trashbin per-item writer** (`write_trash_item_response`) + would need the buffer threaded through its signature (it is a per-item fn, not a + loop with a hoisted buffer); low traffic, deferred. +- **REPORT per-row `href` buffer** (`nc_href` allocates per row) — the ROUND20 + deferred href-buffer item; wants an `nc_href_into` + a precomputed encoded-user, + a separate alloc pass. +- **S3 read zero-copy forward** — a genuine framing tradeoff (fewer, larger + coalesced frames vs more, smaller zero-copy frames) that cannot be faithfully + benchmarked without a real S3/MinIO fixture; not shipped on synthetic evidence. +- **Frontend folder-listing cache / `/resources` ETag** — the real bandwidth win + needs a backend ETag on the listing feed + conditional 304, plus SWR wiring that + respects cursor pagination. A dedicated backend+frontend feature. + +## Environment / methodology + +- Source-only extension of the ROUND27 §H1 change; the benchmark evidence is + `bench_round27_micro` §H1. Verified: `cargo fmt --all --check` clean, + `cargo clippy --features bench -- -D warnings` clean, `cargo test --lib + --features bench` green. diff --git a/src/interfaces/nextcloud/report_handler.rs b/src/interfaces/nextcloud/report_handler.rs index 4f2cfdcd..5d1c6a76 100644 --- a/src/interfaces/nextcloud/report_handler.rs +++ b/src/interfaces/nextcloud/report_handler.rs @@ -24,7 +24,8 @@ use crate::interfaces::api::handlers::webdav_handler::{ }; use crate::interfaces::errors::AppError; use crate::interfaces::nextcloud::webdav_handler::{ - batch_resolve_ids, format_oc_id, nc_href, nc_id_of, write_file_response, write_folder_response, + batch_resolve_ids, format_oc_id_into, nc_href, nc_id_of, write_file_response, + write_folder_response, }; /// Handle WebDAV REPORT and SEARCH methods for Nextcloud compatibility. @@ -174,6 +175,8 @@ async fn handle_filter_files( // per type, not 2N round-trips). Hrefs use `url_user` so the // multi-drive `~{drive}` form is echoed back to the client; // owner-id stays canonical via `&user.username`. + // One oc:id buffer reused across both emit loops (benches/ROUND27.md §H1). + let mut oc_buf = String::new(); for file in &files { // Skip favorites that live outside the caller's chroot // (other-drive favorites); reachable via REST if needed. @@ -188,13 +191,19 @@ async fn handle_filter_files( }; let href = nc_href(url_user, subpath); let fid = nc_id_of(&file_id_map, &file.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; let dead = dead_props_for(&file.id, &file_deads); write_file_response( &mut xml, file, &href, - (fid, oc_id.as_deref()), + (fid, oc_id), &user.username, &favorite_ids, dead, @@ -214,13 +223,19 @@ async fn handle_filter_files( }; let href = format!("{}/", nc_href(url_user, subpath)); let fid = nc_id_of(&folder_id_map, &folder.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; let dead = dead_props_for(&folder.id, &folder_deads); write_folder_response( &mut xml, folder, &href, - (fid, oc_id.as_deref()), + (fid, oc_id), &user.username, &favorite_ids, // REPORT results are a flat filter/search listing, not a @@ -317,6 +332,8 @@ async fn handle_search( let folder_deads = folders_dead_props_map(&state.webdav_dead_props, &folders).await; // Files. + // One oc:id buffer reused across both emit loops (benches/ROUND27.md §H1). + let mut oc_buf = String::new(); for file in &files { let Some(subpath) = strip_home_prefix(chroot, &file.path, home_prefix) else { tracing::debug!( @@ -329,13 +346,19 @@ async fn handle_search( }; let href = nc_href(url_user, subpath); let fid = nc_id_of(&file_id_map, &file.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; let dead = dead_props_for(&file.id, &file_deads); write_file_response( &mut xml, file, &href, - (fid, oc_id.as_deref()), + (fid, oc_id), &user.username, &favorite_ids, dead, @@ -356,13 +379,19 @@ async fn handle_search( }; let href = format!("{}/", nc_href(url_user, subpath)); let fid = nc_id_of(&folder_id_map, &folder.id); - let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); + let oc_id: Option<&str> = match fid { + Some(id) => { + format_oc_id_into(&mut oc_buf, id, file_id_svc); + Some(oc_buf.as_str()) + } + None => None, + }; let dead = dead_props_for(&folder.id, &folder_deads); write_folder_response( &mut xml, folder, &href, - (fid, oc_id.as_deref()), + (fid, oc_id), &user.username, &favorite_ids, // REPORT results are a flat filter/search listing, not a