diff --git a/crates/engine/src/js.rs b/crates/engine/src/js.rs index b797bc8d83..6d344573a3 100644 --- a/crates/engine/src/js.rs +++ b/crates/engine/src/js.rs @@ -70,11 +70,15 @@ fn display(doc: &pdfcraft_cos::Document, f: &Field) -> i32 { /// A form field as scripts see it. pub fn field_state(doc: &pdfcraft_cos::Document, f: &Field) -> pdfcraft_js::FieldState { + field_state_with_display(f, display(doc, f)) +} + +fn field_state_with_display(f: &Field, display: i32) -> pdfcraft_js::FieldState { let mut s = pdfcraft_js::FieldState::new(f.name.clone(), kind(f.kind), f.value.clone()); s.default = f.default.clone(); s.readonly = f.read_only(); s.required = f.has(pdfcraft_forms::flags::REQUIRED); - s.display = display(doc, f); + s.display = display; s.char_limit = f.max_len; s.options = match f.kind { FieldKind::CheckBox | FieldKind::Radio => f.widgets.iter().filter_map(|w| w.on_state.clone()).map(|o| (o.clone(), o)).collect(), @@ -85,10 +89,19 @@ pub fn field_state(doc: &pdfcraft_cos::Document, f: &Field) -> pdfcraft_js::Fiel /// The changes a script made, against the fields as they were. fn changes(before: &[pdfcraft_js::FieldState], after: &[pdfcraft_js::FieldState]) -> Vec { + if after.is_empty() { + return Vec::new(); + } + // Outcomes contain only fields touched by the script. Index the original snapshot once, + // keeping the first match as the old linear lookup did for malformed duplicate names. + let mut by_name = std::collections::HashMap::with_capacity(before.len()); + for field in before { + by_name.entry(field.name.as_str()).or_insert(field); + } after .iter() .filter_map(|a| { - let b = before.iter().find(|b| b.name == a.name)?; + let b = by_name.get(a.name.as_str())?; let c = FieldChange { name: a.name.clone(), value: (a.value != b.value).then(|| a.value.clone()), @@ -106,14 +119,25 @@ pub struct JsRunner { pub doc: pdfcraft_js::DocInfo, pub doc_scripts: Vec, pub output: JsOutput, - /// The fields in the document as the forms code last read them (for `display`). + /// Immutable document snapshot from the start of the edit (for widget `display`). cos: pdfcraft_cos::Document, + // `cos` is the same immutable pre-edit snapshot for the runner's lifetime. Cache only + // its scalar display readings, not COS objects: large forms exceed the object cache and + // otherwise reparse every widget for every calculation. Values/flags still come from + // the live `fields` snapshot on each call. A new edit gets a new runner and cache. + displays: std::collections::HashMap, } impl JsRunner { pub fn new(cos: &pdfcraft_cos::Document, file_name: &str) -> JsRunner { let doc = pdfcraft_js::DocInfo { file_name: file_name.to_string(), num_pages: pdfcraft_model::pages(cos).len(), page: 0, info: info(cos) }; - JsRunner { doc, doc_scripts: pdfcraft_forms::document_scripts(cos), output: JsOutput::default(), cos: cos.clone() } + JsRunner { + doc, + doc_scripts: pdfcraft_forms::document_scripts(cos), + output: JsOutput::default(), + cos: cos.clone(), + displays: std::collections::HashMap::new(), + } } } @@ -130,19 +154,28 @@ fn info(cos: &pdfcraft_cos::Document) -> Vec<(String, String)> { impl Scripts for JsRunner { fn run(&mut self, event: FieldEvent, script: &str, target: &Field, value: &str, fields: &[Field]) -> ScriptResult { - let states: Vec<_> = fields.iter().map(|f| field_state(&self.cos, f)).collect(); + let states: Vec<_> = fields + .iter() + .map(|f| { + let display = match f.widgets.first() { + Some(w) => *self.displays.entry(w.obj).or_insert_with(|| display(&self.cos, f)), + None => pdfcraft_js::DISPLAY_VISIBLE, + }; + field_state_with_display(f, display) + }) + .collect(); + let states = std::sync::Arc::new(states); let ev = pdfcraft_js::Event::field(event.name(), &target.name, value); - let o = pdfcraft_js::run(script, &ev, &self.doc, &states, &self.doc_scripts, Limits::default()); + let o = pdfcraft_js::run_shared(script, &ev, &self.doc, std::sync::Arc::clone(&states), &self.doc_scripts, Limits::default()); self.output.absorb(&o); if o.error.is_some() { // As in Acrobat, a failing script leaves the value alone (the error goes to the console). return ScriptResult { rc: true, value: value.to_string(), changes: Vec::new(), message: None }; } - let after: Vec<_> = states.iter().map(|s| o.changed.iter().find(|c| c.name == s.name).unwrap_or(s).clone()).collect(); ScriptResult { rc: o.rc, value: o.value.clone(), - changes: changes(&states, &after), + changes: changes(&states, &o.changed), message: (!o.rc).then(|| o.alerts.last().cloned()).flatten(), } } @@ -213,8 +246,7 @@ impl Session { None => pdfcraft_js::Event::doc("Console"), }; let mut o = pdfcraft_js::run(script, &event, &runner.doc, &states, &runner.doc_scripts, Limits::default()); - let after: Vec<_> = states.iter().map(|s| o.changed.iter().find(|c| c.name == s.name).unwrap_or(s).clone()).collect(); - let changes = changes(&states, &after); + let changes = changes(&states, &o.changed); let resets: Vec> = o .requests .iter() @@ -368,3 +400,47 @@ impl Session { Ok(names) } } + +#[cfg(test)] +mod review_tests { + use super::*; + + #[test] + fn duplicate_field_names_diff_against_first_original() { + let first = pdfcraft_js::FieldState::new("duplicate", pdfcraft_js::FieldType::Text, vec!["first".into()]); + let mut second = first.clone(); + second.value = vec!["second".into()]; + second.display = pdfcraft_js::DISPLAY_HIDDEN; + let before = vec![first.clone(), second.clone()]; + assert!(changes(&before, std::slice::from_ref(&first)).is_empty()); + let actual = changes(&before, std::slice::from_ref(&second)); + assert_eq!( + actual, + vec![FieldChange { + name: "duplicate".into(), + value: Some(second.value), + display: Some(pdfcraft_js::DISPLAY_HIDDEN), + ..Default::default() + }] + ); + let outcome = pdfcraft_js::run_shared( + "event.value = getField('duplicate').value; getField('duplicate').value = 'updated';", + &pdfcraft_js::Event::field("Calculate", "duplicate", ""), + &pdfcraft_js::DocInfo::default(), + std::sync::Arc::new(before.clone()), + &[], + Limits::default(), + ); + assert_eq!(outcome.error, None); + assert_eq!(outcome.value, "first"); + assert_eq!(outcome.changed.len(), 1); + assert_eq!( + changes(&before, &outcome.changed), + vec![FieldChange { name: "duplicate".into(), value: Some(vec!["updated".into()]), ..Default::default() }] + ); + assert!(changes(&before, &[]).is_empty()); + let mut missing = first; + missing.name = "missing".into(); + assert!(changes(&before, &[missing]).is_empty()); + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f125c41dac..67af975966 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6106,3 +6106,185 @@ fn undo_never_makes_an_unsaved_document_clean() { s.undo(new).unwrap(); assert!(!s.get(new).unwrap().dirty, "once saved, undo back to the saved state is clean"); } +/// Reproducible workload with 3,129 fields and 190 ordered custom calculations. +/// No external PDF assets are needed; only in-memory contributor-original objects. +fn synthetic_acroform(field_count: usize, calculation_count: usize) -> Vec { + use pdfcraft_cos::{Dict, Object, PdfString}; + let mut doc = pdfcraft_cos::Document::open(Arc::new(fixture(6))).unwrap(); + let mut refs = Vec::new(); + let mut order = Vec::new(); + for i in 0..field_count { + let name = match i { + 0 => "ranks".to_string(), + 1 => "trained".to_string(), + 2 => "bonus".to_string(), + _ => format!("field{i}"), + }; + let mut d = Dict::new(); + d.set(b"Type".to_vec(), Object::name("Annot")); + d.set(b"Subtype".to_vec(), Object::name("Widget")); + d.set(b"FT".to_vec(), Object::name(if i == 1 { "Btn" } else { "Tx" })); + d.set(b"T".to_vec(), PdfString::text(&name)); + d.set(b"V".to_vec(), if i == 1 { Object::name("Off") } else { Object::String(PdfString::text("0")) }); + d.set(b"Rect".to_vec(), Object::Array([20, 20, 100, 40].map(Object::Int).to_vec())); + d.set(b"P".to_vec(), Object::Ref(pdfcraft_cos::ObjRef::new(4, 0))); + if (2..calculation_count + 2).contains(&i) { + let js = if i == 2 { + "var r = Number(getField('ranks').value); event.value = r + (r > 0 && getField('trained').isBoxChecked(0) ? 3 : 0);".to_string() + } else { + format!("event.value = Number(getField('{}').value) + 1;", if i == 3 { "bonus".to_string() } else { format!("field{}", i - 1) }) + }; + let mut a = Dict::new(); + a.set(b"S".to_vec(), Object::name("JavaScript")); + a.set(b"JS".to_vec(), PdfString::text(&js)); + let mut aa = Dict::new(); + aa.set(b"C".to_vec(), Object::Dict(a)); + d.set(b"AA".to_vec(), Object::Dict(aa)); + } + let r = doc.add(Object::Dict(d)); + refs.push(Object::Ref(r)); + if (2..calculation_count + 2).contains(&i) { + order.push(Object::Ref(r)); + } + } + doc.update_dict(pdfcraft_cos::ObjRef::new(4, 0), |d| d.set(b"Annots".to_vec(), Object::Array(refs.clone()))).unwrap(); + let mut af = Dict::new(); + af.set(b"Fields".to_vec(), Object::Array(refs)); + af.set(b"CO".to_vec(), Object::Array(order)); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"AcroForm".to_vec(), Object::Dict(af))).unwrap(); + pdfcraft_cos::write_incremental(&doc, &Default::default()).unwrap() +} + +#[test] +#[ignore = "manual performance measurement: 3,129 fields, 190 scripts"] +fn acroform_interaction_benchmark() { + let bytes = Arc::new(synthetic_acroform(3129, 190)); + // Optional local export for comparing CLI build profiles. Never overwrite an input. + if let Some(path) = std::env::var_os("PDFCRAFT_BENCH_EXPORT") { + use std::io::Write; + let mut file = std::fs::OpenOptions::new().write(true).create_new(true).open(path).unwrap(); + file.write_all(&bytes).unwrap(); + } + let mut s = Session::new(); + let id = s.open("synthetic-large-form.pdf", None, bytes, None).unwrap(); + let value = |s: &Session, name: &str| s.get(id).unwrap().form.iter().find(|f| f.name == name).unwrap().value.clone(); + for (name, v) in [ + ("ranks", FieldValue::Text("1".into())), + ("trained", FieldValue::Check(true)), + ("ranks", FieldValue::Text("2".into())), + ("trained", FieldValue::Check(false)), + ("ranks", FieldValue::Text("0".into())), + ] { + let started = std::time::Instant::now(); + s.apply(id, Edit::SetFieldValue { name: name.into(), value: v }).unwrap(); + eprintln!("BENCH {name}: {:.3} ms", started.elapsed().as_secs_f64() * 1000.0); + assert!(s.take_js_output(id).errors.is_empty()); + } + assert_eq!(value(&s, "bonus"), ["0"]); + assert_eq!(value(&s, "field191"), ["189"]); + assert_eq!(value(&s, "field3128"), ["0"]); +} + +#[test] +fn checkbox_and_rank_changes_recalculate_ordered_dependents() { + let mut s = Session::new(); + let id = s.open("skills.pdf", None, Arc::new(synthetic_acroform(8, 4)), None).unwrap(); + for (name, input, bonus) in [ + ("trained", FieldValue::Check(true), "0"), + ("ranks", FieldValue::Text("1".into()), "4"), + ("trained", FieldValue::Check(false), "1"), + ("ranks", FieldValue::Text("2".into()), "2"), + ("trained", FieldValue::Check(true), "5"), + ("ranks", FieldValue::Text("0".into()), "0"), + ] { + s.apply(id, Edit::SetFieldValue { name: name.into(), value: input }).unwrap(); + let fields = &s.get(id).unwrap().form; + let value = |name: &str| fields.iter().find(|f| f.name == name).unwrap().value.clone(); + assert_eq!(value("bonus"), [bonus]); + assert_eq!(value("field5"), [(bonus.parse::().unwrap() + 3).to_string()]); + assert_eq!(value("field7"), ["0"]); + assert!(s.take_js_output(id).errors.is_empty()); + } + s.undo(id).unwrap(); + assert_eq!(s.get(id).unwrap().form.iter().find(|f| f.name == "bonus").unwrap().value, ["5"]); + let saved = s.save_bytes(id).unwrap(); + let reopened = s.open("saved.pdf", None, saved, None).unwrap(); + assert_eq!(s.get(reopened).unwrap().form.iter().find(|f| f.name == "bonus").unwrap().value, ["5"]); + let appearance = |s: &Session| { + let doc = s.get(id).unwrap(); + let widget = doc.form.iter().find(|f| f.name == "bonus").unwrap().widgets.first().unwrap().obj; + doc.editor.as_ref().unwrap().cos.get(widget).as_dict().unwrap().get(b"AP").cloned() + }; + let before = appearance(&s); + assert!(before.is_some()); + s.apply(id, Edit::SetFieldValue { name: "field7".into(), value: FieldValue::Text("9".into()) }).unwrap(); + assert_eq!(appearance(&s), before, "unchanged calculated values keep their appearance streams"); +} + +#[test] +fn dynamic_calculation_reads_and_side_effects_reach_later_fields() { + use pdfcraft_cos::{Dict, Object, PdfString}; + let mut cos = pdfcraft_cos::Document::open(Arc::new(synthetic_acroform(8, 4))).unwrap(); + let fields = pdfcraft_forms::fields(&cos); + let bonus = fields.iter().find(|f| f.name == "bonus").unwrap(); + let selector = fields.iter().find(|f| f.name == "field7").unwrap(); + cos.update_dict(selector.obj, |d| d.set(b"V".to_vec(), PdfString::text("ranks"))).unwrap(); + let mut a = Dict::new(); + a.set(b"S".to_vec(), Object::name("JavaScript")); + a.set(b"JS".to_vec(), PdfString::text( + "var key = getField('field7').value; var v = Number(getField(key).value); getField('field6').value = v * 2; getField('field6').readonly = true; event.value = v + 1;" + )); + let mut aa = Dict::new(); + aa.set(b"C".to_vec(), Object::Dict(a)); + cos.update_dict(bonus.obj, |d| d.set(b"AA".to_vec(), Object::Dict(aa))).unwrap(); + let bytes = pdfcraft_cos::write_incremental(&cos, &Default::default()).unwrap(); + let mut s = Session::new(); + let id = s.open("dynamic.pdf", None, Arc::new(bytes), None).unwrap(); + s.apply(id, Edit::SetFieldValue { name: "ranks".into(), value: FieldValue::Text("4".into()) }).unwrap(); + let fields = &s.get(id).unwrap().form; + let field = |name: &str| fields.iter().find(|f| f.name == name).unwrap(); + assert_eq!(field("bonus").value, ["5"]); + assert_eq!(field("field5").value, ["8"], "later calculations see the ordered result"); + assert_eq!(field("field6").value, ["8"], "side-effect value applied"); + assert!(field("field6").read_only(), "side-effect property applied"); + assert_eq!(field("field7").value, ["ranks"]); + assert!(s.take_js_output(id).errors.is_empty()); +} + +#[test] +fn scripted_display_changes_survive_warm_cache_and_new_edit() { + use pdfcraft_forms::{FieldEvent, Scripts}; + let mut cos = pdfcraft_cos::Document::open(Arc::new(synthetic_acroform(2, 0))).unwrap(); + for (display, flags) in [(1, 6), (3, 36), (2, 0), (0, 4)] { + let fields = pdfcraft_forms::fields(&cos); + let target = fields.iter().find(|f| f.name == "ranks").unwrap(); + let other = fields.iter().find(|f| f.name == "trained").unwrap(); + let old_display = crate::js::field_state(&cos, other).display; + let mut runner = crate::js::JsRunner::new(&cos, "synthetic.pdf"); + // Populate the display cache before a script writes the same field. + let warm = runner.run(FieldEvent::Calculate, "event.value = getField('trained').display;", target, "", &fields); + assert_eq!(warm.value, old_display.to_string()); + let result = runner.run( + FieldEvent::Calculate, + &format!("getField('trained').display = {display}; event.value = getField('trained').display;"), + target, + "", + &fields, + ); + assert_eq!(result.value, display.to_string()); + assert_eq!(result.changes.len(), 1); + assert_eq!(result.changes.first().unwrap().display, Some(display)); + pdfcraft_forms::apply_script_changes(&mut cos, &result.changes, &mut pdfcraft_forms::NoScripts).unwrap(); + assert_eq!(cos.get(other.widgets.first().unwrap().obj).as_dict().unwrap().int(b"F").unwrap() & 38, flags); + // Within an edit the original implementation reads an immutable pre-edit COS snapshot. + let same_edit = runner.run(FieldEvent::Calculate, "event.value = getField('trained').display;", target, "", &fields); + assert_eq!(same_edit.value, old_display.to_string()); + // The next edit must rebuild its cache from the newly written annotation flags. + let mut next = crate::js::JsRunner::new(&cos, "synthetic.pdf"); + let read = next.run(FieldEvent::Calculate, "event.value = getField('trained').display;", target, "", &pdfcraft_forms::fields(&cos)); + assert_eq!(read.value, display.to_string()); + assert!(read.changes.is_empty()); + assert!(runner.output.errors.is_empty()); + assert!(next.output.errors.is_empty()); + } +} diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index d8d9fa8bac..785345c672 100644 --- a/crates/js/src/lib.rs +++ b/crates/js/src/lib.rs @@ -25,6 +25,7 @@ use std::cell::RefCell; use std::collections::BTreeSet; +use std::sync::Arc; use boa_engine::object::builtins::{JsArray, JsFunction}; use boa_engine::object::{FunctionObjectBuilder, ObjectInitializer}; @@ -194,7 +195,7 @@ impl Default for Limits { } struct Host { - fields: Vec, + fields: Arc>, changed: BTreeSet, alerts: Vec, console: Vec, @@ -286,7 +287,8 @@ fn read(ctx: &Context, name: &JsString) -> JsResult { fn write(ctx: &Context, name: &JsString, change: impl FnOnce(&mut FieldState)) -> JsResult<()> { let i = field_index(ctx, name)?; let mut h = host(ctx)?.borrow_mut(); - change(&mut h.fields[i]); + let field = Arc::make_mut(&mut h.fields).get_mut(i).ok_or_else(|| error("the field is no longer available"))?; + change(field); h.changed.insert(i); Ok(()) } @@ -1098,6 +1100,13 @@ fn refuse(source: &str) -> Option { /// Run `script` for `event`, after the document-level scripts (`doc_scripts`, which define the /// functions field scripts call), with the form's `fields`. pub fn run(script: &str, event: &Event, doc: &DocInfo, fields: &[FieldState], doc_scripts: &[String], limits: Limits) -> Outcome { + run_shared(script, event, doc, Arc::new(fields.to_vec()), doc_scripts, limits) +} + +/// Like [`run`], sharing an immutable initial field snapshot. Scripts still get a fresh +/// context; the first field write copies the snapshot, so neither the caller nor another +/// script can observe uncommitted writes. Read-only calculations avoid copying the whole form. +pub fn run_shared(script: &str, event: &Event, doc: &DocInfo, fields: Arc>, doc_scripts: &[String], limits: Limits) -> Outcome { let failed = |why: String| Outcome { rc: true, value: event.value.clone(), change: event.change.clone(), error: Some(why), ..Default::default() }; if let Some(why) = std::iter::once(script).chain(doc_scripts.iter().map(String::as_str)).find_map(refuse) { return failed(why); @@ -1119,12 +1128,12 @@ pub fn run(script: &str, event: &Event, doc: &DocInfo, fields: &[FieldState], do run_here(script, event, doc, fields, doc_scripts, limits) } -fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: &[FieldState], doc_scripts: &[String], limits: Limits) -> Outcome { +fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc>, doc_scripts: &[String], limits: Limits) -> Outcome { let mut ctx = Context::default(); ctx.runtime_limits_mut().set_loop_iteration_limit(limits.loop_iterations); ctx.runtime_limits_mut().set_recursion_limit(limits.recursion); ctx.insert_data(RefCell::new(Host { - fields: fields.to_vec(), + fields, changed: BTreeSet::new(), alerts: Vec::new(), console: Vec::new(), diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index 61d3a71e51..1d2856b8bd 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -1132,3 +1132,37 @@ mod session { assert!(o.abandoned || o.error.as_deref().is_some_and(|e| e.contains("too long")), "{o:?}"); } } + +#[test] +fn shared_field_snapshots_isolate_writes_errors_and_script_globals() { + let initial = Arc::new(fields()); + let before = initial.as_ref().clone(); + let event = Event::field("Calculate", "total", ""); + let write = run_shared( + "var scriptLocal = 1; getField('qty').value = 9; getField('agree').checkThisBox(0, true); event.value = getField('qty').value;", + &event, + &doc(), + Arc::clone(&initial), + &[], + Limits::default(), + ); + assert_eq!(write.error, None); + assert_eq!(write.value, "9"); + assert_eq!(write.changed.iter().find(|f| f.name == "qty").unwrap().value, ["9"]); + assert_eq!(write.changed.iter().find(|f| f.name == "agree").unwrap().value, ["Yes"]); + assert_eq!(*initial, before, "script writes cannot mutate the caller's snapshot"); + let failed = run_shared("getField('qty').value = 99; throw new Error('rejected');", &event, &doc(), Arc::clone(&initial), &[], Limits::default()); + assert!(failed.error.is_some()); + assert_eq!(*initial, before, "an exception cannot leak partial writes into the initial snapshot"); + let read = run_shared( + "event.value = typeof scriptLocal + ':' + getField('qty').value + ':' + getField('agree').isBoxChecked(0);", + &event, + &doc(), + initial, + &[], + Limits::default(), + ); + assert_eq!(read.error, None); + assert_eq!(read.value, "undefined:4:false"); + assert!(read.changed.is_empty()); +} diff --git a/docs/form-performance.md b/docs/form-performance.md new file mode 100644 index 0000000000..c00adf0913 --- /dev/null +++ b/docs/form-performance.md @@ -0,0 +1,278 @@ +# AcroForm JavaScript performance + +Status: implemented; proposed as an independent performance PR. Last reviewed and updated: +2026-10-19. Measurements below are historical synthetic evidence collected before this split, +not fresh measurements on upstream main. No private document is needed for the benchmark. + +## Architecture + +- `crates/ui-egui/src/forms_ui.rs` queues `Edit::SetFieldValue` for a text commit or checkbox + toggle. `crates/automation/src/forms.rs::form_fill` uses the same edit API. +- `crates/engine/src/lib.rs::Session::apply` runs edits on a copy, preserving rollback and undo. + `run_edit` calls `pdfcraft_forms::set_value_with` with an engine `JsRunner`. +- `crates/forms/src/lib.rs` validates and writes the edited value, then runs one calculation + pass. `/CO` precedes calculated fields omitted from `/CO`. Each script sees prior writes. + There is no dependency graph or dependency-based filtering: dynamic references and script + side effects are evaluated on every pass. +- `crates/engine/src/js.rs::JsRunner::run` snapshots the fields, invokes `pdfcraft_js::run_shared`, + then converts changed JavaScript field states into form updates. +- `crates/js/src/lib.rs::run_shared` starts a scoped worker per script. `run_here` builds a fresh Boa + context, installs the API, evaluates document scripts and evaluates the action. No script or + bytecode cache is present in this AcroForm path. XFA uses a separate session implementation. +- `crates/forms/src/scripting.rs::apply_changes` applies script side effects before the next + calculation. It rereads the field snapshot when side effects occur. Calculation results + equal to the existing value already skip value writes and appearance generation. +- Checkboxes write the field `/V` and widget `/AS`; they enter the same calculation pass as + text edits. JavaScript exposes the export value or `Off`, and `isBoxChecked`. +- `Session::refresh_scoped` writes an incremental working revision, rereads fields and rebuilds + the render pool. `crates/ui-egui/src/editing.rs::apply_edit` compares old/new fields and + invalidates affected pages (including dependent fields on other pages). It does not blindly + invalidate every page for an ordinary AcroForm fill. Rendering uses page/tile requests; + widget appearance streams are separate from cached page rasters. + +## Findings and changes + +The dominant measured cost was the bridge, not executing calculation logic: + +1. `JsRunner` expanded a sparse `Outcome::changed` into a complete second snapshot. `changes` + then searched the first snapshot linearly for **each** field, even when JavaScript had not + changed any field directly. That repeated quadratic comparison ran after every calculation. + It now compares the sparse outcome only, with an indexed original snapshot when needed. + Empty outcomes return immediately; unchanged writes still produce no form updates. The + console/button path uses the same conversion. +2. Once that was removed, field-state construction dominated. Reading every widget's display + flags on every script traversed the same immutable pre-edit COS snapshot. This workload + exceeds COS's 1,024-object editing-cache cap, repeatedly reparsing widget dictionaries. + The runner now memoizes only those scalar display readings by widget reference. Each edit + creates a new runner; current values, read-only/required flags and options still come from + the current field snapshot. No global cache or object-cache limit changes were needed. + +All calculation actions still execute in the same order. No dependency filtering, script +removal, context reuse, bytecode caching or recursive scheduling changes were introduced. +Equal calculated values already skipped appearance regeneration; this behavior remains. +The Release investigation also found a full field-state copy inside the JavaScript host on +**every** script invocation. `pdfcraft_js::run_shared` now accepts an immutable shared snapshot; +`Arc::make_mut` detaches it on the first field write. The engine retains the initial snapshot +for comparing effects. Read-only calculations no longer copy every field again. Each script +still gets a fresh context and thread; exceptions, globals and uncommitted writes remain +isolated. Scripts that write fields retain the previous copy isolation. The existing borrowed +`run` API remains available. No new dependencies were added. + +## Measurements + +Historically measured under Rust 1.99.1 on Linux x86_64 with the default optimized development +profile, two-job builds. Serial benchmark runs had no concurrent compilation. These are +**synthetic engine measurements, not measurements of the original Pathfinder PDF or GUI input +latency**. Five edits: ranks 1, checkbox on, ranks 2, checkbox off, ranks 0. Every edit runs all +190 custom calculations among 3,129 fields on a six-page document. Widgets are colocated +on the first page; the fixture is an engine calculation stress test, not a layout replica. + +| Synchronous edit | Before (ms) | After (ms) | +|---|---:|---:| +| ranks 1 | 4,425.274 | 424.465 | +| checkbox on | 4,378.642 | 458.716 | +| ranks 2 | 4,291.131 | 521.721 | +| checkbox off | 4,391.942 | 492.851 | +| ranks 0 | 4,322.570 | 453.392 | +| Median | **4,378.642** | **458.716** | + +Median speedup: **9.55×**, approximately **89.5% less synchronous editing time**. The synthetic +scripts are deliberately small; this does not predict the exact result for other documents. +Raw logs and the baseline test executable are retained locally in `target/form-performance/`. + +Historical serial runs collected with optional instrumentation (mean per edit; inclusive +nested spans). That instrumentation is not included in this PR: + +| Phase | Before (ms) | After (ms) | +|---|---:|---:| +| JavaScript, including thread/context setup | 225.934 | 201.584 | +| Changed-field comparison | 2,814.805 | 0.024 | +| Entire script dispatch, all 190 scripts | 4,320.643 | 352.733 | +| Calculation pass, including changed appearances | 4,399.084 | 390.958 | +| Direct field write | 0.201 | 0.196 | +| Appearance generation, including Format | 61.225 | 22.981 | +| Form event, including direct write and calculations | 4,411.767 | 402.981 | +| Engine refresh/serialization | 23.499 | 21.826 | +| Whole engine edit | 4,442.144 | 431.539 | + +An intermediate run after sparse comparison alone measured 1,193.419 ms/edit in field-state +construction. After display memoization it measured 101.821 ms/edit. The intermediate run's +whole engine edit averaged 1,592.101 ms. Appearance time also benefits from the changed cache +working set; there was no change to which calculated appearances are generated. + +Rendering and compositor time are not included in these edit numbers. The original sheet and +GUI acceptance run remain required to quantify them. + +## Release comparison and remaining cost + +Historical comparison used the repository's Release profile (thin LTO, one code-generation +unit) and an optional standalone profiler, which is not included in this PR. +A fresh in-memory fixture was exported with create-new semantics to +`target/form-performance/synthetic-large.pdf` (SHA-256 +`300431eeae117458d96e52877e12a9e63ea8dcbb8ad4500da717ba00ebb2f1e1`). It remained unchanged. +The example alternated `ranks` between 1 and 2 for six samples per process, always executing +all 190 scripts. Page rendering was timed separately. + +Initial matched comparison, before the additional shared-snapshot change: + +| Build | Samples | Median engine edit (ms) | +|---|---:|---:| +| Development | 6 | 460.697 | +| Release | 6 | 408.358 | + +Release alone reduced the median by about 11%. A detailed Release run measured 106.947 ms/edit +constructing field states, 174.696 ms in JavaScript setup/execution, and 19.808 ms refreshing +engine view data. Calculation-order setup was only 0.319 ms. That supported investigating the +host's extra copy, rather than changing calculation scheduling or disabling calculations. + +The pre-change Release profiler was retained, then compared with the shared-snapshot Release +profiler in alternating runs (old, new, old, new). No compilation ran during measurement. + +| Release version | Samples | Median edit (ms) | Range (ms) | +|---|---:|---:|---:| +| Before shared snapshots | 12 | 381.380 | 353.468–447.833 | +| With shared snapshots | 12 | 267.063 | 258.545–313.777 | + +The additional optimization reduced median engine time by **30.0%** (1.43× speedup). +This is a separate comparison from the earlier five-edit benchmark; do not combine their +speedup factors. The page-render/setup cost in the initial matched comparison was about +6–9 ms for this deliberately overlapping synthetic layout, not for the original sheet. + +After shared snapshots, a historical Release run with optional instrumentation averaged: + +| Phase | Mean per edit (ms) | +|---|---:| +| Field-state construction | 75.754 | +| JavaScript setup/execution | 83.198 | +| Complete script dispatch | 200.480 | +| Calculation pass | 237.780 | +| Calculated-value writes, including appearances | 25.089 | +| Appearances | 23.712 | +| All complete field enumerations (three per edit) | 27.387 | +| Calculation-order setup | 0.305 | +| Engine refresh | 19.128 | +| Whole engine edit | 272.807 | + +These inclusive timings show that scheduling is now negligible on the fixture. The main +remaining costs are constructing/disposing field snapshots and creating/executing isolated +JavaScript contexts. Further snapshot caching or context reuse would need additional semantic +work and real-document evidence; no dependency-based skipping or global/context reuse was +introduced. The current shared-snapshot change was retained because its paired measurement +showed a substantial improvement and isolation regressions passed. + +**Remaining limit:** these numbers do not quantify real-document or compositor latency. +A representative document and delayed field are still needed for a reproducible engine/render/UI +comparison. Prior manual acceptance on the integrated release reported responsive field editing; +that is not a timing measurement or independent validation of this split. + +## Reproduce + +Use the same compiler, build profile and machine for before/after runs. The default development +profile optimizes workspace crates at level 1 and dependencies at level 2. Build with two jobs +and a separate target directory, then run the benchmark without concurrent compilation: + +```sh +CARGO_TARGET_DIR=target/acroform-benchmark cargo test -p pdfcraft-engine -j 2 --lib acroform_interaction_benchmark --no-run +CARGO_TARGET_DIR=target/acroform-benchmark cargo test -p pdfcraft-engine -j 2 --lib acroform_interaction_benchmark -- --ignored --nocapture +``` + +The benchmark creates a six-page, 3,129-field/190-calculation workload from contributor-original +PDF objects in memory and times five synchronous edits with `Instant`. It also checks the final +calculated chain and an independent field. Widgets overlap on page one; this is an engine stress +test, not a realistic page layout. There is no machine-dependent timing threshold in CI. + +To export a synthetic fixture for a standalone comparison, set `PDFCRAFT_BENCH_EXPORT` to a new +filename when running the ignored benchmark. Its parent must exist; export refuses to overwrite +any existing file. The ordinary benchmark writes no PDF. + +## Validation + +Regression coverage includes shared-snapshot write/error/global isolation, duplicate field names, +scripted display changes with a warm cache and a fresh edit, ordered checkbox/rank calculations, +dynamic field lookup, property/value side effects, undo/save/reopen, and unchanged appearances. +The existing JavaScript and engine suites cover the unchanged public scripting behavior. + +Before the split, the ignored benchmark passed both before and after optimization. Earlier +scoped native/WASM checks, Clippy, layering, asset and parity checks, UI form/JavaScript tests, +and automation `javascript_through_tools` checks also passed on the mixed review history. +Those historical checks do not certify these independent branches or hosted CI. + +## Independent branch validation + +Validated on upstream `a12fe8f4116984ebbc870eb514346ea922233316` using a fresh, isolated +Cargo target directory, offline locked dependencies and two build jobs: + +- All five focused regressions passed separately: duplicate names, warm/new-edit display + caches, ordered checkbox/rank dependents, dynamic lookup/side effects and shared-snapshot + write/error/global isolation. +- `cargo test -p pdfcraft-js -p pdfcraft-engine -p pdfcraft-forms -j 2`: engine 285 unit + and 9 integration tests, JavaScript 47 tests, and forms 80 tests passed. Two unit timing + probes and two private-file acceptance tests were ignored in the regular suite. +- The ignored 3,129-field/190-calculation benchmark was then run explicitly and passed; + its final calculated chain and independent field remained correct. This is a diagnostic + timing run, not a new before/after comparison on this upstream base. +- Scoped `cargo clippy --all-targets -- -D warnings` and WASM checks of those three + crates passed, as did formatting, whitespace, layering, asset and parity checks. +- `javascript_through_tools` passed. A CLI built in the isolated target directory then + reproduced bonuses 1 → 4 → 5 → 2 → 0; save/reopen and `page_render` passed, and the + synthetic page image was inspected. + +These are local checks, not hosted CI. No GUI executable was rebuilt and no private PDF +was used for this independent validation. + +## Rebase validation + +Revalidated on upstream `932bf9c6bfdd63e2a21bb831696a3447b9def9cf` (2026-10-18). +The performance implementation and all five focused regressions are unchanged from the +previously reviewed fix. Upstream's newer tests and CSV documentation are retained. + +- Engine: 286 unit and 9 integration tests passed; JavaScript: 48 passed; forms: 82 passed. + Two unit timing probes and two private-file acceptance tests remain ignored in the + regular suite. +- All five focused regressions and the explicit 3,129-field/190-calculation benchmark passed. +- Scoped all-targets Clippy with `-D warnings`, WASM, formatting, whitespace, layering, + asset and parity checks passed. +- `javascript_through_tools` and a freshly built CLI's fill/save/reopen/render smoke check + passed. The bonus chain remained 1 → 4 → 5 → 2 → 0, and the synthetic render was inspected. + +Validation used a separate Cargo target directory. No GUI build or private PDF was needed; +these are local checks, not hosted CI or a new before/after performance comparison. + +## PR #3318 refresh + +Rebased onto freshly fetched upstream main +`e397efa85451be00f8d56df7649807fe31e019c7` on 2026-10-19. Both roadmap conflicts +retain upstream's menu, button-label and content-editing notes alongside the performance +notes. Every performance source, regression-test and parity hunk is exactly the original +performance patch applied to current upstream; upstream's additional tests are retained. +Main advanced after initial validation on `43550c44`; the non-overlapping redaction update +was incorporated and all checks below were repeated on the final base. + +- All five focused regressions passed. +- Engine: 287 unit and 15 integration tests passed. JavaScript: 49 tests passed. Forms: + 84 tests passed. Doc tests passed. The regular suite still ignored two timing probes + and two private-file tests; the synthetic benchmark was then run explicitly and passed. +- The 3,129-field/190-calculation benchmark preserved the expected calculated values. Its + five diagnostic edits took 317.347–328.488 ms; this is not a new before/after comparison. +- Scoped all-targets Clippy with warnings denied, WASM, formatting, whitespace, layering, + assets and parity passed. +- Automation JavaScript and CLI checks passed: bonuses 1 → 4 → 5 → 2 → 0, save/reopen, + and page rendering. The synthetic render was inspected. + +Builds used an isolated Cargo target outside the worktrees. No GUI executable was built, +no private PDF was used, and the compatibility branch was not changed. These are local +checks, not hosted CI or a new performance speedup claim. + +## Revision history + +- 2026-10-19: refresh PR #3318 onto upstream `e397efa8`, retain upstream roadmap additions + and all performance changes, and repeat focused, scoped, benchmark and headless validation. + +- 2026-10-18: rebase onto upstream `932bf9c6`; retain all performance source edits and regression + tests; preserve upstream CSV documentation and repeat scoped native/WASM and headless checks. +- 2026-10-18: separate performance changes for review on current upstream main; add duplicate-name + and scripted-display coverage; keep historical optional instrumentation results as evidence only. +- 2026-10-18: retain shared snapshots after a paired 30% synthetic Release improvement and + verify snapshot isolation. +- 2026-10-17: remove quadratic outcome conversion and repeated display reads; record the + synthetic before/after benchmark. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..4415d7ac5d 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4596,7 +4596,7 @@ milestone = "M6" status = "shipped" commands = ["form.field.properties"] tools = ["form_set_props", "form_fill"] -evidence = ["crates/forms/src/tests.rs::formats_validation_and_calculations_run_like_acrobat"] +evidence = ["crates/forms/src/tests.rs::formats_validation_and_calculations_run_like_acrobat", "crates/engine/src/tests.rs::checkbox_and_rank_changes_recalculate_ordered_dependents", "crates/engine/src/tests.rs::dynamic_calculation_reads_and_side_effects_reach_later_fields"] notes = "/CO order, then other calculated fields; recalculated after every fill and Clear form" [[feature]]