From 6b993d9738b0f16f06b8cf13f37625890a6d0ef6 Mon Sep 17 00:00:00 2001 From: Ken Mantle Date: Wed, 11 Nov 2026 02:55:40 -0500 Subject: [PATCH] perf(organize): decode the XMP packet once for repeated document-information reads (#6918) `info` falls back to the catalog's XMP packet, and the tab and window title read it several times per frame when the document asks for its title to be shown. A packet (up to 16 MiB decoded) was inflated on every read. The last decoded packet is now kept per thread by the identity of its stream object: an edit or another document hands out another object, so it reads afresh. Co-authored-by: KenM76 Co-authored-by: Claude Opus 5.5 --- crates/organize/src/tests.rs | 32 +++++++++++++++++++++++++++++ crates/organize/src/xmp.rs | 40 +++++++++++++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index a04d9f2dcb..fa240f777f 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -689,6 +689,38 @@ fn document_info_falls_back_to_the_xmp_packet() { assert_eq!(info(&doc, "Subject"), None); } +/// The title read from XMP each frame (Initial View ▸ Show: Document Title) decodes the packet +/// once, not on every read; an edit of the packet, or another document, reads afresh. +#[test] +fn the_xmp_packet_is_decoded_once_for_repeated_reads() { + let decodes = || crate::xmp::XMP_DECODES.with(std::cell::Cell::get); + let mut doc = doc_with_xmp(LIBREOFFICE_XMP.as_bytes()); + doc.trailer_mut().remove(b"Info"); + let before = decodes(); + for _ in 0..10 { + assert_eq!(info(&doc, "Title").as_deref(), Some("Untitled 1")); + assert_eq!(info(&doc, "Author").as_deref(), Some("Old Author")); + } + assert_eq!(decodes() - before, 1); + set_info(&mut doc, "Title", "Report").unwrap(); + doc.trailer_mut().remove(b"Info"); + assert_eq!(info(&doc, "Title").as_deref(), Some("Report"), "the edited packet, not the one read before"); + assert_eq!(decodes() - before, 2); + let mut other = doc_with_xmp(LIBREOFFICE_XMP.replace("Untitled 1", "Other").as_bytes()); + other.trailer_mut().remove(b"Info"); + assert_eq!(info(&other, "Title").as_deref(), Some("Other"), "another document's packet"); + assert_eq!(info(&doc, "Title").as_deref(), Some("Report")); + let mut latin1 = LIBREOFFICE_XMP.as_bytes().to_vec(); + latin1.extend_from_slice(&[b'<', 0xE9, b'>']); + let mut bad = doc_with_xmp(&latin1); + bad.trailer_mut().remove(b"Info"); + let before = decodes(); + for _ in 0..10 { + assert_eq!(info(&bad, "Title"), None); + } + assert_eq!(decodes() - before, 1, "an unreadable packet is not decoded again either"); +} + #[test] fn document_info_edits_leave_files_without_usable_xmp_alone() { // No packet: none is added, for a custom property either. diff --git a/crates/organize/src/xmp.rs b/crates/organize/src/xmp.rs index 23eca1e1aa..0a828002e4 100644 --- a/crates/organize/src/xmp.rs +++ b/crates/organize/src/xmp.rs @@ -16,6 +16,9 @@ //! none, and a packet this can't edit safely (not UTF-8, no `rdf:RDF`, too large to decode) is //! left as it was. +use std::cell::RefCell; +use std::sync::{Arc, Weak}; + use pdfcraft_cos::{Document, Object, Stream}; use crate::OrganizeError; @@ -177,12 +180,43 @@ pub(crate) fn info_value(doc: &Document, key: &str) -> Option { let (name, _, shape) = property(key)?; let root = doc.root()?; let r = doc.get(root).as_dict()?.reference(b"Metadata")?; - let obj = doc.get(r); - let Object::Stream(stream) = &*obj else { return None }; - let xmp = String::from_utf8(stream.decoded_within(MAX_XMP).ok()?).ok()?; + let xmp = decoded_packet(&doc.get(r))?; get_property(&xmp, name, shape) } +/// A decoded packet (`None`: unreadable) and the stream object it came from. +type Held = (Weak, Option>); + +thread_local! { + /// The packet [`decoded_packet`] last decoded (`None`: unreadable), with the stream object it + /// came from. The tab title can come from XMP and is read every frame; a packet decodes to + /// up to [`MAX_XMP`] bytes. + static PACKET: RefCell> = const { RefCell::new(None) }; +} + +/// The text of the XMP stream `obj`, `None` when it isn't a stream or isn't UTF-8 within +/// [`MAX_XMP`]. Decoded once while the document keeps handing out the same object: an edit or +/// another document gives another one. The [`Weak`] keeps its address from being reused. +fn decoded_packet(obj: &Arc) -> Option> { + let same = |held: &Held| std::ptr::eq(held.0.as_ptr(), Arc::as_ptr(obj)); + if let Ok(Some(text)) = PACKET.try_with(|p| p.borrow().as_ref().filter(|held| same(held)).map(|held| held.1.clone())) { + return text; + } + let Object::Stream(stream) = &**obj else { return None }; + #[cfg(test)] + XMP_DECODES.with(|n| n.set(n.get() + 1)); + let text = stream.decoded_within(MAX_XMP).ok().and_then(|b| String::from_utf8(b).ok()).map(Arc::::from); + // Past the thread's end (a destructor reading it) the packet just isn't kept. + let _ = PACKET.try_with(|p| *p.borrow_mut() = Some((Arc::downgrade(obj), text.clone()))); + text +} + +#[cfg(test)] +thread_local! { + /// Packets [`decoded_packet`] decoded on this thread. + pub(crate) static XMP_DECODES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// The text of the property `name` in `xmp`, element or attribute form (see [`info_value`]). fn get_property(xmp: &str, name: &str, shape: Shape) -> Option { let value = if let Some((start, end)) = element_span(xmp, name) {