From f5aa20f7a9e815583951ff2980446fab2c516dd6 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 14:03:42 +0100 Subject: [PATCH] M8: a shared parameters value is remembered by the object it ends at Names kept what a value names by its first reference, so streams naming one array each through an object of their own that refers to it missed the memo every time: quadratic time, and now memory too (8000 such streams with 8000 entries, 1.4 MB: 30 s, 504 MiB). It is keyed by the object the references end at, so another reference, or a chain, finds it (the same file: 0.29 s, 7 MiB, measured by the review). matches and added use the same key. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 21 ++++++++++++++++----- crates/redact/src/tests.rs | 17 +++++++++++++---- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 735bbc3667..c8b91f248a 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -23,19 +23,28 @@ pub(crate) fn kept(doc: &Document) -> HashSet { /// `o`, or what references lead to from it. A ring of them ends as a reference, which is no dictionary. pub(crate) fn followed(doc: &Document, o: &Object) -> Arc { + followed_to(doc, o).0 +} + +/// [`followed`], and the last reference on the way: the one that names what it ends at, whichever +/// reference or chain of them led there. `None` when `o` was given directly. +fn followed_to(doc: &Document, o: &Object) -> (Arc, Option) { + let mut last = o.as_ref(); let mut at = doc.resolve(o); for _ in 0..MAX_HOPS { let next = match &*at { Object::Ref(next) => *next, _ => break, }; + last = Some(next); at = doc.get(next); } - at + (at, last) } /// What a parameters value names as `/JBIG2Globals`, and the key it is remembered by: a value the -/// file refers to, by its object number and whether it is read as a list of entries. +/// file refers to, by the number of the object the references end at and whether it is read as a +/// list of entries. pub(crate) type Listed = (Rc<[u32]>, Option<(u32, bool)>); /// Every object a stream names as `/JBIG2Globals`, and each reference on the way to one: in any entry @@ -43,7 +52,7 @@ pub(crate) type Listed = (Rc<[u32]>, Option<(u32, bool)>); /// reader may use less of this, but leaving a dictionary behind is the failure that matters here. /// /// What a value the file refers to names is read once a run, so streams that share one, by -/// reference or through an entry, cost it once however many there are. +/// reference, through a chain of them or through an entry, cost it once however many there are. #[derive(Default)] pub(crate) struct Names { memo: HashMap<(u32, bool), Rc<[u32]>>, @@ -60,12 +69,14 @@ impl Names { /// What `v` names: one entry, or (`list`) an array of them or one. fn value(&mut self, doc: &Document, v: &Object, list: bool) -> Listed { - let key = v.as_ref().map(|r| (r.num, list)); + // By the object the references end at: another reference to it, or a chain, finds it too. + let (held, last) = followed_to(doc, v); + let key = last.map(|r| (r.num, list)); if let Some(hit) = key.and_then(|k| self.memo.get(&k)) { return (hit.clone(), key); } self.read += 1; - let names: Rc<[u32]> = match (&*followed(doc, v), list) { + let names: Rc<[u32]> = match (&*held, list) { (Object::Array(entries), true) => { let mut all: Vec = entries.iter().flat_map(|e| self.value(doc, e, false).0.to_vec()).collect(); all.sort_unstable(); diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index dd1d9b4b43..e7e7854021 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1836,17 +1836,26 @@ fn what_is_made_direct_is_small_however_often_a_large_object_is_named() { assert_eq!(made.dict.get(b"Filter").and_then(Object::as_array).map(Vec::len), Some(9)); } -/// What streams share by reference is read once a run, however many share it: a parameters array -/// of 5,000 entries named by 100 streams costs the array and its one entry, once. +/// What streams share by reference is read once a run, however many share it and however they +/// refer to it: a parameters array of 5,000 entries named by 100 streams, directly, each through an +/// object of its own that refers to it, or through a chain of two, costs the array and its one +/// entry, once. #[test] fn a_shared_parameters_array_is_read_once_however_many_streams_share_it() { let entries = " 4 0 R".repeat(5000); let mut objects = vec![b"<< /Type /Catalog >>".to_vec(), format!("[{entries}]").into_bytes(), stream("", b"symbols"), b"<< /JBIG2Globals 3 0 R >>".to_vec()]; - objects.extend((0..100).map(|_| stream("/Filter /JBIG2Decode /DecodeParms 2 0 R", b""))); + // Objects 5 to 54 refer to the array, and 55 to 5. + objects.extend((0..50).map(|_| b"2 0 R".to_vec())); + objects.push(b"5 0 R".to_vec()); + // Streams 56 to 155: fifty name the array directly, forty-nine through objects 5 to 53, one + // through the chain from 55. + objects.extend((0..50).map(|_| stream("/Filter /JBIG2Decode /DecodeParms 2 0 R", b""))); + objects.extend((5..54).map(|n| stream(&format!("/Filter /JBIG2Decode /DecodeParms {n} 0 R"), b""))); + objects.push(stream("/Filter /JBIG2Decode /DecodeParms 55 0 R", b"")); let doc = pdf(objects); let mut names = crate::globals::Names::default(); - for n in 5..105 { + for n in 56..156 { let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") }; let lists = names.of(&doc, s); assert_eq!(lists.iter().map(|(list, _)| list.to_vec()).collect::>(), [vec![3]]);