perf(round28): extend the PROPFIND oc:id reused buffer (round27 H1) to the REPORT emit loops
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L8gs91AhmazoxMsDcNk3KT
This commit is contained in:
@@ -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.
|
||||||
@@ -24,7 +24,8 @@ use crate::interfaces::api::handlers::webdav_handler::{
|
|||||||
};
|
};
|
||||||
use crate::interfaces::errors::AppError;
|
use crate::interfaces::errors::AppError;
|
||||||
use crate::interfaces::nextcloud::webdav_handler::{
|
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.
|
/// 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
|
// per type, not 2N round-trips). Hrefs use `url_user` so the
|
||||||
// multi-drive `~{drive}` form is echoed back to the client;
|
// multi-drive `~{drive}` form is echoed back to the client;
|
||||||
// owner-id stays canonical via `&user.username`.
|
// 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 {
|
for file in &files {
|
||||||
// Skip favorites that live outside the caller's chroot
|
// Skip favorites that live outside the caller's chroot
|
||||||
// (other-drive favorites); reachable via REST if needed.
|
// (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 href = nc_href(url_user, subpath);
|
||||||
let fid = nc_id_of(&file_id_map, &file.id);
|
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);
|
let dead = dead_props_for(&file.id, &file_deads);
|
||||||
write_file_response(
|
write_file_response(
|
||||||
&mut xml,
|
&mut xml,
|
||||||
file,
|
file,
|
||||||
&href,
|
&href,
|
||||||
(fid, oc_id.as_deref()),
|
(fid, oc_id),
|
||||||
&user.username,
|
&user.username,
|
||||||
&favorite_ids,
|
&favorite_ids,
|
||||||
dead,
|
dead,
|
||||||
@@ -214,13 +223,19 @@ async fn handle_filter_files(
|
|||||||
};
|
};
|
||||||
let href = format!("{}/", nc_href(url_user, subpath));
|
let href = format!("{}/", nc_href(url_user, subpath));
|
||||||
let fid = nc_id_of(&folder_id_map, &folder.id);
|
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);
|
let dead = dead_props_for(&folder.id, &folder_deads);
|
||||||
write_folder_response(
|
write_folder_response(
|
||||||
&mut xml,
|
&mut xml,
|
||||||
folder,
|
folder,
|
||||||
&href,
|
&href,
|
||||||
(fid, oc_id.as_deref()),
|
(fid, oc_id),
|
||||||
&user.username,
|
&user.username,
|
||||||
&favorite_ids,
|
&favorite_ids,
|
||||||
// REPORT results are a flat filter/search listing, not a
|
// 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;
|
let folder_deads = folders_dead_props_map(&state.webdav_dead_props, &folders).await;
|
||||||
|
|
||||||
// Files.
|
// Files.
|
||||||
|
// One oc:id buffer reused across both emit loops (benches/ROUND27.md §H1).
|
||||||
|
let mut oc_buf = String::new();
|
||||||
for file in &files {
|
for file in &files {
|
||||||
let Some(subpath) = strip_home_prefix(chroot, &file.path, home_prefix) else {
|
let Some(subpath) = strip_home_prefix(chroot, &file.path, home_prefix) else {
|
||||||
tracing::debug!(
|
tracing::debug!(
|
||||||
@@ -329,13 +346,19 @@ async fn handle_search(
|
|||||||
};
|
};
|
||||||
let href = nc_href(url_user, subpath);
|
let href = nc_href(url_user, subpath);
|
||||||
let fid = nc_id_of(&file_id_map, &file.id);
|
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);
|
let dead = dead_props_for(&file.id, &file_deads);
|
||||||
write_file_response(
|
write_file_response(
|
||||||
&mut xml,
|
&mut xml,
|
||||||
file,
|
file,
|
||||||
&href,
|
&href,
|
||||||
(fid, oc_id.as_deref()),
|
(fid, oc_id),
|
||||||
&user.username,
|
&user.username,
|
||||||
&favorite_ids,
|
&favorite_ids,
|
||||||
dead,
|
dead,
|
||||||
@@ -356,13 +379,19 @@ async fn handle_search(
|
|||||||
};
|
};
|
||||||
let href = format!("{}/", nc_href(url_user, subpath));
|
let href = format!("{}/", nc_href(url_user, subpath));
|
||||||
let fid = nc_id_of(&folder_id_map, &folder.id);
|
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);
|
let dead = dead_props_for(&folder.id, &folder_deads);
|
||||||
write_folder_response(
|
write_folder_response(
|
||||||
&mut xml,
|
&mut xml,
|
||||||
folder,
|
folder,
|
||||||
&href,
|
&href,
|
||||||
(fid, oc_id.as_deref()),
|
(fid, oc_id),
|
||||||
&user.username,
|
&user.username,
|
||||||
&favorite_ids,
|
&favorite_ids,
|
||||||
// REPORT results are a flat filter/search listing, not a
|
// REPORT results are a flat filter/search listing, not a
|
||||||
|
|||||||
Reference in New Issue
Block a user