A document with many content controls saves in the time they take

Saving a document with many content controls was quadratic in the number of
controls: 32,000 of them took four seconds to save, and doubling that took
four times as long again.

Two loops in the writer cost that. The plan that decides where each control
is written collects the inline markers of every control in one set, and then,
for each paragraph, looks for that paragraph's markers by filtering the whole
set — so one paragraph's answer costs the size of the document's marker set,
and the save costs paragraphs times markers. And to find the block-level
controls that clash, every control was compared with every other control.

The markers are now grouped by paragraph once, so a paragraph's markers are a
lookup, and the block-level controls are grouped by the block they end in,
each group keeping the two earliest closes, which answers the clash question
the same way in one sweep.

What is written does not change: the packages written for ten shapes of
control nesting — inline, around paragraphs, chained, nested — are byte for
byte the same as before.

Tests: crates/docx/tests/content_control_save_cost.rs. The two timing tests
fail without the fix (16.1x and 13.0x the time for four times the controls,
against a bound of 8.0) and pass with it (4.6x).

Fixes #3437.

Co-Authored-By: opencode <noreply@opencode.ai>

crates/docx/src/write/sdt.rs

@@ -5,8 +5,6 @@
 //! elements around runs (inline) or around whole paragraphs and tables (block level). [`plan`]
 //! decides, per block list, which controls are written where so that the XML always nests.
 
-use std::collections::HashSet;
-
 use wordcraft_doc::Blocks;
 use wordcraft_doc::control::{ContentControl, ControlEvent, ControlKind, control_events, pair_events};
 use wordcraft_doc::para::OBJ;
@@ -20,8 +18,11 @@ pub struct Plan<'a> {
     pub opens: Vec<Vec<&'a ContentControl>>,
     /// Controls closing after block `i`.
     pub closes: Vec<u32>,
-    /// Paragraph markers written inline, as (block, byte offset).
-    pub inline: HashSet<(usize, usize)>,
+    /// The paragraph markers written inline, by block: the byte offsets in block `i`, in order.
+    ///
+    /// Grouped by block rather than one set of (block, offset) pairs, so that asking for a
+    /// paragraph's markers is a lookup and not a search of every marker in the document (#13).
+    pub inline: Vec<Vec<usize>>,
 }
 
 impl Plan<'_> {
@@ -38,9 +39,7 @@ impl Plan<'_> {
     }
     /// The byte offsets of paragraph `i`'s markers that are written inline.
     pub fn inline_in(&self, i: usize) -> Vec<usize> {
-        let mut v: Vec<usize> = self.inline.iter().filter(|(b, _)| *b == i).map(|(_, o)| *o).collect();
-        v.sort_unstable();
-        v
+        self.inline.get(i).cloned().unwrap_or_default()
     }
 }
 
@@ -54,7 +53,7 @@ enum Mode {
 /// Markers without a partner are not written.
 pub fn plan(bl: &Blocks) -> Plan<'_> {
     let n = bl.len();
-    let mut out = Plan { opens: vec![Vec::new(); n], closes: vec![0; n], inline: HashSet::new() };
+    let mut out = Plan { opens: vec![Vec::new(); n], closes: vec![0; n], inline: vec![Vec::new(); n] };
     let events = control_events(bl);
     if events.is_empty() {
         return out;
@@ -115,17 +114,37 @@ pub fn plan(bl: &Blocks) -> Plan<'_> {
     }
     // Block-level controls nest by block: one that opens in the block another (not its
     // ancestor) closes after starts with the next block instead.
+    //
+    // Such a clash is with a control that closes before this one opens and ends in the block
+    // this one starts in. The block-level controls are grouped by the block they end in, each
+    // group keeping the two earliest closes in it: the earliest close that is not this
+    // control's own is the one the test is about (#13).
+    let mut first_close: Vec<usize> = vec![usize::MAX; n + 1];
+    let mut second_close: Vec<usize> = vec![usize::MAX; n + 1];
+    let mut first_owner: Vec<usize> = vec![usize::MAX; n + 1];
+    for (q, (_, qc)) in ranges.iter().enumerate() {
+        if mode.get(q) != Some(&Mode::Block) {
+            continue;
+        }
+        let Some(&(_, qe)) = span.get(q) else { continue };
+        let e = qe.min(n);
+        if *qc < first_close[e] {
+            second_close[e] = first_close[e];
+            first_close[e] = *qc;
+            first_owner[e] = q;
+        } else if *qc < second_close[e] {
+            second_close[e] = *qc;
+        }
+    }
     for k in 0..ranges.len() {
         if mode.get(k) != Some(&Mode::Block) {
             continue;
         }
         let Some(&(o, _)) = ranges.get(k) else { continue };
         let Some(&(sb, eb)) = span.get(k) else { continue };
-        let clash = ranges
-            .iter()
-            .enumerate()
-            .any(|(q, (_, qc))| q != k && *qc < o && mode.get(q) == Some(&Mode::Block) && span.get(q).is_some_and(|s| s.1 == sb));
-        if clash {
+        let e = sb.min(n);
+        let close = if first_owner[e] == k { second_close[e] } else { first_close[e] };
+        if close < o {
             let to = (sb + 1).min(eb);
             if let Some(s) = span.get_mut(k) {
                 s.0 = to;
@@ -146,13 +165,19 @@ pub fn plan(bl: &Blocks) -> Plan<'_> {
             }
             _ => {
                 for e in [events.get(*o), events.get(*c)].into_iter().flatten() {
-                    if let Some(off) = e.off() {
-                        out.inline.insert((e.block(), off));
+                    if let Some(off) = e.off()
+                        && let Some(v) = out.inline.get_mut(e.block())
+                    {
+                        v.push(off);
                     }
                 }
             }
         }
     }
+    for v in out.inline.iter_mut() {
+        v.sort_unstable();
+        v.dedup();
+    }
     out
 }
 

crates/docx/tests/content_control_save_cost.rs

@@ -0,0 +1,135 @@
+//! What saving a document with many content controls costs (#13).
+//!
+//! A form-shaped document: a control per paragraph, inline, or a control around two paragraphs.
+//! The writer used to look for each paragraph's markers in the whole set of markers, once per
+//! paragraph, and to compare every control with every other one, so the save was quadratic in the
+//! number of controls.
+
+use std::time::Instant;
+
+use wordcraft_doc::control::{ContentControl, ControlKind};
+use wordcraft_doc::para::InlineObject;
+use wordcraft_doc::props::CharProps;
+use wordcraft_doc::{Document, Paragraph, para_block};
+use wordcraft_docx::write;
+
+/// One paragraph per control, the control inline in it.
+fn inline_controls(n: usize) -> Document {
+    let mut doc = Document::new();
+    doc.body.clear();
+    let props = CharProps::default();
+    for _ in 0..n {
+        let mut p = Paragraph::with_text("value: ", props.clone());
+        let off = p.len();
+        p.insert_object(off, InlineObject::ControlStart { control: Box::new(ContentControl::new(ControlKind::RichText)) }, &props)
+            .expect("insert the start marker");
+        let off = p.len();
+        p.insert_object(off, InlineObject::ControlEnd, &props).expect("insert the end marker");
+        doc.body.push(para_block(p));
+    }
+    doc
+}
+
+/// Two paragraphs per control, the control written around both of them.
+fn block_controls(n: usize) -> Document {
+    let mut doc = Document::new();
+    doc.body.clear();
+    let props = CharProps::default();
+    for _ in 0..n {
+        let mut open = Paragraph::new();
+        let off = open.len();
+        open.insert_object(off, InlineObject::ControlStart { control: Box::new(ContentControl::new(ControlKind::RichText)) }, &props)
+            .expect("insert the start marker");
+        doc.body.push(para_block(open));
+        let mut close = Paragraph::with_text("value", props.clone());
+        let off = close.len();
+        close.insert_object(off, InlineObject::ControlEnd, &props).expect("insert the end marker");
+        doc.body.push(para_block(close));
+    }
+    doc
+}
+
+/// How much more saving the bigger document costs than the smaller one, for the bigger one
+/// `scale` times the size of the smaller one's: ~`scale` when linear, ~`scale`² when quadratic.
+/// Each is the cheapest of several runs after a warm-up, the runs alternating so a busy spell
+/// slows both sizes alike; the smaller document is saved `scale` times over per run, so both
+/// timings are long against the clock's resolution.
+fn cost_ratio(scale: usize, mut small: impl FnMut(), mut big: impl FnMut()) -> (f64, f64, f64) {
+    small();
+    big();
+    let mut small_scaled = || (0..scale).for_each(|_| small());
+    let (mut t1, mut tk) = (f64::INFINITY, f64::INFINITY);
+    for _ in 0..5 {
+        let a = Instant::now();
+        small_scaled();
+        t1 = t1.min(a.elapsed().as_secs_f64() / scale as f64);
+        let b = Instant::now();
+        big();
+        tk = tk.min(b.elapsed().as_secs_f64());
+    }
+    (t1, tk, tk / t1.max(1e-9))
+}
+
+/// A document with many inline content controls saves in the time the controls take: the writer
+/// looked for each paragraph's markers in the whole set of markers, once per paragraph, so a
+/// document with 32,000 controls took four seconds to save where it takes a tenth of one now.
+#[test]
+fn a_document_with_many_inline_content_controls_saves_in_linear_time() {
+    let scale = 4;
+    let small = inline_controls(8_000);
+    let big = inline_controls(8_000 * scale);
+    let (t1, tk, ratio) = cost_ratio(
+        scale,
+        || {
+            write(&small).expect("write");
+        },
+        || {
+            write(&big).expect("write");
+        },
+    );
+    eprintln!("t(n) {t1:.4} s, t(4n) {tk:.4} s, ratio {ratio:.1}");
+    assert!(ratio < 8.0, "four times the controls took {ratio:.1}x the time ({t1:.4} s → {tk:.4} s): not linear");
+}
+
+/// The same for the controls written around whole paragraphs: the writer compared every control
+/// with every other one to find the ones that clash, so the save was quadratic in their number.
+#[test]
+fn a_document_with_many_block_content_controls_saves_in_linear_time() {
+    let scale = 4;
+    let small = block_controls(8_000);
+    let big = block_controls(8_000 * scale);
+    let (t1, tk, ratio) = cost_ratio(
+        scale,
+        || {
+            write(&small).expect("write");
+        },
+        || {
+            write(&big).expect("write");
+        },
+    );
+    eprintln!("t(n) {t1:.4} s, t(4n) {tk:.4} s, ratio {ratio:.1}");
+    assert!(ratio < 8.0, "four times the controls took {ratio:.1}x the time ({t1:.4} s → {tk:.4} s): not linear");
+}
+
+/// The controls are written where they were: the plan that decides whether a control is written
+/// inline or around whole paragraphs gives the same answer as it did before it was made faster.
+#[test]
+fn the_controls_are_written_where_they_were() {
+    // Inline controls: both markers in the same paragraph, so the control is written in it.
+    let bytes = write(&inline_controls(50)).expect("write");
+    let back = wordcraft_docx::read(&bytes).expect("read back");
+    let ranges = back.control_ranges(wordcraft_doc::StoryRef::Body);
+    assert_eq!(ranges.len(), 50, "every control survives the round trip");
+    for r in &ranges {
+        assert_eq!(r.start.path, r.end.path, "the control is written inline: {:?}", r.start.path);
+    }
+
+    // Block controls: the markers in different paragraphs, so the control is written around them.
+    let bytes = write(&block_controls(50)).expect("write");
+    let back = wordcraft_docx::read(&bytes).expect("read back");
+    let ranges = back.control_ranges(wordcraft_doc::StoryRef::Body);
+    assert_eq!(ranges.len(), 50, "every control survives the round trip");
+    for r in &ranges {
+        assert_ne!(r.start.path, r.end.path, "the control is written around the paragraphs");
+    }
+}