perf: round 6 backend — CardDAV cursor streaming, borrowed NC id chain, binary UUID decode, one-alloc hex
Benchmark-gated (equivalence + BEFORE/AFTER in examples/bench_*, results and reproduce commands in benches/ROUND6.md): - CardDAV whole-book REPORT + depth-1 PROPFIND stream through a PG cursor (stream_contacts_by_book, 500-contact pages) instead of materialising every vCard twice: 8 000 contacts TTFB 37.4 → 7.6 ms (4.9x), peak heap 19.0 → 7.0 MiB (2.7x), wall -23%; REPORT and PROPFIND byte-identical to the buffered writers. - NC numeric-id chain fully borrowed: get_or_create_file_ids/folder_ids take &[&str] and return HashMap<Uuid, i64>; batch_resolve_ids callers (PROPFIND pages, REPORT, trashbin, OCS search) pass id slices and look up via nc_id_of. 2.006 → 0.006 allocs/child (334x), 1.53x wall per 500-child page. batch_check_favorites binds &[&str] as text[]. - file_blob_read_repository listing SELECTs drop id::text/folder_id::text server casts: rows decode binary Uuid (16 vs 36 bytes on the wire) and render once in row_to_file. A/B on 500-row pages: 1.225 → 1.044 ms mean (1.17x), p95 1.686 → 1.345 (bench_uuid_text_cast; single-row, param and min() sites left as-is deliberately). - IncrementalHasher::finalize_hex renders through common::fmt::hex_lower instead of one format! per digest byte: 18 → 1 (md5) / 35 → 1 (sha256) allocs per chunk finalize, 14-15x wall. - Share landing overlaps the access-count UPDATE with the unlock fetch via tokio::join! (one round-trip off every public link hit). - REJECTED by benchmark and reverted: try_join_all fan-out of the batch-favorites authz pre-check — 42.6 → 56.4 ms cold, 0.15 → 0.23 ms warm against local-socket PG (bench_favorites_authz kept as evidence). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017aJu9ghvuT8WqC31ZEGTBA
This commit is contained in:
@@ -278,6 +278,27 @@ impl CardDavAdapter {
|
||||
) -> Result<()> {
|
||||
let mut xml_writer = Writer::new(writer);
|
||||
|
||||
Self::write_collection_head(&mut xml_writer, address_book, request, base_href)?;
|
||||
|
||||
// Write contacts if depth > 0
|
||||
if depth != "0" {
|
||||
Self::write_collection_contact_page(&mut xml_writer, contacts, base_href)?;
|
||||
}
|
||||
|
||||
Self::write_carddav_multistatus_end(&mut xml_writer)
|
||||
}
|
||||
|
||||
/// Multistatus opening (DAV + CardDAV + CalendarServer namespaces)
|
||||
/// plus the address book's own `D:response` — the head of a depth-1
|
||||
/// collection PROPFIND. Streaming emitters call this once, then
|
||||
/// [`Self::write_collection_contact_page`] per cursor page, then
|
||||
/// [`Self::write_carddav_multistatus_end`].
|
||||
pub fn write_collection_head<W: Write>(
|
||||
xml_writer: &mut Writer<W>,
|
||||
address_book: &AddressBookDto,
|
||||
request: &PropFindRequest,
|
||||
base_href: &str,
|
||||
) -> Result<()> {
|
||||
xml_writer.write_event(Event::Start(
|
||||
BytesStart::new("D:multistatus").with_attributes([
|
||||
("xmlns:D", "DAV:"),
|
||||
@@ -285,19 +306,25 @@ impl CardDavAdapter {
|
||||
("xmlns:CS", "http://calendarserver.org/ns/"),
|
||||
]),
|
||||
))?;
|
||||
Self::write_addressbook_response(xml_writer, address_book, request, base_href)
|
||||
}
|
||||
|
||||
// Write the address book itself
|
||||
Self::write_addressbook_response(&mut xml_writer, address_book, request, base_href)?;
|
||||
|
||||
// Write contacts if depth > 0
|
||||
if depth != "0" {
|
||||
for contact in contacts {
|
||||
let contact_href = format!("{}{}.vcf", base_href, contact.uid);
|
||||
Self::write_contact_response(&mut xml_writer, contact, &[], &contact_href)?;
|
||||
}
|
||||
/// One depth-1 collection page of contact entries (standard props;
|
||||
/// href buffer reused across the page).
|
||||
pub fn write_collection_contact_page<W: Write>(
|
||||
xml_writer: &mut Writer<W>,
|
||||
contacts: &[ContactDto],
|
||||
base_href: &str,
|
||||
) -> Result<()> {
|
||||
let mut href = String::with_capacity(base_href.len() + 48);
|
||||
for contact in contacts {
|
||||
href.clear();
|
||||
let _ = std::fmt::Write::write_fmt(
|
||||
&mut href,
|
||||
format_args!("{}{}.vcf", base_href, contact.uid),
|
||||
);
|
||||
Self::write_contact_response(xml_writer, contact, &[], &href)?;
|
||||
}
|
||||
|
||||
xml_writer.write_event(Event::End(BytesEnd::new("D:multistatus")))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -648,32 +675,39 @@ impl CardDavAdapter {
|
||||
}
|
||||
|
||||
/// Generate response for contacts (for REPORT)
|
||||
pub fn generate_contacts_response<W: Write>(
|
||||
writer: W,
|
||||
contacts: &[ContactDto],
|
||||
report: &CardDavReportType,
|
||||
base_href: &str,
|
||||
) -> Result<()> {
|
||||
let mut xml_writer = Writer::new(writer);
|
||||
|
||||
/// REPORT `<D:multistatus>` opening tag (DAV + CardDAV namespaces).
|
||||
/// Streaming emitters call this once, then
|
||||
/// [`Self::write_contacts_report_page`] per cursor page, then
|
||||
/// [`Self::write_carddav_multistatus_end`].
|
||||
pub fn write_report_multistatus_start<W: Write>(xml_writer: &mut Writer<W>) -> Result<()> {
|
||||
xml_writer.write_event(Event::Start(
|
||||
BytesStart::new("D:multistatus").with_attributes([
|
||||
("xmlns:D", "DAV:"),
|
||||
("xmlns:CR", "urn:ietf:params:xml:ns:carddav"),
|
||||
]),
|
||||
))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// Borrowed straight out of the request — the old `clone()` copied
|
||||
// the whole Vec of owned QualifiedName strings per REPORT (same
|
||||
// fix the CalDAV surface got in ROUND4).
|
||||
/// Close a multistatus opened by either start writer.
|
||||
pub fn write_carddav_multistatus_end<W: Write>(xml_writer: &mut Writer<W>) -> Result<()> {
|
||||
xml_writer.write_event(Event::End(BytesEnd::new("D:multistatus")))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// One REPORT page of contact responses. Props are borrowed from
|
||||
/// the request; one href buffer is reused across the page.
|
||||
pub fn write_contacts_report_page<W: Write>(
|
||||
xml_writer: &mut Writer<W>,
|
||||
contacts: &[ContactDto],
|
||||
report: &CardDavReportType,
|
||||
base_href: &str,
|
||||
) -> Result<()> {
|
||||
let props = match report {
|
||||
CardDavReportType::AddressbookQuery { props } => props,
|
||||
CardDavReportType::AddressbookMultiget { props, .. } => props,
|
||||
CardDavReportType::SyncCollection { props, .. } => props,
|
||||
};
|
||||
|
||||
// One reused href buffer for the whole listing instead of a
|
||||
// fresh String per contact.
|
||||
let mut href = String::with_capacity(base_href.len() + 48);
|
||||
for contact in contacts {
|
||||
href.clear();
|
||||
@@ -683,13 +717,23 @@ impl CardDavAdapter {
|
||||
);
|
||||
// `write_contact_response` generates the vCard on demand when (and
|
||||
// only when) address-data is actually requested.
|
||||
Self::write_contact_response(&mut xml_writer, contact, props, &href)?;
|
||||
Self::write_contact_response(xml_writer, contact, props, &href)?;
|
||||
}
|
||||
|
||||
xml_writer.write_event(Event::End(BytesEnd::new("D:multistatus")))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn generate_contacts_response<W: Write>(
|
||||
writer: W,
|
||||
contacts: &[ContactDto],
|
||||
report: &CardDavReportType,
|
||||
base_href: &str,
|
||||
) -> Result<()> {
|
||||
let mut xml_writer = Writer::new(writer);
|
||||
Self::write_report_multistatus_start(&mut xml_writer)?;
|
||||
Self::write_contacts_report_page(&mut xml_writer, contacts, report, base_href)?;
|
||||
Self::write_carddav_multistatus_end(&mut xml_writer)
|
||||
}
|
||||
|
||||
/// Write a single contact response element
|
||||
fn write_contact_response<W: Write>(
|
||||
xml_writer: &mut Writer<W>,
|
||||
|
||||
@@ -66,6 +66,12 @@ pub trait ContactStoragePort: Send + Sync + 'static {
|
||||
&self,
|
||||
address_book_id: &Uuid,
|
||||
) -> Result<Vec<Contact>, DomainError>;
|
||||
/// Cursor stream over the book's contacts in listing order — feeds
|
||||
/// the streaming CardDAV emitters.
|
||||
fn stream_contacts_by_book(
|
||||
&self,
|
||||
address_book_id: Uuid,
|
||||
) -> futures::stream::BoxStream<'static, Result<Contact, DomainError>>;
|
||||
async fn get_contacts_by_address_book_paginated(
|
||||
&self,
|
||||
address_book_id: &Uuid,
|
||||
@@ -174,6 +180,15 @@ pub trait ContactUseCase: Send + Sync + 'static {
|
||||
/// List contacts in an address book. `limit`/`offset` bound the
|
||||
/// result for paginated callers (REST API); `None` returns the full
|
||||
/// book, which the CardDAV listing/sync paths rely on.
|
||||
/// Streaming support: cursor over the book's contacts (same Read
|
||||
/// gate as [`Self::list_contacts`], checked once before the cursor
|
||||
/// opens).
|
||||
async fn stream_contacts_by_book(
|
||||
&self,
|
||||
address_book_id: &str,
|
||||
user_id: Uuid,
|
||||
) -> Result<futures::stream::BoxStream<'static, Result<ContactDto, DomainError>>, DomainError>;
|
||||
|
||||
async fn list_contacts(
|
||||
&self,
|
||||
address_book_id: &str,
|
||||
|
||||
@@ -825,6 +825,25 @@ impl ContactUseCase for ContactService {
|
||||
Ok(contacts.into_iter().map(ContactDto::from).collect())
|
||||
}
|
||||
|
||||
async fn stream_contacts_by_book(
|
||||
&self,
|
||||
address_book_id: &str,
|
||||
user_id: Uuid,
|
||||
) -> Result<futures::stream::BoxStream<'static, Result<ContactDto, DomainError>>, DomainError>
|
||||
{
|
||||
use futures::StreamExt;
|
||||
let id = Uuid::parse_str(address_book_id)
|
||||
.map_err(|_| DomainError::validation_error("Invalid address book ID format"))?;
|
||||
// Same Read gate as `list_contacts`, once, before the cursor.
|
||||
self.require_address_book_read_or_public(&id, &user_id)
|
||||
.await?;
|
||||
Ok(Box::pin(
|
||||
self.contact_storage
|
||||
.stream_contacts_by_book(id)
|
||||
.map(|r| r.map(ContactDto::from)),
|
||||
))
|
||||
}
|
||||
|
||||
async fn list_contacts(
|
||||
&self,
|
||||
address_book_id: &str,
|
||||
|
||||
@@ -145,6 +145,12 @@ impl FavoritesUseCase for FavoritesService {
|
||||
// valid (partial success would leak the same oracle we
|
||||
// closed on the single-item path). See
|
||||
// `docs/plan/authz_audit/rest_storage.md`.
|
||||
//
|
||||
// Deliberately serial: a `try_join_all` fan-out measured WORSE
|
||||
// on both the cold (drive_of point-SELECTs) and warm (all-moka)
|
||||
// paths — future orchestration + pool-acquire contention cost
|
||||
// more than the local round trips they overlap. Rejected by
|
||||
// `bench_favorites_authz`; numbers in benches/ROUND6.md.
|
||||
for (item_id, item_type) in items {
|
||||
let resource = Resource::parse(item_type, item_id)?;
|
||||
self.authorization
|
||||
|
||||
@@ -41,55 +41,50 @@ impl NextcloudFileIdService {
|
||||
|
||||
/// Resolve — creating when absent — stable numeric file IDs for many
|
||||
/// UUIDs at once. Cache hits cost nothing; the misses are resolved with a
|
||||
/// single backing query. The returned map is keyed by the caller's
|
||||
/// original id strings; unresolvable inputs are simply absent (mirroring
|
||||
/// the `.ok()` behaviour the callers relied on).
|
||||
pub async fn get_or_create_file_ids(
|
||||
&self,
|
||||
file_ids: &[String],
|
||||
) -> Result<HashMap<String, i64>> {
|
||||
/// single backing query. The returned map is keyed by parsed UUID;
|
||||
/// unparseable/unresolvable inputs are simply absent (mirroring the
|
||||
/// `.ok()` behaviour the callers relied on).
|
||||
pub async fn get_or_create_file_ids(&self, file_ids: &[&str]) -> Result<HashMap<Uuid, i64>> {
|
||||
self.get_or_create_many("file", file_ids).await
|
||||
}
|
||||
|
||||
/// Folder counterpart of [`Self::get_or_create_file_ids`].
|
||||
pub async fn get_or_create_folder_ids(
|
||||
&self,
|
||||
folder_ids: &[String],
|
||||
) -> Result<HashMap<String, i64>> {
|
||||
folder_ids: &[&str],
|
||||
) -> Result<HashMap<Uuid, i64>> {
|
||||
self.get_or_create_many("folder", folder_ids).await
|
||||
}
|
||||
|
||||
async fn get_or_create_many(
|
||||
&self,
|
||||
object_type: &str,
|
||||
raw_ids: &[String],
|
||||
) -> Result<HashMap<String, i64>> {
|
||||
raw_ids: &[&str],
|
||||
) -> Result<HashMap<Uuid, i64>> {
|
||||
let mut result = HashMap::with_capacity(raw_ids.len());
|
||||
// Parsed-UUID → caller's original string; also dedupes the miss list.
|
||||
let mut pending: HashMap<Uuid, String> = HashMap::new();
|
||||
let mut misses: Vec<Uuid> = Vec::new();
|
||||
|
||||
for raw in raw_ids {
|
||||
let Ok(uuid) = Uuid::parse_str(raw) else {
|
||||
continue; // Unparseable ids never had a mapping — skip silently.
|
||||
};
|
||||
if let Some(id) = self.cache.get(&uuid).await {
|
||||
result.insert(raw.clone(), id);
|
||||
result.insert(uuid, id);
|
||||
} else {
|
||||
pending.entry(uuid).or_insert_with(|| raw.clone());
|
||||
misses.push(uuid);
|
||||
}
|
||||
}
|
||||
|
||||
if !pending.is_empty() {
|
||||
let misses: Vec<Uuid> = pending.keys().copied().collect();
|
||||
if !misses.is_empty() {
|
||||
misses.sort_unstable();
|
||||
misses.dedup();
|
||||
let resolved = self
|
||||
.repo()?
|
||||
.get_or_create_many(object_type, &misses)
|
||||
.await?;
|
||||
for (uuid, id) in resolved {
|
||||
self.cache.insert(uuid, id).await;
|
||||
if let Some(original) = pending.get(&uuid) {
|
||||
result.insert(original.clone(), id);
|
||||
}
|
||||
result.insert(uuid, id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -184,10 +179,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn test_get_or_create_file_ids_skips_unparseable() {
|
||||
let svc = NextcloudFileIdService::new_stub();
|
||||
let map = svc
|
||||
.get_or_create_file_ids(&["not-a-uuid".to_string()])
|
||||
.await
|
||||
.unwrap();
|
||||
let map = svc.get_or_create_file_ids(&["not-a-uuid"]).await.unwrap();
|
||||
assert!(map.is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user