From 582d491b2f8ae5fdc819f1649d716efa6095cbab Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 08:07:02 +0800 Subject: [PATCH] M3: a shared /QuadPoints array that no longer fits the budget is not read again Review on #4474: past the text budget every link still resolved and parsed its whole /QuadPoints array before dropping the result, so many links sharing one long array cost its length each. Now no array is read once less than one quadrilateral's bytes is left, and an indirect array that did not fit is remembered and skipped for later links (the budget only shrinks). A guard on the budget being exactly zero alone would rarely trigger, since URIs spend from the same budget. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/render/src/inspect/annotations.rs | 34 +++++++++++++++++++++--- crates/render/src/inspect/tests.rs | 6 +++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index b48c8537a6..e0acc5f2a4 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -62,6 +62,9 @@ impl<'a> Inspector<'a> { /// `text_budget` is what is left of [`MAX_LINK_ATTACHMENT_TEXT_BYTES`]. pub(super) fn annotations(&self, info: &mut DocInfo, text_budget: &mut usize) { + // Indirect `/QuadPoints` arrays whose quadrilaterals did not fit what was left of the budget: + // the budget only shrinks, so they are not read again (review on #4474). + let mut unfit_quads: HashSet = HashSet::new(); let mut pages: Vec<(ObjectId, usize)> = self.page_index.iter().map(|(&id, &page)| (id, page)).collect(); pages.sort_unstable_by_key(|&(_, page)| page); // An annotation, or a whole `/Annots` array, that several entries or pages list is on the @@ -103,7 +106,7 @@ impl<'a> Inspector<'a> { info.attachments.push(self.attachment(fs, AttachmentSource::Annotation { page, index }, text_budget, fallback)); } if subtype == "Link" { - if let Some(link) = self.link(page, index, d, info.uri_base.as_deref(), text_budget) { + if let Some(link) = self.link(page, index, d, info.uri_base.as_deref(), text_budget, &mut unfit_quads) { if matches!(&link.target, LinkTarget::Other(s) if s == "JavaScript") { info.has_javascript = true; } @@ -195,7 +198,16 @@ impl<'a> Inspector<'a> { } /// `base` is the catalog's `/URI /Base`, which relative URI actions are resolved against. - fn link(&self, page: usize, index: usize, d: &Dictionary, base: Option<&str>, text_budget: &mut usize) -> Option { + /// `unfit_quads` holds the indirect `/QuadPoints` arrays already found too large for the budget. + fn link( + &self, + page: usize, + index: usize, + d: &Dictionary, + base: Option<&str>, + text_budget: &mut usize, + unfit_quads: &mut HashSet, + ) -> Option { let rect = rect4(self.resolve(d.get(b"Rect").ok()?)); let target = if let Ok(dest) = d.get(b"Dest") { let (page, view) = self.dest(dest, 0)?; @@ -229,9 +241,16 @@ impl<'a> Inspector<'a> { None => return None, } }; + // Not read at all once no quadrilateral would fit what is left of the budget, or when this + // shared array already did not fit: many links can share one long array, and reading it + // again for each would cost its length every time (review on #4474). + let shared = d.get(b"QuadPoints").ok().and_then(|o| o.as_reference().ok()); + let readable = *text_budget >= std::mem::size_of::<[f32; 8]>() && shared.is_none_or(|id| !unfit_quads.contains(&id)); let quads = match d.get(b"QuadPoints").map(|o| self.resolve(o)) { // Read no further than one value past the cap: a longer array isn't used. - Ok(Object::Array(q)) if q.len() <= crate::MAX_LINK_QUADS.saturating_mul(8) => { + Ok(Object::Array(q)) if readable && q.len() <= crate::MAX_LINK_QUADS.saturating_mul(8) => { + #[cfg(test)] + QUAD_ARRAYS_READ.with(|n| n.set(n.get() + 1)); let values: Vec> = q.iter().map(|o| self.resolve(o).as_float().ok()).collect(); let quads = crate::link_quads(&values, rect); // Many links can share one long array, which each would otherwise copy. Past the @@ -241,6 +260,9 @@ impl<'a> Inspector<'a> { *text_budget = text_budget.saturating_sub(bytes); quads } else { + if let Some(id) = shared { + unfit_quads.insert(id); + } Vec::new() } } @@ -500,3 +522,9 @@ fn rect4(o: &Object) -> [f32; 4] { _ => [0.0; 4], } } + +#[cfg(test)] +thread_local! { + /// `/QuadPoints` arrays whose values were read. + pub(super) static QUAD_ARRAYS_READ: std::cell::Cell = const { std::cell::Cell::new(0) }; +} diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index fe5468f461..13bd9bf267 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1068,11 +1068,17 @@ fn links_sharing_quad_points_or_a_long_base_keep_them_within_a_budget() { let count = MAX_LINK_ATTACHMENT_TEXT_BYTES / (crate::MAX_LINK_QUADS * std::mem::size_of::<[f32; 8]>()) + 20; let quads = format!("[{}]", "10 10 60 10 60 30 10 30 ".repeat(crate::MAX_LINK_QUADS)); let links = "<< /Subtype /Link /Rect [0 0 100 100] /QuadPoints 5 0 R /A << /S /URI /URI (https://example.org/) >> >> ".repeat(count); + super::annotations::QUAD_ARRAYS_READ.with(|n| n.set(0)); let info = fill("", &links, &quads); assert_eq!(info.links.len(), count); let kept: usize = info.links.iter().map(|l| std::mem::size_of_val(l.quads.as_slice())).sum(); assert!(kept > 0 && kept <= MAX_LINK_ATTACHMENT_TEXT_BYTES, "{kept} quad bytes kept"); assert!(info.links.iter().any(|l| l.quads.is_empty()), "past the budget a link is active over its /Rect"); + // Review on #4474: the shared array is read once for each link it fits, and once more to find + // that it no longer does; not again for every link after that. + let fitted = info.links.iter().filter(|l| !l.quads.is_empty()).count(); + let read = super::annotations::QUAD_ARRAYS_READ.with(std::cell::Cell::get); + assert_eq!(read, fitted + 1, "{read} reads for {fitted} links within the budget, of {count}"); let base = format!("https://example.com/{}/", "b".repeat(crate::MAX_URI_BASE_BYTES - 32)); let count = MAX_LINK_ATTACHMENT_TEXT_BYTES / base.len() + 20;