perf(xlsx): stream XLSX at constant memory via OpenXML, replacing ClosedXML (rec #1) - #226
Merged
Conversation
…g ClosedXML The XLSX writers used ClosedXML, which materializes the whole workbook in memory before saving — a documented exception to the constant-memory guarantee (D14) that OOMs on a large report (~190 MB retained for a 400k-row sheet, ~20x the output file). Both the MIT single-sheet writer and the Pro multi-sheet workbook writer are now built on DocumentFormat.OpenXml's SAX OpenXmlWriter and stream at constant memory. How: - Each worksheet's XML streams straight to a per-sheet temp file via OpenXmlPartWriter (nothing buffered per row). Temp files are created 0600 on Unix (matching the zip-download hardening) with random names, and deleted on every path (finalize, exception, DisposeAsync). - FinalizeAsync hand-assembles the .xlsx with System.IO.Compression.ZipArchive in Create mode, written directly to the pipeline's write-only output stream — this deliberately bypasses System.IO.Packaging, whose ZipPackage (Update mode) buffers every part in RAM until dispose (the trap the first attempt fell into). The small fixed parts ([Content_Types].xml, rels, workbook.xml, styles.xml) are emitted by hand; each worksheet is copied from its temp file into its entry. - The stylesheet (number/date formats, bold header) is precomputed once in Initialize — one entry per distinct format, never per row — so cells reference a precomputed style index. Strings are inline (no shared-string table). Measured live memory is flat (~1.2–1.7 MB) writing 100k→2.4M rows while the output grows past 60 MB; a regression test drives the writer over a write-only FileStream and asserts <40 MB retained for 400k rows (the old writer held ~190 MB). Added the missing number/date-format assertions (C2/N0/date). The one behavioural change is the dropped column auto-fit (AdjustToContents is O(rows×cols) and can't stream). ClosedXML is removed from both writer packages; the Xlsx source-reader tests keep their own ClosedXML reference (used only to read fixtures) and all pass, validating the hand-assembled package. Updates D14 and the memory benchmark's XLSX label.
…ming writers - CodeQL: Path.Combine -> Path.Join (temp path + memory test); declaration-form await-using so the worksheet FileStream dispose is statically visible; assign the OpenXmlPartWriter straight to its array field instead of a flagged local. - S6966: await FileStream.DisposeAsync() in the async FinalizeAsync/DisposeAsync paths (ordering preserved; OpenXmlWriter is IDisposable-only in OpenXml 3.x). - S8969: drop the redundant column.Format! null-forgiving (IsNullOrEmpty is [NotNullWhen(false)]). - S3220: AppendChild(x) and empty-container-then-AppendChild instead of the params Append/constructors, across the stylesheet and inline-string builders. - IDE0066/IDE0008: switch expression in BuildCell; explicit type for one local. Stylesheet output unchanged (format-assertion test still green). No behaviour change.
|
…E0008) await-using var for the worksheet copy stream (type apparent from new FileStream), and an explicit XlsxSheetPart[] for the single-sheet assemble list.
|
Owner
Author
|
Resolved: this is a CodeQL false positive. The |
This was referenced Jul 30, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



First of the maintainer-decision recommendations from the full-project audit. Resolves the biggest enterprise gap — XLSX OOM on large reports.
The problem (D14)
Both XLSX writers used ClosedXML, which materializes the entire workbook in memory before saving — a documented exception to the constant-memory guarantee. A large report OOMs: the audit measured ~190 MB retained for a 400k-row sheet (~20× the output file), growing linearly with rows. The headline "constant memory on a 1M-row report" acceptance criterion did not hold for XLSX.
The fix
Both the MIT single-sheet
XlsxWriterand the Pro multi-sheetXlsxWorkbookWriterare rewritten onDocumentFormat.OpenXml's SAXOpenXmlWriter, streaming at constant memory. (DocumentFormat.OpenXmlwas already in CPM — the Xlsx source reader uses it — so no new dependency; ClosedXML is removed from both writer packages.)OpenXmlPartWriter(nothing buffered per row). Temp files are0600on Unix, random names, deleted on every path (finalize, exception,DisposeAsync).FinalizeAsynchand-assembles the.xlsxwithSystem.IO.Compression.ZipArchivein Create mode, written directly to the pipeline's write-only output stream. This deliberately bypassesSystem.IO.Packaging, whoseZipPackage(Update mode) buffers every part in RAM until dispose — the exact trap a first attempt fell into (it swapped ClosedXML's object model for ZipPackage's entry buffer and still grew O(rows); caught by measurement, then re-architected).Proof of constant memory
Measured live managed memory (
GC.GetTotalMemory(true)just before finalize), driving the writer over a write-onlyFileStream:Multi-sheet (3 interleaved sections): flat ~1.5 MB up to 2.4M total rows / 60 MB output. A regression test (
Writing_streams_to_disk_at_constant_memory) drives 400k rows over a write-onlyFileStreamand asserts <40 MB retained — it would have failed on the old writer.Correctness
C2→#,##0.00,N0→#,##0, date →yyyy-mm-dd)._rels/.rels,workbook.xmlr:id↔workbook.xml.relsconsistency, worksheet/styles content types) and the temp-file lifecycle by hand (the automated review agent hit a session limit mid-run) — no leak path, relationship IDs consistent, autoFilter written aftersheetDataper schema.AdjustToContentsis O(rows×cols) and can't stream — it was also flagged as wasteful in the audit). All value/type/format/header/sheet-name/auto-filter semantics preserved.Updates D14 (streaming XLSX now resolved, no longer post-MVP) and the memory benchmark's XLSX label.
First of recs #1–#6; the rest (auth startup warning, dead ABI exceptions, retry default, whole-job deadline + async error scrub, and the minor items) follow in order.