Repository navigation
docs(bench): plot the results, and fold the tables under them - #451
Conversation
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (12)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
38c9c55 to
f319cd1
Compare
|
Rebased onto main, which moved a long way under this branch — and that turned up two things worth flagging separately from the charts.
The figures had been drawn from the wrong file. They came from a Also: the reproduce block still named Rebased past the merged #438, so this targets |
Merging this PR will improve performance by 12.53%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_emit[transport-s-lpspec-lp] |
24.6 MB | 21.6 MB | +13.71% |
| ⚡ | test_emit[profiled-s-lpspec-lp] |
34.3 MB | 30.8 MB | +11.37% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing docs/benchmark-charts (35aefe2) with main (c14b180)
Footnotes
-
10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
@codspeedbot explain why this is faster |
Short answer: it isn't faster, and it isn't this PR. The single reported change is a peak-heap measurement (memory mode, not time), and the code it measures is byte-identical between The measured tree did not change
The allocation profile says "same program, different machine"
The same number of allocations, each slightly smaller. A real optimisation normally removes allocations; a buffer-sizing change removes bytes while the call count stays put. That is what machine-dependent sizing looks like — and the runner CPU did change under this comparison ( The rest of the matrix moved both waysIn the same pair of runs:
Verdict: unattributable to the diff; do not claim it in the changelog. It should revert to ~6.9 MB the next time this branch is measured on a 7763-class runner. One thing worth fixing, if these numbers are to be compared over time
No changes committed here — there was nothing to fix in the branch. Summary
Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot. |
The benchmarks page was 110 table rows in twelve tables, all weighted the
same, with no shape and no headline — and the one chart page we had was a
separate HTML file linked from a mid-paragraph, absent from the nav. So
the numbers were published and the *finding* was not.
Four figures now lead the Results section, written by `bench.plot` from
the same jsonl the tables come from:
wall time to a loaded solver, faceted by sink, log-log
peak the same builds' resident memory, its own chart, never a
second axis on the first
cases all six models as small multiples
phases where the time goes at `l` — the chart the tables cannot be
read for at all, since they publish one total per row
`bench/svg.py` draws them: no plotting library, because these pages are
read on GitHub as often as on the site and a chart has to be a committed
file rather than something a renderer produces at view time. Two files
per figure, light and dark, because mkdocs-material's toggle stamps the
host page and an `<img>`-referenced SVG cannot see it; the `#only-light`
/ `#only-dark` suffixes choose, and GitHub takes the first of the pair.
The palette is validated rather than chosen — slots 1 and 2 of a
reference categorical palette, run through the six checks against both
surfaces (lightness band, chroma floor, CVD separation, normal-vision
floor, contrast). Every series is direct-labelled as well as coloured, so
identity survives a reader who sees neither hue.
The twelve tables stay, one `<details>` per case, and their headings
became bold text: a heading inside a fold still lands in the table of
contents, which is twelve rail entries for the appendix.
`docs/bench.svg` deleted — drawn for #19, hardcoded light-mode colours,
referenced from nowhere since. `tests/test_docs_site.py` now checks both
directions, so the next one fails instead of rotting.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things, both about reading rather than drawing. **The charts stopped shrinking below the tablet breakpoint.** They are `<img>`-referenced SVG, so they scaled with the column like any image — right on a wide screen and wrong on a narrow one, where a uniform downscale takes 11px axis labels to about 5px and leaves a chart whose axes cannot be read. Now the figure scrolls sideways instead and the chart keeps its size. Done in CSS, selected on the `src` path, so the markdown stays exactly what GitHub renders — which is the whole reason these are files rather than inline SVG. **The interactive page is in the nav.** It has always had the cursor the static figures cannot: a pointer readout over the same run, ratio bands across all six models, and its own light/dark. It was reachable only through one link in the middle of a paragraph. Hover was never missing software — it was a missing nav entry. The Results section says so once, under the figures, rather than four times. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three things, and the first is why the other two matter. **`bench.plot` could not run.** #448 moved the readers to pytest-benchmark's `.json` while `bench/results/` still holds the `.jsonl` the previous runner wrote, so refreshing the chart page or the figures died on a missing file. `results.records` now takes either — the old format is one record per line and already in the shape both readers want — and the paths resolve to whichever is committed. **The figures were drawn from the wrong measurements.** They came from a `latest.jsonl` at lpspec 0.0.0a27 that still carried `duckdb` and `duckdb@1GB` arms — a file from before that engine was removed. Regenerated against what the repo ships today, lpspec 0.0.1a55, so the figures, the tables and the chart page all read the same run. The chart page's own numbers were already in step and did not move. **The reproduce block named a module that no longer exists.** `bench.run` went with #448; the commands are the pytest ones now, which is what bench/README.md has said since. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A full run at f319cd1: six models, four rungs, both arms, and for the first time all three sinks — 192 measurements, 35 minutes, 192 passed. **Under pytest-benchmem 0.5**, whose isolated pass no longer warms up, so a `peak` here is the cold first call in a fresh process. That is the thing this page describes and the thing a caller building one model pays; a 0.4 peak is a different quantity wearing the same name, so the floor moved to `>=0.5` rather than letting the two mix. The release's headline fix — Linux `ru_maxrss` reading the parent's high-water mark — does not touch these numbers; macOS was never affected. **Peak reproduced; wall did not.** Against the previous run this one agrees on every peak cell to within 0.02x — the same 1.46x, 1.10x, 0.33x — and disagrees on wall by up to 40% relative. Peak is a property of what the code allocates; wall on a shared laptop is a property of what else was running. The page now says so, and says to read the wall column for its sign and order of magnitude rather than its second digit. **Gurobi is the slowest destination for both arms**, and the one where the lanes converge: gurobipy's per-row and per-column ingestion (#434) dominates the build in front of it. The fourth figure had to change with the harness. #448 made `test_emit` one number for build-and-hand-over and left the only other measurement a warm in-process rebuild, so there is no honest way to stack a phase bar any more; it is now the `l` rung through every sink, which is the question the third sink makes worth asking. `bench/conftest.py` tracks gurobipy and scipy from here on — a ratio through a solver has to say which solver. This file predates that by minutes, so the two versions are named in the page instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A second full run — nine rounds per measurement instead of five, on a machine at load 1.5 rather than 4.5, clean tree at f10c3ee — and a cell-by-cell comparison against the first. peak median difference 0.7% worst cell 7.7% wall median difference 5.3% worst cell 65% So the two columns are not the same kind of number. Peak is a property of what the code allocates and it reproduces: 1.46x is 1.46x both times. Wall on a shared laptop is partly a property of what else was running — `dispatch` through `lp` came out 0.46x and then 0.75x. The page now publishes that comparison, says to read a wall cell for its sign and order of magnitude rather than its second digit, and makes only the claims that survived both runs. Which changes one of them: through `gurobi` we lead on all six models, because gurobipy's ingestion is a constant added to both arms and what is left showing is the build in front of it. `profiled` is still the loss through `highs` and `lp`. This run needed no prose to patch its provenance: clean tree, and gurobipy and scipy recorded in the file by the tracking added last commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
82caa01 to
35aefe2
Compare
|
Re-ran the ladder a second time to check reproducibility — nine rounds per measurement instead of five, on a machine at load 1.5 rather than 4.5, clean tree. Compared cell by cell against the first run:
Peak reproduces (1.46x is 1.46x both times); wall does not, per cell — Rebased onto #455, which reached the pytest-benchmem 0.5 conclusion independently and added the instrument to the fingerprint — the right call, and stronger than the note I had written. |
The benchmarks page was 110 table rows in twelve tables, all weighted the
same, with no headline and no shape — and the one chart page we had
(
benchmarks-scaling.html) was a separate file, absent from the nav, linkedfrom the middle of a paragraph. The numbers were published; the finding was
not.
Judge it here: the Read the Docs preview build on this PR renders the real
page in both schemes — that is the surface to look at, not the diff.
Four figures, from the same jsonl as the tables
l— the chart the tables cannot be read for, since they publish one total per rowphasesis the one I would keep if only one survived: our claim lives inbuild, and after #438emitis at parity. The tables mix those together intoa single ratio.
How they are drawn
bench/svg.py, no plotting library. These pages are read on GitHub as often ason the site, so a chart has to be a committed file rather than something a
renderer produces at view time — and a dependency that draws axes is a poor
trade for a docs build that installs in seconds.
Two files per figure. mkdocs-material's palette toggle stamps the host
page, which an
<img>-referenced SVG cannot see, so the page carries both andMaterial's
#only-light/#only-darksuffixes choose. GitHub takes the firstof the pair. Dark is drawn first because dark is the site's default scheme.
The palette is validated, not chosen. Slots 1 and 2 of a reference
categorical palette, run through six checks against both surfaces — lightness
band, chroma floor, CVD separation (worst adjacent ΔE 24.7 light / 26.8 dark),
normal-vision floor, contrast. Every series is direct-labelled as well as
coloured, so identity never rests on hue alone.
What else changed
<details>per case.<details>rather thanmkdocs'
???because it folds on GitHub too.the table of contents, and twelve rail entries for the appendix was most of
what made the page feel like a wall.
docs/bench.svgdeleted — drawn for Backend router, vtype, shift(), and piecewise: blocks #19 in the duckdb era, hardcodedlight-mode colours, referenced from nowhere since.
tests/test_docs_site.pychecks figures in both directions: everyreference resolves, every file is referenced, every dark has a light twin. The
orphan above is exactly what the second half prevents.
Notes
Stacked on #438 (it touches
bench/report.pytoo) — GitHub will retarget thisto
mainwhen that merges.The figures plot the existing committed results, which carry
lpandhighs. The gurobi facet appears on your next ladder run;bench.plotdrops asink that has no measurements rather than drawing an empty panel, since a facet
with no line reads as a measurement of zero.
🤖 Generated with Claude Code