perf(cos): an object that can't be read is searched for once, not on every read

Document::try_get kept only objects that loaded. An object whose entry
points at something that doesn't parse falls back to a whole-file
search (scan_for); that failed too, nothing was kept, and the next read
searched the whole file again. A caller that reads such an object
repeatedly, such as the window title of a file whose document
information dictionary is damaged and that asks for its title to be
shown, paid one whole-file search per read, several per frame.

The cache now keeps the load result, a failure as well. It is the same
map with the same bounds, clears and per-reader privacy, so every place
that drops loaded objects (authentication, reconstruction, private scan
readers, eviction) drops remembered failures too. Edits are read from
the overlay before the cache, as before, so a set or freed number still
answers for itself. A failure is kept in nested loads too, as a success
already is; only a poisoned lock is never kept. Failures count towards
the cache's object caps and hold no bytes of their own. Sharing the map
between clones stays sound because entries, data and the stream limit
don't change after open.

Test: an_unreadable_object_is_searched_for_once counts whole-file
searches through a test-only counter (like SECTION_READS): one for
eleven reads and a clone's read. With failures not kept it counts 22.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9

crates/cos/src/document.rs

@@ -24,10 +24,10 @@ pub(crate) use reader::find;
 #[cfg(test)]
 use reader::rfind;
 pub(crate) use state::ObjectReader;
-use state::{ObjStm, Slot};
-pub use state::{ReadState, SourceIdentity};
+use state::{Loaded, ObjStm, Slot};
 #[cfg(test)]
-use state::{SECTION_READS, objstm_member, owned_bytes};
+use state::{OBJECT_SEARCHES, SECTION_READS, objstm_member, owned_bytes};
+pub use state::{ReadState, SourceIdentity};
 use xref::XrefIndex;
 pub use xref::{Revision, XrefEntry};
 
@@ -47,7 +47,9 @@ pub struct Document {
     /// `fill_xref_gaps`.
     located: Arc<BTreeMap<u32, (u64, u16)>>,
     repair_log: Arc<Vec<String>>,
-    cache: Arc<Mutex<HashMap<u32, Arc<Object>>>>,
+    /// What loading each file object gave, a failure as well: a damaged object is otherwise
+    /// searched for across the whole file on every read (`Document::load`).
+    cache: Arc<Mutex<HashMap<u32, Loaded>>>,
     objstms: Arc<Mutex<HashMap<u32, Arc<ObjStm>>>>,
     /// What `/JBIG2Globals` streams of the file decode to, by object number: the images of a scan
     /// share them, and each would otherwise decode them again (see `Document::jbig2_globals`).

crates/cos/src/document/objects.rs

@@ -1,5 +1,5 @@
 use super::reader::find;
-use super::state::{LENGTHS, LoadGuard, ObjStm, ObjectReader, ReadState, Slot, SourceIdentity, objstm_member, owned_bytes};
+use super::state::{LENGTHS, LoadGuard, Loaded, ObjStm, ObjectReader, ReadState, Slot, SourceIdentity, loaded_bytes, objstm_member};
 use super::xref::{Revision, XrefEntry};
 use super::{
     Arc, CosError, Cow, Dict, Document, Lexer, MAX_DECODED, MAX_JBIG2_GLOBALS, ObjRef, Object, Stream, is_whitespace, parse_indirect_shared,
@@ -388,20 +388,23 @@ impl Document {
             Some(Slot::Freed(_)) => return Ok(Arc::new(Object::Null)),
             None => {}
         }
-        if let Some(o) = self.cache.lock().map_err(|_| CosError::Poisoned)?.get(&num) {
-            return Ok(o.clone());
+        if let Some(loaded) = self.cache.lock().map_err(|_| CosError::Poisoned)?.get(&num) {
+            return loaded.clone();
         }
         // Loading can re-enter (indirect stream `/Length`, object streams); damaged files make
         // cycles such as `6 0 obj << /Length 6 0 R >>`. Bound the nesting on every path.
         let _guard =
             LoadGuard::enter(num).ok_or_else(|| CosError::Syntax { offset: 0, detail: format!("reference cycle while loading object {num}") })?;
-        let obj = self.load(num, 0)?;
-        let obj = Arc::new(obj);
-        // The editing cache is bounded like the scan reader's: re-parsing a file-backed view
-        // is cheap, and returned objects outlive eviction.
-        self.cache.lock().map_err(|_| CosError::Poisoned)?.insert(num, obj.clone());
-        self.evict_editor_cache_if_over_budget(num, &obj)?;
-        Ok(obj)
+        let loaded = self.load(num, 0).map(Arc::new);
+        // A failure is kept like an object, or every read of a damaged object searches the whole
+        // file again. A poisoned lock says nothing about the object.
+        if !matches!(loaded, Err(CosError::Poisoned)) {
+            // The editing cache is bounded like the scan reader's: re-parsing a file-backed view
+            // is cheap, and returned objects outlive eviction.
+            self.cache.lock().map_err(|_| CosError::Poisoned)?.insert(num, loaded.clone());
+            self.evict_editor_cache_if_over_budget(num, &loaded)?;
+        }
+        loaded
     }
 
     /// Bounds for the editing cache: roomier than the scan reader's (128 objects / 8 MB)
@@ -430,17 +433,17 @@ impl Document {
     /// byte budget can double.
     const EDITOR_LARGE_OBJECT: usize = 256 * 1024;
 
-    fn evict_editor_cache_if_over_budget(&self, fresh_num: u32, fresh: &Object) -> Result<(), CosError> {
+    fn evict_editor_cache_if_over_budget(&self, fresh_num: u32, fresh: &Loaded) -> Result<(), CosError> {
         // Fast path: the full fold is O(cache) per insert, so skip it for a small newcomer
         // below the count cap. `len` is copied out first; nothing here holds the lock.
         let len = self.cache.lock().map_err(|_| CosError::Poisoned)?.len();
-        if len <= Self::EDITOR_CACHE_OBJECTS && owned_bytes(fresh, &self.data, 0) <= Self::EDITOR_LARGE_OBJECT {
+        if len <= Self::EDITOR_CACHE_OBJECTS && loaded_bytes(fresh, &self.data) <= Self::EDITOR_LARGE_OBJECT {
             return Ok(());
         }
         // Walk a snapshot off-lock; re-lock only to evict.
-        let live: Vec<(u32, Arc<Object>)> = self.cache.lock().map_err(|_| CosError::Poisoned)?.iter().map(|(n, o)| (*n, o.clone())).collect();
+        let live: Vec<(u32, Loaded)> = self.cache.lock().map_err(|_| CosError::Poisoned)?.iter().map(|(n, o)| (*n, o.clone())).collect();
         let data = &self.data;
-        let owned: usize = live.iter().fold(0usize, |n, (_, o)| n.saturating_add(owned_bytes(o, data, 0)));
+        let owned: usize = live.iter().fold(0usize, |n, (_, o)| n.saturating_add(loaded_bytes(o, data)));
         if live.len() <= Self::EDITOR_CACHE_OBJECTS && owned <= Self::EDITOR_CACHE_BYTES {
             return Ok(());
         }
@@ -450,7 +453,7 @@ impl Document {
         let mut cache = self.cache.lock().map_err(|_| CosError::Poisoned)?;
         if cache.len() <= Self::EDITOR_CACHE_OBJECTS {
             let data = &self.data;
-            let owned: usize = cache.values().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, data, 0)));
+            let owned: usize = cache.values().fold(0usize, |n, o| n.saturating_add(loaded_bytes(o, data)));
             if owned <= Self::EDITOR_CACHE_BYTES {
                 return Ok(());
             }
@@ -602,6 +605,8 @@ impl Document {
 
     /// Last-resort lookup: scan the file for `num G obj`.
     fn scan_for(&self, num: u32) -> Option<(ObjRef, Object)> {
+        #[cfg(test)]
+        super::OBJECT_SEARCHES.with(|c| c.set(c.get() + 1));
         let needle = format!("{num} ");
         let data = &self.data;
         let mut found = None;

crates/cos/src/document/state.rs

@@ -30,6 +30,14 @@ impl LoadGuard {
     }
 }
 
+/// What loading a file object gave, kept in the document's cache either way.
+pub(super) type Loaded = Result<Arc<Object>, CosError>;
+
+/// The bytes a load result holds of its own ([`owned_bytes`]); a failure holds none.
+pub(super) fn loaded_bytes(loaded: &Loaded, data: &Arc<Vec<u8>>) -> usize {
+    loaded.as_ref().map_or(0, |o| owned_bytes(o, data, 0))
+}
+
 impl Drop for LoadGuard {
     fn drop(&mut self) {
         let outermost = LOADING.with(|l| {
@@ -165,7 +173,7 @@ impl ObjectReader {
         // have cached large objects.
         if let Ok(mut cache) = self.document.cache.lock() {
             let data = &self.document.data;
-            if cache.len() >= Self::OBJECTS || cache.values().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, data, 0))) > Self::STREAM_BYTES {
+            if cache.len() >= Self::OBJECTS || cache.values().fold(0usize, |n, o| n.saturating_add(loaded_bytes(o, data))) > Self::STREAM_BYTES {
                 cache.clear();
             }
         }
@@ -177,6 +185,8 @@ impl ObjectReader {
 thread_local! {
     /// Cross-reference sections read on this thread.
     pub(super) static SECTION_READS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
+    /// Whole-file searches for an object (`Document::scan_for`) on this thread.
+    pub(super) static OBJECT_SEARCHES: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
 }
 
 /// Nesting [`owned_bytes`] walks: past the parser's own nesting limit, so every parsed object

crates/cos/src/document/tests.rs

@@ -588,6 +588,31 @@ fn an_entry_that_misses_its_object_is_found_by_searching_but_is_not_exact() {
     assert!(doc.xref_entry_is_exact(1) && doc.xref_entry_is_exact(2));
 }
 
+/// An object that can't be read at its entry or anywhere else is searched for in the whole file
+/// once: every later read, in a clone too, gives the same error without searching again. An
+/// edit of that number still answers for it.
+#[test]
+fn an_unreadable_object_is_searched_for_once() {
+    let bodies = ["<< /Type /Catalog /Pages 2 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>", "<< /Odd Key(x) >>"];
+    let doc = Document::open(Arc::new(build(&bodies, "/Root 1 0 R /Info 3 0 R"))).unwrap();
+    // Counted wherever the whole-file search is done (`scan_for`).
+    OBJECT_SEARCHES.with(|c| c.set(0));
+    let first = doc.try_get(3);
+    assert!(first.is_err(), "{first:?}");
+    for _ in 0..10 {
+        assert_eq!(doc.try_get(3), first);
+        assert_eq!(*doc.get(ObjRef::new(3, 0)), Object::Null);
+    }
+    let mut edited = doc.clone();
+    assert_eq!(edited.try_get(3), first);
+    assert_eq!(OBJECT_SEARCHES.with(|c| c.get()), 1, "searches for an object that isn't there");
+    edited.set(ObjRef::new(3, 0), Object::Int(7));
+    assert_eq!(edited.try_get(3).as_deref(), Ok(&Object::Int(7)));
+    assert_eq!(doc.try_get(3), first, "the edit is the clone's alone");
+    edited.free(ObjRef::new(3, 0));
+    assert_eq!(edited.try_get(3).as_deref(), Ok(&Object::Null));
+}
+
 /// A classic table with `/Size` rows and object 0's free entry first, its subsection numbered
 /// `1 n` instead of `0 n` (#2676): every row is one number off.
 fn misnumbered_table(start: u32) -> Vec<u8> {
@@ -1109,7 +1134,7 @@ fn temporary_reader_keeps_file_views_and_evicts_data_of_its_own() {
     // Data of its own (decrypted streams, strings) counts: past the budget the cache is
     // cleared after the next read, including a failed one.
     let owned = Object::Stream(crate::Stream::from_raw(Dict::new(), vec![b'y'; ObjectReader::STREAM_BYTES + 1]));
-    reader.document.cache.lock().unwrap().insert(3, Arc::new(owned));
+    reader.document.cache.lock().unwrap().insert(3, Ok(Arc::new(owned)));
     assert_eq!(*reader.get(ObjRef::new(99, 0)), Object::Null);
     assert!(reader.document.cache.lock().unwrap().is_empty(), "owned data over budget is evicted");
 }
@@ -1198,8 +1223,8 @@ fn editor_cache_counts_deeply_nested_strings() {
         let Object::Ref(reference) = reference else { panic!("indirect object") };
         drop(doc.get(*reference));
         let cache = doc.cache.lock().unwrap();
-        let owned = cache.values().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, &doc.data, 0)));
-        let strings = cache.values().filter(|o| o.as_array().is_some()).count();
+        let owned = cache.values().flatten().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, &doc.data, 0)));
+        let strings = cache.values().flatten().filter(|o| o.as_array().is_some()).count();
         assert!(strings << 20 <= Document::EDITOR_CACHE_BYTES + (1 << 20), "{strings} MiB strings cached, {owned} bytes counted");
     }
 }