Skip to content

perf(xlsx): hash-indexed style interning - #240

Open
eliahilse wants to merge 1 commit into
mainfrom
perf/xlsx-style-interning
Open

perf(xlsx): hash-indexed style interning#240
eliahilse wants to merge 1 commit into
mainfrom
perf/xlsx-style-interning

Conversation

@eliahilse

Copy link
Copy Markdown
Member

TL;DR:

Summary:

  • style pools intern through hash indexes instead of linear scans; every memo hit re-verifies against the live pool, so public same-length mutations can never yield a stale index
  • number-format allocation validates candidates against the live table and fails explicitly (NumFmtTableFull) when ids are exhausted instead of silently colliding
  • range formatting stages against a cloned stylesheet and commits only on success — mid-range exhaustion leaves the workbook untouched
  • note for review: Stylesheet gains private interning caches, which prevents exhaustive struct-literal construction downstream (changeset carries a minor bump)

Test plan:

  • cargo test -p betteroffice-xlsx-model -p betteroffice-xlsx-ops green
  • apply formatting to a large range on a style-heavy workbook — timing before/after
  • round-trip styles.xml byte-fidelity on real workbooks

@openooxml-bot

openooxml-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA — thank you! ✍️ ✅

Posted by the CLA bot.

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces linear XLSX style interning with hash-indexed, live-pool-validated caches and makes custom number-format exhaustion explicit and atomic.

  • Adds private, serialization-neutral caches for fonts, fills, borders, cell formats, and number formats.
  • Introduces fallible number-format allocation and propagates exhaustion through operation and collaboration paths.
  • Stages range formatting against cloned style state before committing cells.
  • Adds tests for cache invalidation, duplicate IDs, serialization, exhaustion, and transactional behavior.

Confidence Score: 5/5

The PR appears safe to merge; no concrete changed-code defect remains after checking cache mutation, allocation exhaustion, operation atomicity, undo behavior, and dependency changes.

Cache hits are checked against live pools, custom-format allocation rejects exhausted or colliding IDs, production callers propagate that failure, and range operations commit only after complete staging succeeds.

Important Files Changed

Filename Overview
crates/xlsx-model/src/styles.rs Adds validated style-pool caches, borrowed format resolution, and collision-safe fallible custom number-format allocation with extensive mutation and serialization tests.
crates/xlsx-ops/src/apply.rs Propagates number-format exhaustion and stages complete range-format changes before committing styles, cells, and inverses.
crates/betteroffice-xlsx/src/authority.rs Adapts collaborative style materialization to propagate the newly fallible style-interning result.
crates/xlsx-parse/src/tests.rs Updates style-writing coverage for the fallible interning API while preserving existing package-fidelity assertions.
crates/xlsx-render/src/lib.rs Updates rendering tests to unwrap the new fallible style-interning result.
crates/xlsx-model/Cargo.toml Adds serde_json only as a development dependency for stylesheet serialization tests.
Cargo.lock Records the xlsx-model serde_json development dependency without changing the flagged transitive dependency versions.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Range formatting operation] --> B[Clone stylesheet]
  B --> C[Resolve each current cell format]
  C --> D[Apply requested formatting]
  D --> E[Intern through validated hash caches]
  E --> F{Number-format ID available?}
  F -->|No| G[Return NumFmtTableFull]
  G --> H[Discard staged state]
  F -->|Yes| I[Stage cell and style indices]
  I --> J{All cells complete?}
  J -->|No| C
  J -->|Yes| K[Commit stylesheet]
  K --> L[Commit cells and build inverse]
Loading

Reviews (1): Last reviewed commit: "perf(xlsx): hash-indexed style interning" | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant