From 5e2552dcb0e7cef8a24f5e930f4b13a3ddd14a9a Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Tue, 10 Nov 2026 22:18:19 +0100 Subject: [PATCH] M8: named property lists and their MCIDs are read once for a stream The tags cleanup resolved the page's `/Properties` at every named `BDC`, on main too: 20,000 of them with 5,000 entries took 28 s, as measured in review (optimized), against 0.07 s with one entry. And the second commit's `MCID` by reference was read again at every `BDC`: the document keeps an object larger than its cache only until it reads another, so reading the `MCID` evicted a property list that large, which was parsed again, all of it, at the next `BDC` (50 `BDC` took 12.3 s against 0.44 s with the `MCID` given directly). Now the named property lists are read once for the stream, in place when they are direct, and each name's `MCID` once. Tests (`crates/redact/tests/operator_work.rs`, a binary of its own whose tests run one at a time), each the least of three runs against one entry or operator, allowing ten times as long plus a second: `named_property_lists_are_read_once_for_a_page` (5,000 `BDC`, 2,000 entries) and `an_oversized_property_list_is_read_once_for_a_stream` (100 `BDC` naming a list too large to keep). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/tags.rs | 22 +++++-- crates/redact/tests/operator_work.rs | 97 ++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 4 deletions(-) create mode 100644 crates/redact/tests/operator_work.rs diff --git a/crates/redact/src/tags.rs b/crates/redact/src/tags.rs index 15096433d5..03573a9743 100644 --- a/crates/redact/src/tags.rs +++ b/crates/redact/src/tags.rs @@ -35,6 +35,14 @@ impl Marked { /// left are taken as changed (their descriptions go) rather than compared. const MAX_COMPARED: usize = 1 << 28; +/// `f` of `o`, or of the object it refers to, without copying a direct one. +fn with_held(doc: &Document, o: &Object, f: impl FnOnce(&Object) -> R) -> R { + match o { + Object::Ref(_) => f(&doc.resolve(o)), + o => f(o), + } +} + fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> Marked { let mut out = Marked::default(); // Open sequences: the MCID with where its content starts in `ops` and the drawing operators @@ -49,16 +57,22 @@ fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> Marked { e.1 |= drawn > drawn_before; } }; + // The named property lists, read once, and each name's MCID, read once: a stream can name one + // at every `BDC` of thousands, and reading an object again can parse it again, all of it (the + // document keeps an object larger than its cache only until it reads another). A direct one + // is read where it is. + let held = resources.get(b"Properties").filter(|p| matches!(p, Object::Ref(_))).map(|p| doc.resolve(p)); + let named = held.as_deref().or_else(|| resources.get(b"Properties")).and_then(Object::as_dict); + let mut mcids: HashMap, Option> = HashMap::new(); for op in pdfcraft_content::parse(data).ops { match op.op.as_slice() { b"BDC" => { // The property list is the last operand, as the renderer reads it. let id = match op.back(0) { - Some(Object::Name(n)) => resources.get(b"Properties").and_then(|p| { - let props = doc.resolve(p); - let prop = doc.resolve(props.as_dict()?.get(n)?); + Some(Object::Name(n)) => *mcids.entry(n.to_vec()).or_insert_with(|| { + let prop = named?.get(n)?; // A named property list may give its MCID by reference. - prop.as_dict()?.get(b"MCID").and_then(|id| doc.resolve(id).as_int()) + with_held(doc, prop, |prop| prop.as_dict()?.get(b"MCID").and_then(|id| with_held(doc, id, Object::as_int))) }), Some(p) => doc.resolve(p).as_dict().and_then(|d| d.int(b"MCID")), _ => None, diff --git a/crates/redact/tests/operator_work.rs b/crates/redact/tests/operator_work.rs new file mode 100644 index 0000000000..95bf8e33d5 --- /dev/null +++ b/crates/redact/tests/operator_work.rs @@ -0,0 +1,97 @@ +//! Work regressions for what a redaction looks up at operators a stream can hold thousands of: the +//! tags cleanup's named property lists at each `BDC` are not read whole each time. A test binary of +//! its own, so that none of the crate's other tests shares its process, and its tests run one at a +//! time. Each test compares two timings of its own, the least of three runs each, with many +//! entries or operators and with one, not a fixed time. The bound leaves room for slower machines: +//! each regression took seconds against milliseconds. + +use std::sync::{Arc, Mutex, PoisonError}; +use std::time::{Duration, Instant}; + +use pdfcraft_annot::{Meta, NewAnnotation, Shape, Style, add_annotation, rect_quad}; +use pdfcraft_cos::Document; +use pdfcraft_redact::apply; +use pdfcraft_testkit::{Pdf, stream_obj}; + +/// One page with `resources` and `extra` objects (from object 5 on), drawing a word under a mark +/// (so the redaction rewrites the page) and then `ops`. +fn page(resources: &str, ops: &[u8], extra: Vec>) -> Document { + let content = [b"BT /F1 10 Tf 20 260 Td (SECRET) Tj ET ".as_slice(), ops].concat(); + let mut objs = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + format!( + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 << /Type /Font /Subtype /Type1 /BaseFont /Helvetica >> >> {resources} >> >>" + ) + .into_bytes(), + stream_obj("", content), + ]; + objs.extend(extra); + let mut doc = Document::open(Arc::new(Pdf::new().objs(objs).build())).unwrap(); + let shape = Shape::Redact { quads: vec![rect_quad([0.0, 200.0, 300.0, 300.0])], overlay: String::new(), look: Default::default() }; + let style = Style::default_for(&shape); + add_annotation(&mut doc, &NewAnnotation { page: 0, shape, style, contents: String::new(), author: "Synthetic test".into() }, &Meta::default()) + .unwrap(); + doc +} + +/// One page whose `/Properties` holds `entries` entries, drawing `ops`. +fn properties(entries: usize, ops: &[u8]) -> Document { + let lists: String = (0..entries).map(|i| format!("/M{i} << /Lang (en) >> ")).collect(); + page(&format!("/Properties << {lists}>>"), ops, Vec::new()) +} + +/// One page whose property list `/M0` is object 5, which gives its `MCID` as object 6 and holds +/// 400,000 more entries: larger than the document keeps, so that reading it again parses it again, +/// all of it, once another object has been read since. The page draws `ops`. +fn oversized(ops: &[u8]) -> Document { + let more: String = (0..400_000).map(|i| format!("/J{i} /DeviceGray ")).collect(); + page("/Properties << /M0 5 0 R >>", ops, vec![format!("<< /MCID 6 0 R {more}>>").into_bytes(), b"0".to_vec()]) +} + +/// The tests' lock: they run one at a time, so that none is timed while another works. +static SERIAL: Mutex<()> = Mutex::new(()); + +fn one_at_a_time() -> std::sync::MutexGuard<'static, ()> { + SERIAL.lock().unwrap_or_else(PoisonError::into_inner) +} + +/// How long redacting a document `make` builds takes, the least of three runs. +fn redaction(make: impl Fn() -> Document) -> Duration { + (0..3) + .map(|_| { + let mut doc = make(); + let start = Instant::now(); + let report = apply(&mut doc, None).unwrap(); + let took = start.elapsed(); + assert_eq!(report.glyphs, 6, "the word under the mark goes"); + took + }) + .min() + .unwrap_or_default() +} + +/// Whether `many` took about as long as `one`. +fn about_as_long(many: Duration, one: Duration) -> bool { + many < one * 10 + Duration::from_secs(1) +} + +/// Written when the tags cleanup resolved the page's `/Properties` at every named `BDC`: 20,000 of +/// them with 5,000 entries took 28 s against 0.07 s with one entry (optimized). +#[test] +fn named_property_lists_are_read_once_for_a_page() { + let _serial = one_at_a_time(); + let ops = b"/P /M0 BDC EMC ".repeat(5_000); + let (one, many) = (redaction(|| properties(1, &ops)), redaction(|| properties(2_000, &ops))); + assert!(about_as_long(many, one), "2,000 entries {many:?}, one {one:?}"); +} + +/// Written when the tags cleanup read a named property list's `MCID` by reference at each `BDC`: a +/// list larger than the document keeps was parsed again at each, 50 of them taking 12.3 s against +/// 0.44 s with the `MCID` given directly (optimized). +#[test] +fn an_oversized_property_list_is_read_once_for_a_stream() { + let _serial = one_at_a_time(); + let (one, many) = (redaction(|| oversized(b"/P /M0 BDC EMC ")), redaction(|| oversized(&b"/P /M0 BDC EMC ".repeat(100)))); + assert!(about_as_long(many, one), "100 operators {many:?}, one {one:?}"); +}