Cap unbounded editing and text caches; index form fields once (#1115)
* Cap editing and text-layer caches that grew without bound The cos editing cache kept every touched object until save, so editing a large document pinned its whole object graph. Bound it like the scan reader: 1024 objects / 32 MB of owned bytes, 256 object streams / 32 MB, evicting to the newcomer. Returned objects outlive eviction, and clones share the caches, so this only costs re-parsing. The viewer text cache had the same shape: every visited page's text stayed for the session. Retain visible, selected and matched pages plus a 24-page prefetch window each frame; evicted pages re-extract on revisit. Skipped while a search is active, where demand still wants every page. Failures are never evicted: they cost nothing to remember. cli info on a 1024-page/147 KB file (debug build, same host/command): before 66.7 MB RSS, after 54.0 MB RSS (3 runs, tight). Tests: editor count bound, byte bound, snapshot-clone reads (cos); evict-far/keep-visible+matched+selection, search-active keep-all (canvas). All mutation-checked: each bound goes red when disabled. Signed-off-by: Chiranjeevi U <244287281+chiranjeevi-max@users.noreply.github.com> * Index form fields once per refresh instead of once per field The form-scope refresh ran a linear find over all fields for every field, O(F^2) on each keystroke; forms reach 10k fields. Build one name index up front, O(F). entry().or_insert() keeps the old first-match semantics for repeated field names. New test with two same-named widgets pins first-wins and goes red under insert. 141 engine tests green, clippy warnings and perf lints clean. Signed-off-by: Chiranjeevi U <244287281+chiranjeevi-max@users.noreply.github.com> --------- Signed-off-by: Chiranjeevi U <244287281+chiranjeevi-max@users.noreply.github.com>
This commit is contained in:
Chiranjeevi U
GitHub
parent
92c9de7aad
commit
9644aeb9e2
|
|
@ -145,6 +145,8 @@ impl ObjectReader {
|
|||
|
||||
pub(crate) fn try_get(&mut self, reference: ObjRef) -> Result<Arc<Object>, CosError> {
|
||||
let object = self.document.try_get(reference.num);
|
||||
// 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() {
|
||||
// Only insertion changes this private cache between reads. Most neighboring
|
||||
// objects reuse a decoded stream: do not sum the whole store for every object.
|
||||
|
|
@ -607,10 +609,81 @@ impl Document {
|
|||
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)
|
||||
}
|
||||
|
||||
/// Bounds for the editing cache: roomier than the scan reader's (128 objects / 8 MB)
|
||||
/// so interactive editing does not thrash, but still capped so touching a large
|
||||
/// document cannot pin its whole object graph. Only owned (decrypted/copied) bytes
|
||||
/// count; file-backed views re-parse for free.
|
||||
///
|
||||
/// Worst case per document: input bytes + up to `EDITOR_CACHE_OBJECTS` ×
|
||||
/// `EDITOR_LARGE_OBJECT` (1024 × 256 KB = 256 MB) of small objects between full
|
||||
/// accounting passes + `EDITOR_OBJSTM_BYTES` (32 MB) + unsaved `overlay` edits.
|
||||
/// The count cap always holds; byte accounting between passes is approximate.
|
||||
/// Clones share these caches, so one clone's eviction drops another's hot set;
|
||||
/// that only costs re-parsing, never correctness. Eviction keeps just the newcomer:
|
||||
/// file-backed views re-parse cheaply and editor working sets stay far below 1024.
|
||||
const EDITOR_CACHE_OBJECTS: usize = 1024;
|
||||
const EDITOR_CACHE_BYTES: usize = 32 * 1024 * 1024;
|
||||
/// Object streams decode to megabytes each, so fewer are kept than plain objects.
|
||||
const EDITOR_OBJSTM_OBJECTS: usize = 256;
|
||||
const EDITOR_OBJSTM_BYTES: usize = 32 * 1024 * 1024;
|
||||
/// Newcomers above this force a full accounting pass even below the count cap:
|
||||
/// `EDITOR_CACHE_BYTES / 128`, so 128 large newcomers always trigger a pass before the
|
||||
/// 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> {
|
||||
// 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 {
|
||||
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 data = &self.data;
|
||||
let owned: usize = live.iter().fold(0usize, |n, (_, o)| n.saturating_add(owned_bytes(o, data, 0)));
|
||||
if live.len() <= Self::EDITOR_CACHE_OBJECTS && owned <= Self::EDITOR_CACHE_BYTES {
|
||||
return Ok(());
|
||||
}
|
||||
// Re-check under the final lock: another thread may have inserted or evicted since
|
||||
// the snapshot. Keep its newcomers, and skip the eviction if back within budget.
|
||||
let snapshot: std::collections::HashSet<u32> = live.into_iter().map(|(n, _)| n).collect();
|
||||
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)));
|
||||
if owned <= Self::EDITOR_CACHE_BYTES {
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
cache.retain(|n, _| *n == fresh_num || !snapshot.contains(n));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn evict_editor_streams_if_over_budget(&self, current: u32) -> Result<(), CosError> {
|
||||
// The caller inserts `current` first, so retaining it keeps a live entry.
|
||||
let over = {
|
||||
let streams = self.objstms.lock().map_err(|_| CosError::Poisoned)?;
|
||||
let bytes = streams.values().fold(0usize, |bytes, stream| {
|
||||
bytes
|
||||
.saturating_add(stream.data.capacity())
|
||||
.saturating_add(stream.index.capacity().saturating_mul(std::mem::size_of::<(u32, usize)>()))
|
||||
});
|
||||
bytes > Self::EDITOR_OBJSTM_BYTES || streams.len() > Self::EDITOR_OBJSTM_OBJECTS
|
||||
};
|
||||
if over {
|
||||
self.objstms.lock().map_err(|_| CosError::Poisoned)?.retain(|n, _| *n == current);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Visit every current object without filling this document's editing cache.
|
||||
///
|
||||
/// Full-document searches (for example, standalone timestamps) need to inspect objects
|
||||
|
|
@ -720,7 +793,10 @@ impl Document {
|
|||
index.push((onum.max(0) as u32, first + off.max(0) as usize));
|
||||
}
|
||||
let stm = Arc::new(ObjStm { index, data: Arc::new(data) });
|
||||
// Same bound as the object cache: decoded object streams can be megabytes each.
|
||||
// Insert first so the retained `num` is actually in the map.
|
||||
self.objstms.lock().map_err(|_| CosError::Poisoned)?.insert(num, stm.clone());
|
||||
self.evict_editor_streams_if_over_budget(num)?;
|
||||
Ok(stm)
|
||||
}
|
||||
|
||||
|
|
@ -1361,6 +1437,74 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn editor_cache_stays_bounded() {
|
||||
let mut source = Document::new_empty();
|
||||
let objects: Vec<_> = (0..(Document::EDITOR_CACHE_OBJECTS + 64))
|
||||
.map(|i| {
|
||||
let mut value = vec![b'x'; 1024];
|
||||
value[..4].copy_from_slice(&(i as u32).to_le_bytes());
|
||||
Object::Ref(source.add(Object::String(crate::PdfString::literal(value))))
|
||||
})
|
||||
.collect();
|
||||
source.update_dict(source.root().unwrap(), |d| d.set(b"Extension".to_vec(), Object::Array(objects))).unwrap();
|
||||
let bytes = Arc::new(crate::write_full(&source, &crate::SaveOptions::default()).unwrap());
|
||||
let doc = Document::open(bytes).unwrap();
|
||||
let root = doc.get(doc.root().unwrap());
|
||||
let references = root.as_dict().unwrap().get(b"Extension").unwrap().as_array().unwrap().clone();
|
||||
let mut held = None;
|
||||
for (i, reference) in references.iter().enumerate() {
|
||||
let Object::Ref(reference) = reference else { panic!("indirect object") };
|
||||
let object = doc.get(*reference);
|
||||
assert_eq!(&object.as_string().unwrap().bytes[..4], &(i as u32).to_le_bytes());
|
||||
if i == 0 {
|
||||
held = Some(object);
|
||||
}
|
||||
assert!(doc.cache.lock().unwrap().len() <= Document::EDITOR_CACHE_OBJECTS, "editing must not pin the whole object graph");
|
||||
// Eviction keeps the newcomer.
|
||||
assert!(doc.cache.lock().unwrap().contains_key(&reference.num), "eviction must keep the newcomer");
|
||||
}
|
||||
// Returned objects survive eviction, through both handles of the shared cache.
|
||||
assert_eq!(&held.unwrap().as_string().unwrap().bytes[..4], &0u32.to_le_bytes());
|
||||
let snapshot = doc.clone();
|
||||
let first = references[0].as_ref().unwrap();
|
||||
assert_eq!(&snapshot.get(first).as_string().unwrap().bytes[..4], &0u32.to_le_bytes());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn editor_cache_byte_bound_fires() {
|
||||
// 40 × 1 MB strings: far below the count cap, above the 32 MB byte budget. Each
|
||||
// newcomer exceeds EDITOR_LARGE_OBJECT, so every insert takes a full accounting pass.
|
||||
let mut source = Document::new_empty();
|
||||
let objects: Vec<_> = (0..40)
|
||||
.map(|i| {
|
||||
let mut value = vec![b'y'; 1024 * 1024];
|
||||
value[..4].copy_from_slice(&(i as u32).to_le_bytes());
|
||||
Object::Ref(source.add(Object::String(crate::PdfString::literal(value))))
|
||||
})
|
||||
.collect();
|
||||
source.update_dict(source.root().unwrap(), |d| d.set(b"Extension".to_vec(), Object::Array(objects))).unwrap();
|
||||
let bytes = Arc::new(crate::write_full(&source, &crate::SaveOptions::default()).unwrap());
|
||||
let doc = Document::open(bytes).unwrap();
|
||||
let root = doc.get(doc.root().unwrap());
|
||||
let references = root.as_dict().unwrap().get(b"Extension").unwrap().as_array().unwrap().clone();
|
||||
let mut held = None;
|
||||
for (i, reference) in references.iter().enumerate() {
|
||||
let Object::Ref(reference) = reference else { panic!("indirect object") };
|
||||
let object = doc.get(*reference);
|
||||
assert_eq!(&object.as_string().unwrap().bytes[..4], &(i as u32).to_le_bytes());
|
||||
if i == 0 {
|
||||
held = Some(object);
|
||||
}
|
||||
// Eviction keeps the newcomer.
|
||||
assert!(doc.cache.lock().unwrap().contains_key(&reference.num), "eviction must keep the newcomer");
|
||||
}
|
||||
assert!(doc.cache.lock().unwrap().len() < references.len(), "the byte budget must evict even below the count cap");
|
||||
let last = references[references.len() - 1].as_ref().unwrap();
|
||||
assert!(doc.cache.lock().unwrap().contains_key(&last.num), "eviction must keep the newcomer");
|
||||
assert_eq!(&held.unwrap().as_string().unwrap().bytes[..4], &0u32.to_le_bytes());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_stream_limited_document_cannot_be_saved() {
|
||||
let bytes = Arc::new(crate::write_full(&Document::new_empty(), &crate::SaveOptions::default()).unwrap());
|
||||
|
|
|
|||
|
|
@ -2675,8 +2675,15 @@ impl Session {
|
|||
Scope::Metadata => return Ok(()),
|
||||
Scope::Comments => doc.info.annotations = comment_list(&editor.cos),
|
||||
Scope::Form => {
|
||||
// PERF: keystroke path, index once. A linear find per field is O(F^2);
|
||||
// forms reach 10k fields.
|
||||
let mut by_name = std::collections::HashMap::with_capacity(form.len());
|
||||
for x in form.iter() {
|
||||
// First match wins (do not simplify to collect: it keeps the last).
|
||||
by_name.entry(x.name.as_str()).or_insert(x);
|
||||
}
|
||||
for f in &mut doc.info.fields {
|
||||
if let Some(ff) = form.iter().find(|x| x.name == f.name) {
|
||||
if let Some(ff) = by_name.get(f.name.as_str()) {
|
||||
f.value = match ff.kind {
|
||||
pdfcraft_forms::FieldKind::CheckBox | pdfcraft_forms::FieldKind::Radio => {
|
||||
Some(ff.value.first().cloned().unwrap_or_else(|| "Off".into()))
|
||||
|
|
|
|||
|
|
@ -1193,6 +1193,37 @@ fn form_edits_refresh_field_values_without_a_full_inspection() {
|
|||
assert_eq!(values(&d.info.fields), [("name".to_string(), Some("Ada".to_string())), ("ok".to_string(), Some("Yes".to_string()))]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn form_refresh_keeps_first_value_when_field_names_repeat() {
|
||||
// Two widgets share one name; the refresh reads the first.
|
||||
let objs = [
|
||||
"<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R 5 0 R 6 0 R] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 7 0 R >> >> >> >>",
|
||||
"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 300] >>",
|
||||
"<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R 6 0 R] >>",
|
||||
"<< /Type /Annot /Subtype /Widget /FT /Tx /T (dup) /V (first) /Rect [20 250 180 270] /P 3 0 R >>",
|
||||
"<< /Type /Annot /Subtype /Widget /FT /Tx /T (dup) /V (second) /Rect [20 220 180 240] /P 3 0 R >>",
|
||||
"<< /Type /Annot /Subtype /Widget /FT /Tx /T (other) /Rect [20 190 180 210] /P 3 0 R >>",
|
||||
"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>",
|
||||
];
|
||||
let mut bytes = b"%PDF-1.7\n".to_vec();
|
||||
let mut offsets = Vec::new();
|
||||
for (i, o) in objs.iter().enumerate() {
|
||||
offsets.push(bytes.len());
|
||||
bytes.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes());
|
||||
}
|
||||
let xref = bytes.len();
|
||||
bytes.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes());
|
||||
for o in offsets {
|
||||
bytes.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes());
|
||||
}
|
||||
bytes.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes());
|
||||
let mut s = Session::new();
|
||||
let id = s.open("dup.pdf", None, Arc::new(bytes), None).unwrap();
|
||||
s.apply(id, Edit::SetFieldValue { name: "other".into(), value: FieldValue::Text("x".into()) }).unwrap();
|
||||
let dup = s.get(id).unwrap().info.fields.iter().find(|f| f.name == "dup").unwrap().value.clone();
|
||||
assert_eq!(dup.as_deref(), Some("first"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preparing_a_form_adds_renames_and_deletes_fields() {
|
||||
let (mut s, id) = session_with(1);
|
||||
|
|
|
|||
|
|
@ -48,6 +48,10 @@ const RESULTS_PER_FRAME: usize = 8;
|
|||
const MAX_THUMB_DEMAND: usize = 256;
|
||||
/// At most this many whole-page rasters are cached beyond the ones on screen.
|
||||
const MAX_CACHED_PAGES: usize = 24;
|
||||
/// At most this many text layers are cached beyond visible, selected and matched pages.
|
||||
/// Text layers are glyph structs, far smaller than rasters; deliberately independent of
|
||||
/// `MAX_CACHED_PAGES` so raster tuning never silently retunes text memory.
|
||||
const MAX_CACHED_TEXT_PAGES: usize = 24;
|
||||
|
||||
fn rgba_bytes(width: usize, height: usize) -> usize {
|
||||
width.saturating_mul(height).saturating_mul(4)
|
||||
|
|
@ -1380,11 +1384,39 @@ impl DocView {
|
|||
}
|
||||
self.thumbs.retain(|p, _| admitted.contains(p));
|
||||
self.stale_thumbs.retain(|p| admitted.contains(p));
|
||||
self.retain_text_layers();
|
||||
// Visible main pages and thumbnails take precedence over background search work.
|
||||
queue.extend(requests.into_iter().filter(|r| r.kind == RequestKind::Text));
|
||||
queue
|
||||
}
|
||||
|
||||
/// Drop text layers nothing can still need. Every visited page's text used to stay
|
||||
/// cached for the session. Keep visible, selected and matched pages plus the nearest
|
||||
/// prefetch window; evicted pages re-extract on revisit. (Count window only: unlike
|
||||
/// rasters there is no byte budget here.) While a search is active keep everything:
|
||||
/// demand still wants every page, so evicting would re-request evicted pages each
|
||||
/// frame and churn the workers. Failures are never evicted either: they cost nothing
|
||||
/// to remember, and evicting one would re-request and re-fail it every frame.
|
||||
fn retain_text_layers(&mut self) {
|
||||
if self.texts.len() <= MAX_CACHED_TEXT_PAGES {
|
||||
return;
|
||||
}
|
||||
if self.find.as_ref().is_some_and(|f| !f.case_query.trim().is_empty()) {
|
||||
return;
|
||||
}
|
||||
let mut keep: HashSet<usize> = self.frame_visible.clone();
|
||||
if let Some(s) = self.selection {
|
||||
keep.insert(s.page);
|
||||
}
|
||||
if let Some(f) = self.find.as_ref() {
|
||||
keep.extend(f.matches.iter().map(|(p, _)| *p));
|
||||
}
|
||||
let mut rest: Vec<usize> = self.texts.keys().filter(|p| !keep.contains(*p)).copied().collect();
|
||||
rest.sort_by_key(|p| p.abs_diff(self.current));
|
||||
keep.extend(rest.into_iter().take(MAX_CACHED_TEXT_PAGES));
|
||||
self.texts.retain(|p, _| keep.contains(p));
|
||||
}
|
||||
|
||||
pub(crate) fn finish_render_frame(&mut self, ctx: &egui::Context, info: &DocInfo, pool: &RenderPool) {
|
||||
let queue = self.prepare_render_queue(info, ctx.pixels_per_point());
|
||||
if queue != self.last_queue {
|
||||
|
|
@ -3996,6 +4028,60 @@ trailer << /Root 1 0 R >>
|
|||
DocView::new(DocId(1), &info, ViewDefaults { layout, continuous, ..Default::default() })
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn text_layers_evict_far_pages_but_keep_visible_and_matches() {
|
||||
let info = thumbnail_info(100);
|
||||
let mut v = DocView::new(DocId(1), &info, ViewDefaults::default());
|
||||
v.current = 50;
|
||||
v.frame_visible = HashSet::from([50, 51]);
|
||||
for p in 0..100 {
|
||||
v.texts.insert(p, Arc::new(PageText::default()));
|
||||
}
|
||||
v.text_failed.insert(0);
|
||||
v.text_failed.insert(99);
|
||||
v.find = Some(Find { matches: vec![(0, 0..1)], ..Default::default() });
|
||||
let _ = v.prepare_render_queue(&info, 1.0);
|
||||
assert!(v.texts.contains_key(&50) && v.texts.contains_key(&51), "visible pages stay");
|
||||
assert!(v.texts.contains_key(&0), "matched pages stay");
|
||||
assert!(v.texts.contains_key(&49), "nearest prefetch window stays");
|
||||
assert!(!v.texts.contains_key(&99), "far pages go");
|
||||
assert_eq!(v.texts.len(), 2 + 1 + MAX_CACHED_TEXT_PAGES, "visible + matched + prefetch window");
|
||||
assert!(v.text_failed.contains(&0) && v.text_failed.contains(&99), "failures are bytes-free: never evict");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn text_layers_keep_the_selection_page() {
|
||||
let info = thumbnail_info(100);
|
||||
let mut v = DocView::new(DocId(1), &info, ViewDefaults::default());
|
||||
let text = PageText {
|
||||
glyphs: vec![pdfcraft_render::TextGlyph { text: "x".into(), rect: [0.0, 0.0, 10.0, 10.0], quad: [[0.0, 0.0]; 4], direction: [1.0, 0.0] }],
|
||||
line_of: vec![0],
|
||||
space_before: vec![false],
|
||||
};
|
||||
for p in 0..100 {
|
||||
v.texts.insert(p, Arc::new(text.clone()));
|
||||
}
|
||||
v.select_text(0, 0, 0);
|
||||
v.current = 90;
|
||||
v.frame_visible = HashSet::from([90, 91]);
|
||||
let _ = v.prepare_render_queue(&info, 1.0);
|
||||
assert_eq!(v.selected_text().as_deref(), Some("x"), "stale selections still copy");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn text_layers_stay_while_a_search_is_active() {
|
||||
let info = thumbnail_info(100);
|
||||
let mut v = DocView::new(DocId(1), &info, ViewDefaults::default());
|
||||
v.current = 50;
|
||||
v.frame_visible = HashSet::from([50]);
|
||||
for p in 0..100 {
|
||||
v.texts.insert(p, Arc::new(PageText::default()));
|
||||
}
|
||||
v.find = Some(Find { query: "dup".into(), case_query: "dup".into(), matches: vec![(0, 0..1)], ..Default::default() });
|
||||
let _ = v.prepare_render_queue(&info, 1.0);
|
||||
assert_eq!(v.texts.len(), 100, "evicting mid-search would churn the workers");
|
||||
}
|
||||
|
||||
fn thumbnail_info(count: usize) -> DocInfo {
|
||||
DocInfo {
|
||||
pages: (0..count)
|
||||
|
|
|
|||
Loading…
Reference in New Issue