perf(odf): halve what a spreadsheet costs to decode, and stop two repeat bombs

AGENTS.md

@@ -106,6 +106,17 @@ cmake --build cmake-build-relwithdebinfo --target translate  # CLI: file → HTM
   need the data, and several gigabytes should not arrive because someone turned
   tests on. The `update_test_data` target moves existing checkouts onto the
   pins. The two private repositories need credentials.
+- **pugixml is built in compact mode** (`PUGIXML_COMPACT`, set on the imported
+  target in `CMakeLists.txt`, paired with `pugixml/*:header_only` in
+  `conanfile.py`). The odf/ooxml/svg/xml engines keep the parsed DOM resident as
+  their backing store, so its size *is* the document's: a 12-byte node instead
+  of 64 took a 297 MB `content.xml` from 594 MB to 116 MB. The define is ABI
+  affecting and mixing it across translation units is silent corruption, not a
+  link error — hence the imported target, and no prebuilt library to mismatch
+  against. A new target that includes `pugixml.hpp` has to get it too, and the
+  installed internal headers expose pugixml types, so `odr` carries the define
+  INTERFACE and `package_info` declares it. `util/xml_util.cpp` asserts the
+  layout it compiled against.
 
 ## Releasing
 

CHANGELOG.md

@@ -26,6 +26,20 @@ The release run heads these entries with the version and opens a fresh
   `text/html` and `application/xhtml+xml`. Classification only: no `open`, no
   `translate_html`, and never detected from its bytes.
 
+- **Breaking** An ods sheet stores a repeated cell once, at the range it
+  covers, rather than once per position: `SheetCell::position()` reports the
+  anchor of that range instead of the position the cell was looked up at, and
+  `Sheet::cell()` hands back the same element everywhere in it.
+- An ods `table:number-columns-repeated` beside a `table:number-rows-repeated`
+  no longer inflates — the grid's own `1048576 × 1024` asked for three billion
+  elements out of four hundred bytes.
+- An xlsx `<mergeCell>` whose `ref` names more positions than the sheet has
+  cells opens again; `ref="A1:XFD1048576"` used to visit all 17 billion.
+- Decoding a spreadsheet costs about half the memory it did — pugixml is built
+  in compact mode and the ods sheet index is sorted vectors rather than maps.
+  A consumer that includes an internal header now needs `PUGIXML_COMPACT`, and
+  the conan package declares it.
+
 ## v6.11.0 - 2026-08-29
 
 - No view declares `<base target="_blank">` any more. A link back into what

CMakeLists.txt

@@ -54,6 +54,12 @@ endif ()
 add_compile_options("$<$<CXX_COMPILER_ID:MSVC>:/utf-8>")
 
 find_package(pugixml REQUIRED)
+# A compact `xml_node_struct` is 12 bytes rather than 64, and the engines keep
+# the dom resident as their backing store. The define is ABI affecting and a
+# mismatch is silent, so it rides on the imported target and is paired with
+# `header_only` in conanfile.py. See AGENTS.md.
+set_property(TARGET pugixml::pugixml APPEND PROPERTY
+        INTERFACE_COMPILE_DEFINITIONS PUGIXML_COMPACT)
 find_package(md4c REQUIRED)
 find_package(miniz REQUIRED)
 find_package(cryptopp REQUIRED)
@@ -311,6 +317,10 @@ target_link_libraries(odr
         uchardet::uchardet
         utf8::cpp
 )
+# pugixml is linked PRIVATE, but the installed internal headers expose its
+# types - so a consumer of those needs the same layout. `cpp_info.defines` in
+# conanfile.py is the same thing for the conan package.
+target_compile_definitions(odr INTERFACE PUGIXML_COMPACT)
 
 if (ODR_WITH_HTTP_SERVER)
     find_package(httplib REQUIRED)

conanfile.py

@@ -38,6 +38,9 @@ class OpenDocumentCoreConan(ConanFile):
         "with_apple": False,
         "with_wasm": False,
         "bundle_assets": False,
+        # paired with PUGIXML_COMPACT in CMakeLists.txt: no prebuilt library
+        # to mismatch against the node layout the define changes
+        "pugixml/*:header_only": True,
     }
 
     exports_sources = ["apple/*", "cli/*", "cmake/*", "jni/*", "python/*", "resources/dist/*", "wasm/*", "src/*", "CMakeLists.txt"]
@@ -103,3 +106,6 @@ def package(self):
 
     def package_info(self):
         self.cpp_info.libs = ["odr"]
+        # the installed internal headers expose pugixml types, whose layout
+        # PUGIXML_COMPACT changes
+        self.cpp_info.defines = ["PUGIXML_COMPACT"]

src/odr/internal/odf/AGENTS.md

@@ -19,13 +19,15 @@ Unlike the binary engines (`oldms/`, `pdf/`), which parse bytes into their own
 structures, ODF keeps the parsed **`content.xml` / `styles.xml` DOMs resident**
 (`Document` owns `m_content_xml`, `m_styles_xml`). The `ElementRegistry` is a
 thin *index over* that DOM: every `ElementRegistry::Element` stores a live
-`pugi::xml_node` alongside its tree ids. Style/content/attribute access always
+`pugi::xml_node` alongside its tree ids. The DOM being the backing store, and
+not a parse artifact, is why pugixml is built in compact mode — see the top
+level [`AGENTS.md`](../../../../AGENTS.md). Style/content/attribute access always
 goes back to the node. **This is why ODF alone can edit and save**: a text edit
 is a local DOM splice, and `save` re-serialises the mutated tree. The other
 engines throw away the source, so their models are read-only.
 
-Everything else follows the shared registry/adapter pattern: flat
-`std::vector<Element>`, id = index + 1, `null_element_id == 0`, parent/child/
+Everything else follows the shared registry/adapter pattern: a flat element
+store, id = index + 1, `null_element_id == 0`, parent/child/
 sibling ids, per-subtype side maps (`m_texts`, `m_tables`, `m_sheets`,
 `m_sheet_cells`). One mega `ElementAdapter` multiply-inherits every abstract
 per-type adapter and dispatches by returning `this`/`nullptr` on `element_type`.
@@ -46,11 +48,25 @@ element and breaks the run. Reading expands `text:s`→N spaces (via `text:c`),
 `text:tab`→`\t`.
 
 **Sheets are modelled sparsely, off-tree.** A `Sheet` side-struct holds
-position-keyed `columns`/`rows`/`cells` maps rather than a child chain. Repeated
-columns/rows/cells are stored **once** at a range key and resolved with
-`lookup_greater_than`, so a 5000-row `number-columns-repeated` doesn't inflate.
-Only non-empty cells get a real `sheet_cell` Element; empty ones are recorded as
-repeated ranges. Cells carry a `TablePosition` + `is_repeated` flag.
+`columns`/`rows`/`cells` keyed by position rather than a child chain. Repeated
+columns/rows/cells are stored **once**, at the *end* of the range they repeat
+over, and resolved with an upper bound — whether or not the cell has content.
+That last part is load-bearing: expanding a repeat per position let a 400-byte
+document ask for a `1048576 × 1024` grid of elements, both counts being legal
+repeats. Only non-empty cells get a real `sheet_cell` Element; empty ones are
+recorded as ranges alone. Cells carry a `TablePosition` (the anchor of the
+range, not each position it covers) + `is_repeated` flag.
+
+The three containers are **sorted vectors, not maps**: parsing appends in
+document order, so the keys only grow, and a rb-tree node costs more than the 12
+bytes it carries — on a million-row sheet, 275 MB of maps against 89 MB of
+vectors. The cells of every row live in one array per sheet, each row recording
+where its own run starts, so a sheet is two allocations rather than one per row.
+`register_cell` therefore has to follow its row's `register_row`.
+
+The elements themselves are a `std::deque`: `create_element` hands back a
+reference the parser holds on to, and a vector both invalidates it and peaks
+holding two copies.
 
 **Styles resolve to a flattened `ResolvedStyle`, eagerly.** `StyleRegistry`
 first builds name→node indices from *both* files (automatic and named styles land

src/odr/internal/odf/odf_document.cpp

@@ -469,7 +469,7 @@ class ElementAdapter final : public abstract::ElementAdapter,
     TableDimensions result;
 
     TableCursor cursor;
-    for (const pugi::xml_node row : table_rows(node)) {
+    for_each_table_row(node, [&](const pugi::xml_node row) {
       const auto rows_repeated =
           row.attribute("table:number-rows-repeated").as_uint(1);
       cursor.add_row(rows_repeated);
@@ -492,7 +492,7 @@ class ElementAdapter final : public abstract::ElementAdapter,
           result.columns = new_cols;
         }
       }
-    }
+    });
 
     return result;
   }
@@ -731,20 +731,20 @@ class ElementAdapter final : public abstract::ElementAdapter,
     TableDimensions result;
     TableCursor cursor;
 
-    for (const pugi::xml_node column : table_columns(node)) {
+    for_each_table_column(node, [&](const pugi::xml_node column) {
       const auto columns_repeated =
           column.attribute("table:number-columns-repeated").as_uint(1);
       cursor.add_column(columns_repeated);
-    }
+    });
 
     result.columns = cursor.column();
     cursor = {};
 
-    for (const pugi::xml_node row : table_rows(node)) {
+    for_each_table_row(node, [&](const pugi::xml_node row) {
       const auto rows_repeated =
           row.attribute("table:number-rows-repeated").as_uint(1);
       cursor.add_row(rows_repeated);
-    }
+    });
 
     result.rows = cursor.row();
 

src/odr/internal/odf/odf_element_registry.cpp

@@ -1,7 +1,6 @@
 #include <odr/internal/odf/odf_element_registry.hpp>
 
-#include <odr/internal/util/map_util.hpp>
-
+#include <algorithm>
 #include <stdexcept>
 
 namespace odr::internal::odf {
@@ -246,13 +245,26 @@ void ElementRegistry::check_sheet_cell_id(const ElementIdentifier id) const {
 void ElementRegistry::Sheet::register_column(const std::uint32_t column,
                                              const std::uint32_t repeated,
                                              const pugi::xml_node element) {
-  columns[column + repeated] = {.node = element};
+  const std::uint32_t end = column + repeated;
+  if (!columns.empty() && columns.back().end >= end) {
+    columns.back() = {.end = end, .node = element};
+    return;
+  }
+  columns.push_back({.end = end, .node = element});
 }
 
 void ElementRegistry::Sheet::register_row(const std::uint32_t row,
                                           const std::uint32_t repeated,
                                           const pugi::xml_node element) {
-  rows[row + repeated].node = element;
+  const std::uint32_t end = row + repeated;
+  if (!rows.empty() && rows.back().end >= end) {
+    rows.back().end = end;
+    rows.back().node = element;
+    return;
+  }
+  rows.push_back({.end = end,
+                  .first_cell = static_cast<std::uint32_t>(cells.size()),
+                  .node = element});
 }
 
 void ElementRegistry::Sheet::register_cell(const std::uint32_t column,
@@ -261,40 +273,56 @@ void ElementRegistry::Sheet::register_cell(const std::uint32_t column,
                                            const std::uint32_t rows_repeated,
                                            const pugi::xml_node element,
                                            const ElementIdentifier element_id) {
-  Cell &cell = rows[row + rows_repeated].cells[column + columns_repeated];
-  cell.node = element;
-  cell.element_id = element_id;
+  const std::uint32_t row_end = row + rows_repeated;
+  if (rows.empty() || rows.back().end != row_end) {
+    throw std::invalid_argument(
+        "ElementRegistry::Sheet::register_cell: no row to hold the cell");
+  }
+
+  const std::uint32_t end = column + columns_repeated;
+  if (cells.size() > rows.back().first_cell && cells.back().end >= end) {
+    cells.back() = {.end = end, .node = element, .element_id = element_id};
+    return;
+  }
+  cells.push_back({.end = end, .node = element, .element_id = element_id});
 }
 
+namespace {
+
+/// The entry whose range covers @p at, i.e. the first one ending past it.
+template <typename Entry>
+const Entry *lookup(const std::span<const Entry> entries,
+                    const std::uint32_t at) {
+  const auto it = std::ranges::upper_bound(entries, at, {}, &Entry::end);
+  return it != std::end(entries) ? &*it : nullptr;
+}
+
+} // namespace
+
 const ElementRegistry::Sheet::Column *
 ElementRegistry::Sheet::column(const std::uint32_t column) const {
-  if (const auto it = util::map::lookup_greater_than(columns, column);
-      it != std::end(columns)) {
-    return &it->second;
-  }
-  return nullptr;
+  return lookup<Column>(columns, column);
 }
 
 const ElementRegistry::Sheet::Row *
 ElementRegistry::Sheet::row(const std::uint32_t row) const {
-  if (const auto it = util::map::lookup_greater_than(rows, row);
-      it != std::end(rows)) {
-    return &it->second;
-  }
-  return nullptr;
+  return lookup<Row>(rows, row);
 }
 
 const ElementRegistry::Sheet::Cell *
 ElementRegistry::Sheet::cell(const std::uint32_t column,
                              const std::uint32_t row) const {
-  if (const Row *row_entry = this->row(row); row_entry != nullptr) {
-    const auto &cells = row_entry->cells;
-    if (const auto cell_it = util::map::lookup_greater_than(cells, column);
-        cell_it != std::end(cells)) {
-      return &cell_it->second;
-    }
-  }
-  return nullptr;
+  const Row *row_entry = this->row(row);
+  return row_entry != nullptr ? lookup<Cell>(row_cells(*row_entry), column)
+                              : nullptr;
+}
+
+std::span<const ElementRegistry::Sheet::Cell>
+ElementRegistry::Sheet::row_cells(const Row &row) const {
+  const auto next = &row + 1;
+  const std::size_t end =
+      next != rows.data() + rows.size() ? next->first_cell : cells.size();
+  return {cells.data() + row.first_cell, end - row.first_cell};
 }
 
 [[nodiscard]] pugi::xml_node

src/odr/internal/odf/odf_element_registry.hpp

@@ -7,7 +7,9 @@
 #include <odr/table_dimension.hpp>
 #include <odr/table_position.hpp>
 
+#include <deque>
 #include <map>
+#include <span>
 #include <unordered_map>
 #include <vector>
 
@@ -36,25 +38,33 @@ class ElementRegistry final {
     pugi::xml_node last;
   };
 
+  /// Columns, rows and cells keyed by the *end* of the range they repeat over
+  /// and resolved with an upper bound, so a run of 5000 is one entry. Sorted
+  /// vectors, not maps: parsing appends in document order. The cells of every
+  /// row live in one array per sheet, each row holding where its run starts.
   struct Sheet final {
     struct Column final {
+      std::uint32_t end{0};
       pugi::xml_node node;
     };
 
     struct Cell final {
+      std::uint32_t end{0};
       pugi::xml_node node;
       ElementIdentifier element_id{null_element_id};
     };
 
     struct Row final {
+      std::uint32_t end{0};
+      std::uint32_t first_cell{0};
       pugi::xml_node node;
-      std::map<std::uint32_t, Cell> cells;
     };
 
     TableDimensions dimensions;
 
-    std::map<std::uint32_t, Column> columns;
-    std::map<std::uint32_t, Row> rows;
+    std::vector<Column> columns;
+    std::vector<Row> rows;
+    std::vector<Cell> cells;
 
     ElementIdentifier first_shape_id{null_element_id};
     ElementIdentifier last_shape_id{null_element_id};
@@ -63,6 +73,7 @@ class ElementRegistry final {
                          pugi::xml_node element);
     void register_row(std::uint32_t row, std::uint32_t repeated,
                       pugi::xml_node element);
+    /// Has to follow the @ref register_row of the row it belongs to.
     void register_cell(std::uint32_t column, std::uint32_t row,
                        std::uint32_t columns_repeated,
                        std::uint32_t rows_repeated, pugi::xml_node element,
@@ -73,6 +84,9 @@ class ElementRegistry final {
     [[nodiscard]] const Cell *cell(std::uint32_t column,
                                    std::uint32_t row) const;
 
+    /// The cells of @p row - one of this sheet's `rows` - in column order.
+    [[nodiscard]] std::span<const Cell> row_cells(const Row &row) const;
+
     [[nodiscard]] pugi::xml_node column_node(std::uint32_t column) const;
     [[nodiscard]] pugi::xml_node row_node(std::uint32_t row) const;
     [[nodiscard]] pugi::xml_node cell_node(std::uint32_t column,
@@ -126,7 +140,10 @@ class ElementRegistry final {
   void append_sheet_cell(ElementIdentifier sheet_id, ElementIdentifier cell_id);
 
 private:
-  std::vector<Element> m_elements;
+  /// A deque, not a vector: `create_element` hands back a reference the parser
+  /// holds on to, and a vector both invalidates it and peaks holding two
+  /// copies.
+  std::deque<Element> m_elements;
   std::unordered_map<ElementIdentifier, Text> m_texts;
   std::unordered_map<ElementIdentifier, Table> m_tables;
   std::unordered_map<ElementIdentifier, Sheet> m_sheets;