Skip to content

feat(perf): Sprint 3 — file-shape benchmark matrix - #32

Merged
wolfiesch merged 2 commits into
masterfrom
feat/perf-file-shape
Apr 28, 2026
Merged

feat(perf): Sprint 3 — file-shape benchmark matrix#32
wolfiesch merged 2 commits into
masterfrom
feat/perf-file-shape

Conversation

@wolfiesch

Copy link
Copy Markdown
Collaborator

Summary

Sprint 3 of the 7-Dimension Extension. Adds the file-shape axis
(wide / tall / sparse / many-sheets) to the perf matrix, orthogonal to
S2's dtype axis. Same total cell count, different geometries — answers
"does this library handle 100k cells the same way regardless of
whether they're square, wide, tall, sparse, or split across 100
sheets?"
.

  • 12 file-shape scenarios across 4 categories × 3 tiers (10k / 100k /
    1M; 1M gated behind --include-1m)
  • New n_sheets + sheet_pattern workload fields for many-sheets
    fan-out (bulk_write_grid and bulk_sheet_values); default
    n_sheets=1 preserves S1/S2 callers
  • excelbench perf-file-shape CLI subcommand mirroring perf-shape
  • _section_file_shape dashboard tab — per-category log-normalized
    heatmaps for read/write
  • DEC-020 logged in decisions.md
  • 21 new tests in tests/test_perf_file_shape.py

Holds dtype constant (int) so file-shape cost is orthogonal to S2's
dtype axis. Cross-product (5 shapes × 10 dtypes = 60 scenarios) is
deferred — file-shape cost is mostly dtype-independent because adapters
use the same row/col iterators regardless of cell content.

Test plan

  • `uv run pytest tests/` — 1192 passed, 32 skipped, 6 xfailed
  • `uv run ruff check src/ tests/ scripts/` — clean
  • `uv run mypy src/` — clean
  • Coverage 68.01% (gate 65%)
  • `excelbench perf-file-shape --rows 10000 --shapes wide --adapter
    openpyxl` — wide_10k completes with op_count=10000 for both
    read and write
  • `excelbench perf-file-shape --rows 100000 --shapes many_sheets
    --adapter openpyxl` — many_sheets_10x10k completes with
    op_count=100000 = 10 sheets × 10k cells, fan-out works correctly
  • `excelbench perf-file-shape --rows 10000 --shapes sparse
    --adapter openpyxl` — sparse_10pct_10k correctly counts only
    filled cells (1000 = 10000/10)
  • Full 16+ adapter run at 1M tier (deferred to bench machine; ~25
    min wall time)

Decisions

DEC-020 — File-shape parametric scenarios + n_sheets fan-out.

🤖 Generated with Claude Code

Adds the wide/tall/sparse/many-sheets file-shape axis (DEC-020),
orthogonal to S2's dtype axis. Holds dtype constant (int) so file-
shape cost is isolated from dtype cost.

12 scenarios across 4 categories:
- wide_{10k,100k,1m} — many cols, few rows
- tall_{10k,100k,1m} — many rows, few cols
- sparse_10pct_{10k,100k,1m} — 90% blank via sparse_every=10
- many_sheets_{10x10k,100x10k,1000x1k} — same total cells fanned
  across N sheets, stresses per-sheet XML overhead

Implementation:
- generate_file_shape_scenarios() in scripts/generate_throughput_fixtures.py,
  gated behind --file-shape-only and --include-1m flags
- New n_sheets / sheet_pattern workload fields in _run_workload_write,
  _run_workload_read, and _measure_write_workload_iteration. Default
  n_sheets=1 preserves single-sheet callers from S1/S2 unchanged.
- excelbench perf-file-shape CLI subcommand mirroring perf-shape, with
  --shapes filter (wide/tall/sparse/many_sheets/all) and --rows tier cap
- _section_file_shape dashboard tab with per-category log-normalized
  heatmaps; nav link gated behind has_file_shape detection
- 21 new tests in test_perf_file_shape.py covering n_sheets fan-out,
  CLI helpers, dashboard rendering, and end-to-end perf_file_shape

Smoke verified end-to-end on openpyxl:
- wide_10k: 10000 op_count for both read and write, sane ms numbers
- many_sheets_10x10k: 100000 op_count = 10 sheets × 10000 cells
- sparse_10pct_10k: 1000 op_count = 10000 grid / sparse_every=10

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 27, 2026 16:43

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce1d581654

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/excelbench/cli.py
if generator_script.exists():
if generator_script.stat().st_mtime > manifest_path.stat().st_mtime:
return True
if needs_1m:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Verify file_shape entries even when 1M tier is not requested

_file_shape_fixtures_stale only validates manifest contents under the if needs_1m branch, so default perf-file-shape runs (--rows 100000, needs_1m=False) can treat a non-file-shape manifest as fresh. A common flow is running perf-shape first (which writes only data_shape_* features to the shared default fixtures directory), then perf-file-shape; this skips regeneration and later fails with No matching features in manifest instead of auto-generating file-shape fixtures. This breaks normal sequential benchmarking workflows and can block experiments until users add --regenerate manually.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2800e84 — both _file_shape_fixtures_stale and _shape_fixtures_stale (which had the symmetric bug) now always verify at least one entry of their respective prefix is in the manifest, regardless of needs_1m. The cross-command flow now correctly regenerates fixtures when the previous command wrote a sibling-prefix-only manifest. Added two regression tests covering the case (one in each test file).

Both _shape_fixtures_stale and _file_shape_fixtures_stale only validated
manifest contents under the needs_1m branch. A fresh-mtime manifest
written by the *other* command (e.g. perf-shape writes only data_shape_*
entries) would pass the staleness check, then run_perf would fail with
"No matching features in manifest".

Both functions now unconditionally read the manifest and verify at least
one entry of their respective prefix (data_shape_* / file_shape_*) is
present. The needs_1m-specific check stays as an additional gate.

Adds two symmetric regression tests covering the cross-command case.

Addresses Codex P1 review comment on PR #32 (id 3148898068).
@wolfiesch
wolfiesch merged commit c7cf25d into master Apr 28, 2026
5 checks passed
@wolfiesch
wolfiesch deleted the feat/perf-file-shape branch April 28, 2026 01:17
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