Fix CI: bounded hexbin payloads + pyplot per-build overhead - #46
Merged
Merged
Conversation
Two regressions from #37 kept CI red on main and every open PR: Hexbin (benchmarks/test_codspeed_kernels.py::test_first_payload_hexbin_core_2d): _emit_hexbin expanded every cell into six fan triangles across seven f32 columns (168 B/cell — 3.17 MB at gridsize=128, 2x the raw input; #45 had loosened the benchmark bound to accept it). Hexagons all share one geometry, so ship centers plus one color value per cell (12 B/cell, 227 KB) and expand locally in each renderer: instanced triangles in the WebGL client (_buildHexbinMark), one polygon per cell in SVG, vectorized triangles in the raster exporter. Headless-Chromium readback is pixel-identical to the old wire shape. The benchmark bound is restored to grid-bounded centers. Perf guardrail (tests/pyplot/test_perf_guardrail.py, 10k case): pyplot's fixed per-build cost had grown to ~2x the declarative baseline locally. Removed per-build revalidation and O(n) temporaries: - styles.compile_axis_style / compile_mark_style and _validate.style_mapping now memoize per exact input dict — the same few rc-derived chrome dicts were re-parsed through the native CSS grammar on every build. - The rc chrome snapshot (_load_rc_chrome) is cached per (RcParams.version, dpi); RcParams now versions its mutations. - plot() gap/path detection drops an astype copy + diff + compare for the native is_sorted kernel and skips the isfinite mask entirely for clean data via a sum probe (non-finite anywhere poisons the sum). Local ratio at 10k: 2.03x -> ~1.6x; 100k: 1.26x. Full suite, ruff, ty, render smoke pass; hexbin WebGL/SVG/PNG verified.
Merging this PR will improve performance by 70.32%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_first_payload_hexbin_core_2d |
34.2 ms | 20.1 ms | +70.32% |
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 fix/ci-hexbin-payload-and-pyplot-overhead (6f57cb2) with main (c36e908)
Footnotes
-
1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩
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.
Fixes the two tests that have kept CI red on main since #37 (and made every open PR — e.g. #42 — fail checks it didn't cause).
Hexbin payload (CodSpeed /
test_first_payload_hexbin_core_2d)_emit_hexbinexpanded every cell into six fan triangles across seven f32 columns — 168 B/cell, 3.17 MB at gridsize=128 vs the 1.6 MB raw input (#45 had loosened the benchmark bound to accept this). Every hexagon shares the same geometry, so the payload now ships centers + one color value per cell (12 B/cell, 227 KB, 14× smaller) withhex_dx/hex_dyin the trace style, and each renderer expands locally:_buildHexbinMarkexpands to instanced triangles in the centers' encoded space (deltas scale bymeta.scale), reuses the mesh draw path unchanged.<polygon>hexagon per cell (18,890 elements instead of 113k triangles).cmd.trianglesop.Verified pixel-identical WebGL readback (headless Chromium, same lit-pixel hash old vs new), plus SVG/PNG render checks. The benchmark bound is restored to the grid-bounded centers contract and re-asserts
payload < raw input.Pyplot perf guardrail (
test_pyplot_build_tracks_declarative[10000-60-1.6])The shim's fixed per-build cost had drifted to ~2× the declarative baseline (locally measured). The failures were exactly the "per-build revalidation / O(n) copy" class the guardrail exists to catch:
styles.compile_axis_style/compile_mark_style/_validate.style_mappingnow memoize per exact input dict — the same handful of rc-derived chrome dicts were re-run through the native CSS grammar on every chart build._load_rc_chrome) is cached per(RcParams.version, dpi);RcParamsnow versions all mutations. Per-axes mutable members (_theme_tokens,_hidden_spines) are copied from the snapshot.plot()gap/path detection: replaced anastypecopy +np.diff+ compare with the nativeis_sortedkernel, and clean (all-finite) data skips theisfinitemask via a sum probe — a non-finite value anywhere poisons the sum, so the exact scan only runs when there might be gaps. Gap/parametric routing semantics verified unchanged (NaN, inf, unsorted-x, sorted-clean).Local best-of-N ratios at 10k: 2.03× → ~1.6×; at 100k: 1.26× (ceiling 1.5). Guardrail passes repeatedly locally.
Validation
uv run pytest: 1770 passed, 1 skippedcargo test,ruff check,ruff format --check,ty check: cleanscripts/render_smoke_nonumpy.py: OK🤖 Generated with Claude Code