feat(perf): Sprint 2 — data-shape benchmark matrix (10 dtypes × 4 tiers) - #30
feat(perf): Sprint 2 — data-shape benchmark matrix (10 dtypes × 4 tiers)#30wolfiesch wants to merge 1 commit into
Conversation
Extends the throughput-fixture pipeline with a parametric (dtype × scale) matrix so the dashboard can answer "how does library X handle int vs string vs formula at 1M cells?" — the most actionable question for users picking between libraries. What ships: - 7 new value_type branches in _run_workload_write (float, date, datetime, boolean, formula_simple, formula_cross_sheet, mixed_realistic); string-short/long fold into existing string op via string_length. - generate_data_shape_scenarios() in the throughput generator: 10 dtypes × 3 tiers (1k/10k/100k) by default, +1M tier behind --include-1m. New --shape-only flag for fast-iteration runs. - excelbench perf-shape subcommand with --rows/--types/--regenerate/ --memory-mode (inherits Sprint 1's memory-mode plumbing). - _section_data_shape dashboard heatmap (read + write), per-dtype-column log-scale color so slow columns don't wash out fast ones. - DEC-019 with mixed_realistic 60/30/5/3/2 ratio rationale and fixtures/synthetic_calibration/sample_set.md calibration provenance. Branched off feat/perf-mem-honesty (S1) since #28 hasn't merged; once it does, this PR will retarget master. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03e9e0bf33
ℹ️ 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".
| elif value_type == "formula_simple": | ||
| row_vals.append(f"=SUM(A{row_num}:B{row_num})") | ||
| elif value_type == "formula_cross_sheet": | ||
| row_vals.append(f"=Sheet2!A{row_num}") |
There was a problem hiding this comment.
The new formula_cross_sheet branch writes =Sheet2!A{row} formulas, but the write benchmark setup only creates the primary workload sheet before _run_workload_write runs. That means this scenario is benchmarked against dangling cross-sheet references (or adapter-specific failures) instead of a real cross-sheet workbook, so reported write timings for this dtype can be misleading. Initialize/populate Sheet2 for this value type before bulk-writing values.
Useful? React with 👍 / 👎.
| gen_cmd = [ | ||
| sys.executable, | ||
| str(generator_script), | ||
| "--shape-only", |
There was a problem hiding this comment.
Regeneration in perf-shape always invokes the generator with --shape-only, writing into the configured fixtures directory. In --shape-only mode the generator emits a manifest containing only data-shape entries, so running this command overwrites any existing mixed/legacy throughput manifest at that path. This can break subsequent perf runs that rely on non-shape features from the same fixture set (missing features or incomplete experiment coverage) until fixtures are regenerated without --shape-only.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Sprint 2 of the perf benchmarking “7-Dimension Extension”, adding a new “data-shape” benchmark matrix (dtype × scale) across fixture generation, CLI execution, perf runner value generation, and dashboard visualization.
Changes:
- Add data-shape fixture generation (10 dtypes × tiers, with 1M gated) and update throughput fixtures docs/versioning.
- Add
excelbench perf-shapeCLI command that selects dtype×tier features, regenerates fixtures when stale, and runs the perf harness (including Sprint 1--memory-mode). - Extend the perf runner to write additional dtype variants and update the HTML dashboard with a conditional “Data Shape” heatmap tab.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/excelbench/results/html_dashboard.py |
Adds conditional nav link + new “Data Shape” heatmap section parsing data_shape_* perf entries. |
src/excelbench/perf/runner.py |
Extends bulk-write value generation to support new value types used by data-shape scenarios. |
src/excelbench/cli.py |
Adds perf-shape command plus helper logic for dtype/tier selection and fixture staleness detection. |
scripts/generate_throughput_fixtures.py |
Adds generator for data-shape fixtures, workload specs, and --shape-only / --include-1m flags. |
fixtures/throughput_xlsx/README.md |
Documents the new data-shape matrix, tiers, dtypes, and generator invocation. |
fixtures/synthetic_calibration/sample_set.md |
Documents calibration provenance for mixed_realistic. |
decisions.md |
Adds DEC-019 documenting rationale and design for data-shape scenarios + mixed_realistic ratio. |
TRACKER.md |
Marks Sprint 2 as “In Progress”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _DATA_SHAPE_TIER_CAPS: list[tuple[str, int]] = [ | ||
| ("1k", 1_000), | ||
| ("10k", 10_000), | ||
| ("100k", 100_000), | ||
| ("1m", 1_000_000), |
There was a problem hiding this comment.
_DATA_SHAPE_TIER_CAPS uses 100,000 for the "100k" tier, but the fixtures/generator define that tier as 316×316 = 99,856 cells. This makes --rows 99856 (the actual fixture size) incorrectly exclude the "100k" tier. Align the cap with the real cell count (or derive caps from the generator tiers) to keep feature selection accurate.
| filled = (linear_idx % sparse_every) == 0 | ||
| # Spreadsheet row number for formula references | ||
| # (1-based, offset by start cell row). | ||
| row_num = r0 + r_idx | ||
| linear_idx_val = linear_idx |
There was a problem hiding this comment.
In bulk_write_grid, row_num is constant for a given row but is recomputed inside the inner column loop. Since this code runs for up to 1M cells and is included in the timed "exercise" phase, consider moving row_num (and any per-row cached strings) outside the column loop to reduce Python overhead and keep write benchmarks focused on adapter performance.
| elif value_type == "formula_simple": | ||
| row_vals.append(f"=SUM(A{row_num}:B{row_num})") | ||
| elif value_type == "formula_cross_sheet": | ||
| row_vals.append(f"=Sheet2!A{row_num}") |
There was a problem hiding this comment.
For formula_simple / formula_cross_sheet, the formula string is formatted per cell even though it only varies by row. Precomputing the per-row formula once and reusing it across columns can significantly reduce Python string formatting overhead in large grids (and reduce noise in the measured write time).
| def _section_data_shape(perf: dict[str, Any] | None) -> str: | ||
| """Render the data-shape heatmap section (Sprint 2). | ||
|
|
||
| Two heatmaps (read, write) showing ms-per-100k-cells at the largest tier | ||
| each (library, dtype) cell has data for. Per-column log-scale color so |
There was a problem hiding this comment.
There are existing dashboard rendering tests, but none appear to exercise the new data-shape section (e.g., verifying that id="data-shape" is rendered and the nav link is present when perf results contain data_shape_* entries). Adding a focused unit test with a minimal perf JSON payload would help prevent regressions in the regex parsing, ordering, and conditional nav behavior.
| def perf_shape( | ||
| rows: int = typer.Option( | ||
| 1_000_000, | ||
| "--rows", |
There was a problem hiding this comment.
The new perf-shape command introduces non-trivial parsing/staleness logic (_resolve_shape_features, _shape_fixtures_stale) but there don't appear to be tests covering edge cases like unknown types, boundary --rows values, or 1M-tier regeneration detection. Adding unit tests for these helpers (without running full perf) would improve confidence and prevent accidental CLI behavior changes.
| "--rows", | |
| "--rows", | |
| min=1, |
| The 40 shape feature names follow the pattern | ||
| `data_shape_<dtype>_<tier>_bulk_<read|write>`, e.g. | ||
| `data_shape_formula_cross_sheet_1m_bulk_read`. |
There was a problem hiding this comment.
This sentence says "The 40 shape feature names…", but the matrix produces 2 features per (dtype, tier) pair (bulk_read + bulk_write). That’s 60 features for the default 1k/10k/100k tiers and 80 features when --include-1m is used. Consider removing the hardcoded count or updating it to reflect both cases.
| The 40 shape feature names follow the pattern | |
| `data_shape_<dtype>_<tier>_bulk_<read|write>`, e.g. | |
| `data_shape_formula_cross_sheet_1m_bulk_read`. | |
| Shape feature names follow the pattern | |
| `data_shape_<dtype>_<tier>_bulk_<read|write>`, e.g. | |
| `data_shape_formula_cross_sheet_1m_bulk_read`. This yields 60 features for the | |
| default `1k`/`10k`/`100k` tiers and 80 when `--include-1m` is used. |
| if dtype == "boolean": | ||
| for r in range(rows): | ||
| for c in range(cols): | ||
| ws.write_boolean(r, c, bool((r * cols + c) % 2)) |
There was a problem hiding this comment.
In the data-shape generator, the boolean pattern is offset vs the runner: runner uses start=1 and writes bool(v % 2) (so first cell is True), but the fixture writes bool((r*cols+c) % 2) (first cell False). If the intent is to keep fixtures mirroring _run_workload_write (per docstring), update this branch to use the same 1-based value index as the runner.
| ws.write_boolean(r, c, bool((r * cols + c) % 2)) | |
| v = r * cols + c + 1 | |
| ws.write_boolean(r, c, bool(v % 2)) |
| if dtype == "date": | ||
| date_fmt = wb.add_format({"num_format": "yyyy-mm-dd"}) | ||
| base = date(2020, 1, 1) | ||
| for r in range(rows): | ||
| for c in range(cols): | ||
| v = r * cols + c | ||
| ws.write_datetime(r, c, base + timedelta(days=v), date_fmt) | ||
| return |
There was a problem hiding this comment.
The generated date fixture values are off by one compared to the write workload spec: _data_shape_write_workload sets start=1 and the runner writes date_epoch + timedelta(days=v), but this fixture uses v = r*cols + c (starting at 0). Consider switching to a 1-based index here (and similarly for datetime) so the generator and runner stay consistent as documented.
Summary
What ships
value_typebranches in_run_workload_write(float,date,datetime,boolean,formula_simple,formula_cross_sheet,mixed_realistic).string_short/string_longfold into the existingstringop viastring_length=16/string_length=512.generate_data_shape_scenarios()emits 10 dtypes × 3 tiers (1k / 10k / 100k) by default = 60 manifest rows; the 1M tier is gated behind--include-1m(~5 min generation cost). New--shape-onlyflag skips legacy scenarios for fast iteration.excelbench perf-shapesubcommand with--rows,--types,--regenerate,--memory-mode,--adapter. Inherits Sprint 1's three-mode memory plumbing for free. Auto-regenerates fixtures when the manifest is older than the generator script or the requested 1M tier is missing.Data Shape (dtype × scale)tab — two heatmaps (read, write), rows = library sorted by overall median latency, columns = 10 dtypes, cell = ms-per-100k-cells at the largest tier each (library, dtype) was run at, log-scale color normalized per dtype-column so slow columns don't wash out fast ones.mixed_realisticlives atfixtures/synthetic_calibration/sample_set.md(50-file public xlsx survey, class-weighted means).In Progress(acceptance +Shippedflip happens after merge per file's own protocol).Branching note
Targets
feat/perf-mem-honesty(Sprint 1) rather thanmasterbecause #28 is still open. Sprint 2 explicitly inherits Sprint 1's--memory-modeplumbing — stacking is the only way to test the integration end-to-end. Will retargetmasteronce #28 lands; the diff stays exactly the same.Test plan
uv run ruff check src/ tests/ scripts/— cleanuv run mypy src/excelbench/perf/ src/excelbench/cli.py src/excelbench/results/html_dashboard.py— no issuesuv run pytest tests/— 1140 passed, 32 skipped, 6 xfailed (no regressions)generate_throughput_fixtures.py --shape-onlyproduces 60 fixtures, 1M correctly gatedexcelbench perf-shape --rows 1000 --types int,string_short,formula_simple --adapter wolfxl --iters 1— all 6 features ran cleanly with valid p50 latenciesexcelbench perf-shapeagainst the 5 new dtypes (boolean / date / datetime / formula_cross_sheet / mixed_realistic) with wolfxl + openpyxl — all 20 entries populated; wolfxl 3-6× faster, validating the divergence story_section_data_shapedirect invocation — produces correctly-structured HTML withid="data-shape", log-scale per-column coloring, hover tooltipsOut of scope (per plan)
append_rows,iter_rows_values, font access cost) — Sprint 4mixed_realisticrecalibration against a >500-file corpus — flagged as TODO in DEC-019🤖 Generated with Claude Code