perf(writer): remove per-cell serializer allocations - #12
Merged
Conversation
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.
Use stack-backed A1 references, direct numeric formatting and direct shared-string escaping. Preserve legacy floating-point formatting and XML whitespace/entity semantics.
Before / after evidence
Baseline
43d93c63fa6ecfd238134d2a76ba6f270b18fd19; candidate922fe9af37d62d730603b1b514c37720a6fa17a7. Both use identicalwriter_hotpathsbenchmark source (added by the serializer PR; copy it into the original baseline checkout before building). Linux x86_64 AMD EPYC 9V74, Rust 1.98.1, checked-in release fat LTO / one codegen unit. Native complete save (XML + ZIP), with workbook population excluded from timed and allocation-counted regions.Numeric saves remove exactly 300,000 allocation calls and improve 1.362× at 100k cells. Unique strings remove 500,000 calls and improve 1.325×. Peak RSS is largely unchanged; the benefit is latency and allocation traffic.
Ordinary rows are 100,000 total cells: two ABBA cycles, seven measured saves after a warmup per invocation, 28 samples per side/workload. The
large-*row is 1,000,000 total cells with one measured save per side after warmup: a directional scale/RSS check, not a robust latency claim. One sheet uses 5,000×20 cells normally, 50,000×20 at scale. Peak RSS comes from an isolated child process and includes population, warmup and save; cumulative allocated bytes are allocation traffic, not peak live memory. No competing task builds or benchmarks ran during measurement.Every uncompressed ZIP member matched SHA256 in every run. Entire ZIP archives were byte-identical. Output sizes and archive/member hashes are in the raw evidence.
Raw measurements, commands and binary hashes.
Correctness and reproduction
cargo test -p wolfxl-writerpasses on the candidate. Added all-column address boundary equivalence plus 10,000 deterministic floating-point bit-pattern checks, including NaN, infinities and integer boundaries. Existing writer roundtrip, streaming and feature tests remain green. The baseline already increments shared-string reference counts on repeated saves; tests compare matching save ordinals rather than claim an unchanged string workbook is byte-stable across saves.Native measurements do not imply the same percentage improvement for complete Python workloads.
CI follow-up
Workspace CI exposed pre-existing formatting in
crates/wolfxl-cli/src/render.rs. A whitespace-only follow-up fixes its two expressions and synchronizes the stack. The measured writer code and benchmark binaries remain unchanged; the baseline/candidate SHAs above identify the measured revisions.