mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-12 05:42:52 +00:00
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 <KenM76> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2 files changed
+69
-3
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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<String> {
|
||||
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<Object>, Option<Arc<str>>);
|
||||
|
||||
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<Option<Held>> = 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<Object>) -> Option<Arc<str>> {
|
||||
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::<str>::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<usize> = 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<String> {
|
||||
let value = if let Some((start, end)) = element_span(xmp, name) {
|
||||
|
||||
Reference in new issue
Block a user