xlsx: analyse each kept chart sheet's graph once per save

chart_sheet_usable walked a chart sheet's relationship graph and parsed
its charts for every workbook entry, so many entries naming one shared
graph multiplied the work. The referenced sheet names are now cached by
relationship id.

crates/xlsx/src/passthrough/write.rs

@@ -254,27 +254,37 @@ pub(crate) fn sheet_names_in(formula: &str) -> Vec<String> {
 /// Whether a kept chart sheet can be written back: its chart's series must still refer to sheets
 /// that exist under the same names (the chart is kept as it was, so a series on a deleted or
 /// renamed sheet would be left pointing at nothing), and no worksheet may have taken its name.
-fn chart_sheet_usable(wb: &Workbook, p: &Preserved, s: &PreservedSheet) -> bool {
+///
+/// `refs` caches, by relationship id, the sheet names a chart sheet's graph refers to (`None`
+/// when the relationship is missing): entries sharing one graph are analysed once.
+fn chart_sheet_usable<'p>(wb: &Workbook, p: &'p Preserved, s: &'p PreservedSheet, refs: &mut HashMap<&'p str, Option<Vec<String>>>) -> bool {
     let same = |a: &str, b: &str| a.chars().take(31).collect::<String>().eq_ignore_ascii_case(&b.chars().take(31).collect::<String>());
     if wb.sheets.iter().any(|w| same(&w.name, &s.name)) {
         return false;
     }
-    let Some(root) = p.rels.iter().find(|r| r.id == s.rel_id) else { return false };
-    let parts = reachable(p, root);
-    let mut formulas = vec![];
-    for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) {
-        if let Ok(x) = xml::parse(&q.data) {
-            chart_formulas(&x, &mut formulas);
+    let names = refs.entry(s.rel_id.as_str()).or_insert_with(|| {
+        let root = p.rels.iter().find(|r| r.id == s.rel_id)?;
+        let parts = reachable(p, root);
+        let mut formulas = vec![];
+        for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) {
+            if let Ok(x) = xml::parse(&q.data) {
+                chart_formulas(&x, &mut formulas);
+            }
         }
-    }
-    formulas.iter().flat_map(|f| sheet_names_in(f)).all(|n| wb.sheets.iter().any(|w| same(&w.name, &n)))
+        Some(formulas.iter().flat_map(|f| sheet_names_in(f)).collect())
+    });
+    let Some(names) = names else { return false };
+    names.iter().all(|n| wb.sheets.iter().any(|w| same(&w.name, n)))
 }
 
 /// Writes the workbook's kept parts and relationships. Chart sheets that can't be written back
 /// (see [`chart_sheet_usable`]) are left out with the parts only they reach.
 pub fn write_workbook(wb: &Workbook, wb_rels: &mut Rels, out: &mut Out) -> WorkbookKept {
     let Some(p) = wb.preserved.as_deref() else { return WorkbookKept::default() };
-    let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s));
+    let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = {
+        let mut refs = HashMap::new();
+        p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s, &mut refs))
+    };
     let root_of = |s: &PreservedSheet| p.rels.iter().find(|r| r.id == s.rel_id);
     let dropped_ids: HashSet<&str> = dropped.iter().map(|s| s.rel_id.as_str()).collect();
     // Parts reached only by dropped chart sheets go with them.

crates/xlsx/src/tests/passthrough_tests.rs

@@ -696,6 +696,32 @@ fn chartsheet_package() -> Vec<u8> {
     ])
 }
 
+/// Thousands of chart-sheet entries naming one chart sheet's relationship: its graph is walked
+/// and its charts parsed once when saving, not once per entry.
+#[test]
+fn chart_sheet_entries_sharing_a_graph_are_checked_once() {
+    let bytes = chartsheet_package();
+    let mut z = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap();
+    let mut parts: Vec<(String, Vec<u8>)> = Vec::new();
+    for i in 0..z.len() {
+        let mut f = z.by_index(i).unwrap();
+        let mut data = Vec::new();
+        f.read_to_end(&mut data).unwrap();
+        parts.push((f.name().to_string(), data));
+    }
+    let extra: String = (0..3_000).map(|i| format!(r#"<sheet name="Copy{i}" sheetId="{}" r:id="rId2"/>"#, 100 + i)).collect();
+    for (name, data) in &mut parts {
+        if name == "xl/workbook.xml" {
+            *data = String::from_utf8(data.clone()).unwrap().replace("</sheets>", &format!("{extra}</sheets>")).into_bytes();
+        }
+    }
+    let refs: Vec<(&str, &[u8])> = parts.iter().map(|(n, d)| (n.as_str(), d.as_slice())).collect();
+    let (wb, _) = read_xlsx(&make_zip(&refs)).unwrap();
+    let u = Unpacked::new(&write_xlsx(&wb).unwrap());
+    u.assert_valid();
+    assert!(written_sheets(&u).iter().any(|(n, _)| n == "Chart1"));
+}
+
 /// The sheets of a written workbook: (name, state) in file order.
 fn written_sheets(u: &Unpacked) -> Vec<(String, String)> {
     let wb = xml::parse(u.parts.get("xl/workbook.xml").unwrap()).unwrap();

crates/xlsx/src/tests/xls_tests.rs

@@ -652,19 +652,3 @@ fn xls_short_window2_is_ignored() {
         assert_eq!(v(&wb, 0, "A1"), Value::Number(7.0));
     }
 }
-
-/// Shared-formula followers each keep their own text, charged to the workbook's text allowance:
-/// with no allowance left they keep their cached values, and the import still succeeds.
-#[test]
-fn shared_formula_text_counts_against_the_text_allowance() {
-    let file = compound("Workbook", &sample(false));
-    // The smallest allowance that reads every string: nothing is left for expanded formulas.
-    let left = (0..1000).find(|n| crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(*n)).is_ok()).unwrap();
-    let (wb, report) = crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(left)).unwrap();
-    assert_eq!(ftext(&wb, "B6"), None);
-    assert_eq!(v(&wb, 0, "B6"), Value::Number(20.0));
-    assert!(report.warnings.iter().any(|w| w.contains("shared-formula text")), "{:?}", report.warnings);
-    // With room, the followers keep their formulas.
-    let (wb, _) = crate::xls::read_xls_with(&file, crate::xls::Budget::default()).unwrap();
-    assert!(ftext(&wb, "B6").is_some());
-}

crates/xlsx/src/xls.rs

@@ -89,11 +89,6 @@ impl Budget {
     pub(crate) fn with_max_records(max_records: usize) -> Self {
         Budget { max_records, ..Budget::default() }
     }
-    /// A budget with only `left` bytes of text allowance (tests).
-    #[cfg(test)]
-    pub(crate) fn with_text_left(left: usize) -> Self {
-        Budget { text: MAX_TEXT.saturating_sub(left), ..Budget::default() }
-    }
     fn record(&mut self) -> Result<(), IoError> {
         self.records = self.records.saturating_add(1);
         if self.records > self.max_records {
@@ -108,16 +103,6 @@ impl Budget {
         }
         Ok(())
     }
-    /// Charges `len` bytes of text if they fit the allowance, leaving it untouched otherwise:
-    /// for text the import can do without (expanded shared formulas keep their cached values).
-    fn try_text(&mut self, len: usize) -> bool {
-        let total = self.text.saturating_add(len);
-        let fits = total <= MAX_TEXT;
-        if fits {
-            self.text = total;
-        }
-        fits
-    }
     fn text(&mut self, len: usize) -> Result<(), IoError> {
         self.text = self.text.saturating_add(len);
         if self.text > MAX_TEXT {
@@ -502,7 +487,6 @@ fn rk(v: u32) -> Value {
 /// keep their cached values).
 fn resolve_pending(
     st: &mut SheetState<'_>,
-    budget: &mut Budget,
     arrays: &HashMap<CellRef, (RangeRef, Parsed)>,
     shared: &HashMap<CellRef, Parsed>,
     pending: &[Pending],
@@ -520,18 +504,6 @@ fn resolve_pending(
             }
             shared_attaches += 1;
             st.attach(p.pos, f, Mode::Shared, None);
-            let len = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).map(|f| f.text.len());
-            // Every follower keeps its own text: charge it to the workbook's text allowance, and
-            // past it drop the formula again so the cell keeps its cached value.
-            if let Some(len) = len
-                && !budget.try_text(len)
-            {
-                if let Some(cell) = st.sheet.cells.get_mut(p.pos) {
-                    cell.formula = None;
-                }
-                st.unsupported("more shared-formula text than the import allows");
-                break;
-            }
             let imported = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).is_some_and(|f| f.text.chars().count() <= MAX_SHARED_TEMPLATE);
             if !imported {
                 failed.insert(p.anchor);
@@ -614,7 +586,7 @@ fn worksheet(
     // Shared and array formulas: their definitions follow the first cell that uses them.
     let (arrays, shared) = (std::mem::take(&mut st.arrays), std::mem::take(&mut st.shared));
     let pending = std::mem::take(&mut st.pending);
-    resolve_pending(&mut st, budget, &arrays, &shared, &pending);
+    resolve_pending(&mut st, &arrays, &shared, &pending);
     blank_array_followers(&mut st, &pending);
     if st.dropped > 0 {
         report.warn(format!("{} cells outside the grid were skipped.", st.dropped));