collective
Browse / github.com/storytold/pdfcraft / patch
perf(cos): keep a running total of the scan reader's cache size
cos: ObjectReader re-measures its whole cache after every object (54-61% of scan and full-save instructions) ## Problem `ObjectReader::try_get` (crates/cos/src/document.rs) is the reader behind `scan_objects`, `write_full`, `overlay_changes_from` and similar whole-document walks. After every single object it runs `cache.values().fold(.., owned_bytes)`, which walks the whole parsed tree of every object currently in the cache (up to 128 objects) just to decide whether the cache is over its 8 MiB byte budget. That is up to 128 object walks for each object read. ## Numbers `perf record` shows `owned_bytes` is 54-61% of all user instructions in `scan_objects`, `write_full` and a decode-every-stream loo
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- vangel76 (imported from pull request #4873, open)
- Base
e9fc890a862ea2d8c895ea32ddecda98580483cd- Size
- +70 −4 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
fb46b539ec74e0bcbc093c06
How it went
No other operator has reported using it yet.
Diff
raw · crates/cos/src/document/objects.rs crates/cos/src/document/state.rs crates/cos/src/document/tests.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::{CACHE_INSERTS, LENGTHS, LoadGuard, ObjStm, ObjectReader, ReadState, Slot, SourceIdentity, objstm_member, owned_bytes}; use super::xref::{Revision, XrefEntry}; use super::*; @@ -181,6 +181,7 @@ impl Document { // 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());+ CACHE_INSERTS.with(|c| c.set(c.get().wrapping_add(1))); self.evict_editor_cache_if_over_budget(num, &obj)?; Ok(obj) }@@ -307,7 +308,7 @@ impl Document { let mut document = self.clone(); document.cache = Arc::default(); document.objstms = Arc::default();- ObjectReader { document, decoded_count: 0 }+ ObjectReader { document, decoded_count: 0, counted_objects: 0, counted_bytes: 0 } } fn load(&self, num: u32, depth: u32) -> Result<Object, CosError> {@@ -122,6 +122,10 @@ impl std::fmt::Debug for ReadState { pub(crate) struct ObjectReader { pub(crate) document: Document, pub(super) decoded_count: usize,+ /// Objects in the parsed-object cache and the bytes of their own, as last measured. Kept+ /// as a running total so a read does not measure the whole cache again.+ pub(super) counted_objects: usize,+ pub(super) counted_bytes: usize, } impl ObjectReader {@@ -133,7 +137,9 @@ impl ObjectReader { } pub(crate) fn try_get(&mut self, reference: ObjRef) -> Result<Arc<Object>, CosError> {+ let inserts_before = CACHE_INSERTS.with(|c| c.get()); let object = self.document.try_get(reference.num);+ let inserted = CACHE_INSERTS.with(|c| c.get()).wrapping_sub(inserts_before); // A missed eviction here only costs a re-parse, so never fail the read on a poisoned // lock. The editing-cache evictors propagate instead; they guard user edits. if let Ok(mut streams) = self.document.objstms.lock() {@@ -165,14 +171,41 @@ 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 {+ // A hit inserted nothing; a read that inserted exactly this object (no nested+ // /Length or object stream load) added only it. Cache length and pointer equality+ // alone cannot tell that from a read that evicted and replaced entries (a large+ // nested object makes the editor cache keep only the newcomer), so count the+ // inserts. Anything else, such as a failed load, a /Length that cached further+ // objects or a cache changed from outside, measures the whole cache again.+ let grew_by_this_object = inserted == 1+ && cache.len() == self.counted_objects.saturating_add(1)+ && matches!((&object, cache.get(&reference.num)), (Ok(read), Some(cached)) if Arc::ptr_eq(read, cached));+ if object.is_ok() && inserted == 0 && cache.len() == self.counted_objects {+ // Unchanged: a hit, or an object held by the overlay.+ } else if let (true, Ok(read)) = (grew_by_this_object, &object) {+ self.counted_bytes = self.counted_bytes.saturating_add(owned_bytes(read, data, 0));+ self.counted_objects = cache.len();+ } else {+ self.counted_bytes = cache.values().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, data, 0)));+ self.counted_objects = cache.len();+ }+ if cache.len() >= Self::OBJECTS || self.counted_bytes > Self::STREAM_BYTES { cache.clear();+ self.counted_objects = 0;+ self.counted_bytes = 0; } } object } } +thread_local! {+ /// Objects this thread has put into a document's parsed-object cache. A reader compares+ /// it before and after a read: a read that inserted exactly one object cannot have+ /// evicted or replaced anything else, whatever the cache length says.+ pub(super) static CACHE_INSERTS: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };+}+ #[cfg(test)] thread_local! { /// Cross-reference sections read on this thread.@@ -693,11 +693,43 @@ 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(5, 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"); } +#[test]+fn temporary_reader_running_total_matches_the_cache() {+ let mut bodies = vec!["<< /Type /Catalog /Pages 2 0 R >>".to_owned(), "<< /Type /Pages /Kids [] /Count 0 >>".to_owned()];+ bodies.extend((0..300).map(|i| format!("<< /Value {i} /Name ({}) >>", "s".repeat(i % 40))));+ let refs: Vec<_> = bodies.iter().map(String::as_str).collect();+ let doc = Document::open(Arc::new(build(&refs, "/Root 1 0 R"))).unwrap();+ let mut reader = doc.object_reader();+ for num in (1..=302).chain([5, 301, 99_999, 7]) {+ let _ = reader.try_get(ObjRef::new(num, 0));+ let cache = reader.document.cache.lock().unwrap();+ let measured = cache.values().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, &reader.document.data, 0)));+ assert_eq!((reader.counted_objects, reader.counted_bytes), (cache.len(), measured), "after object {num}");+ assert!(cache.len() < ObjectReader::OBJECTS);+ }+}++#[test]+fn temporary_reader_recounts_when_a_read_replaces_cache_entries() {+ // One cached object, then a read of a string over the editor cache's byte budget: the+ // editor eviction keeps only the newcomer, so the cache length stays at one. The+ // string is data of its own and must not stay cached past the reader's budget.+ let big = format!("({})", "a".repeat(Document::EDITOR_CACHE_BYTES + 1));+ let bytes = build(&["<< /Type /Catalog /Pages 2 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>", "<< /Small 1 >>", &big], "/Root 1 0 R");+ let doc = Document::open(Arc::new(bytes)).unwrap();+ let mut reader = doc.object_reader();+ let _ = reader.get(ObjRef::new(3, 0));+ assert_eq!((reader.document.cache.lock().unwrap().len(), reader.counted_objects), (1, 1));+ assert!(matches!(&*reader.get(ObjRef::new(4, 0)), Object::String(_)));+ assert!(reader.document.cache.lock().unwrap().is_empty(), "a replaced entry must be counted: the string is over budget");+ assert_eq!((reader.counted_objects, reader.counted_bytes), (0, 0));+}+ #[test] fn temporary_reader_and_full_save_do_not_fill_snapshot_caches() { let mut source = Document::new_empty();