test(eng2): expand renderer bench suite - #816
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
aram-devdocs
marked this pull request as ready for review
July 7, 2026 23:51
Contributor
|
Claude finished @aram-devdocs's task in 5m 50s —— View job Review:
|
| # | File:Line | Issue |
|---|---|---|
| 1 | goud_engine/tests/renderer3d_frame_counts.rs:76-111 vs goud_engine/tests/spec/eng2_p0_03_bench_suite_expansion.rs:54-78 |
The cull-scaling and plane/cube draw-count invariants (total/visible/culled/draw_calls) are asserted twice, in near-identical tests in both files, just with different n values (10k/1k vs 10k/512). Not wrong, just redundant coverage — could consolidate into one location (spec test can be the sole source of truth per testing-v2.md, or the frame-counts file can own it and the spec test can just check wiring). Not asking for a change, flagging for awareness. |
| 2 | .github/workflows/*.yml |
scripts/bench-gate.py (and the renderer3d_frame_benchmarks/engine_tick_benchmarks targets this PR wires new groups into) is not invoked from any CI workflow — confirmed via grep across all .github/workflows/*.yml. benchmarks.yml only runs ecs_benchmarks/asset_benchmarks/sprite_batch_benchmarks and never calls bench-gate.py. This predates this PR and isn't its responsibility to fix, but perf-work.md names this script as the CI-runnable gate mechanism — worth a follow-up issue so the gate is actually enforced automatically rather than manual-only. |
| 3 | deny.toml / Cargo.lock |
The security-refresh commit (crossbeam-epoch 0.9.18→0.9.20, memmap2 0.9.10→0.9.11, dropping the RUSTSEC-2026-0186/-0189 ignore entries) is bundled into a PR titled/scoped as bench-suite test infra. It's small and verified consistent (memmap2 bump backs the 0186 removal; rmcp was already 1.5.0 on main, so the 0189 removal is stale-entry cleanup), so not blocking, but it's scope outside the stated PR description. |
Verification performed
- Read all 11 changed files in full (
scene3d.rs, both bench files, both test files,bench-gate.py,Cargo.toml,deny.toml, baseline JSON). - Confirmed
cull_scaling_scene/dynamic_primitive_sceneobject-id and position math is internally consistent (ids1..=nsequential fromcreate_primitive, visible/hidden positions non-overlapping). - Confirmed the new
[[bench]] renderer3d_real_wgpu_shadow_benchmarkstarget is correctly separated from the default CPU-safe Criterion suite and gated behindGOUD_BENCH_REAL_WGPU_SHADOW=1, keeping raw wgpu calls opt-in and CI-safe perrender-v2.md/perf-work.md. - Confirmed the ratio-normalized
bench-gate.pydesign (comparing each bench againstengine_tick/tick_10kon the same run) means the full baseline recapture incriterion_baseline.json(which shifted absolutemean_nsfor pre-existing groups) is not a red flag — ratios, not raw times, are what's gated. - Confirmed
is_multiple_of(used in the newlabel()helper) has repo precedent (engine_tick_benchmarks.rs:81,renderer3d/core_model_animation/mod.rs:237), so it's not a toolchain risk. - Confirmed the new spec test file name/location (
goud_engine/tests/spec/eng2_p0_03_bench_suite_expansion.rs) matches the tracked issueENG2-P0-03indocs/src/runbook/phase-specs.md:78andphases/phase-0.md:19, and follows theeng2_p<N>_<nn>_<slug>.rsconvention fromtesting-v2.md. cargo fmt --all -- --check— clean.- Could not verify:
cargo build/cargo test/cargo bench/cargo clippyin this sandbox — the defaultdesktop-nativefeature set pulls inwinit/gilrs, which need the systemlibudevdev package; it isn't installed here andsudo apt-get installis blocked by this environment's sandbox policy. Reviewed by careful reading instead of full compilation; the PR body's reported local runs (cargo check, spec tests,renderer3d_frame_counts, both new Criterion benches,bench-gate.py --quick) look consistent with the code as written.
Positive callouts
- Real-GPU shadow bench is cleanly isolated in its own file/target with an explicit opt-in env var, keeping the default suite headless/CI-safe — good adherence to
render-v2.md's backend-isolation rule. - New scenes (
cull_scaling_scene,dynamic_primitive_scene) are deterministic and backed by dedicated exact-count assertions inrenderer3d_frame_counts.rs, matchingtesting-v2.md's "assert exact draw/culled/visible counts" requirement. - No FFI, unsafe, or SDK surface touched; file sizes are well under the 500-line cap (largest changed file ~410 lines).
bench-gate.py's ratio-vs-reference design is a solid piece of engineering — it insulates the gate from machine-to-machine noise, which is exactly why the wholesale baseline recapture here isn't a problem.
Owner
Author
|
Claude warning triage:
No code changes are needed for these warnings. |
aram-devdocs
force-pushed
the
codex/issue-743-bench-suite-expansion
branch
from
July 8, 2026 20:11
5d6297e to
462e2ed
Compare
aram-devdocs
enabled auto-merge (squash)
July 8, 2026 20:43
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.
Overview
Type: test
Summary:
Adds the ENG2-P0-03 renderer benchmark coverage for frustum-cull scaling, legacy primitive draw-call cost, and an opt-in real-wgpu shadow benchmark. Also records the expanded Criterion baseline and wires the new CPU-safe groups into
scripts/bench-gate.py.Related Issues: Fixes #743
Changes Made
Engine Core (
goud_engine/src/)No engine runtime changes.
FFI Layer (
goud_engine/src/ffi/)No changes.
C# SDK (
sdks/csharp/)No changes.
Python SDK (
sdks/python/)No changes.
TypeScript SDK (
sdks/typescript/)No changes.
Codegen Pipeline (
codegen/)No changes.
Proc Macros (
goud_engine_macros/)#[goud_api]attribute changesNo changes.
Tools (
tools/)No
tools/changes.scripts/bench-gate.pynow trackscull_scalingandprimitive_draw_callsby default.WASM (
goud_engine/src/wasm/)No changes.
Examples (
examples/)No changes.
Documentation
No docs changes; benchmark usage is documented in the new bench file comments.
Architectural Compliance
./codegen.shranlint-layerscleanly.Testing
cargo testpassescargo clippy -- -D warningsis cleancargo fmt --all -- --checkpassespython3 sdks/python/test_bindings.py) — if SDK changeddotnet test sdks/csharp.tests/) — if SDK changedcd sdks/typescript && npm test) — if TS SDK changed./codegen.sh)Additional local checks:
cargo checkpassed.cargo test --test spec -- --nocapturepassed.cargo test --test renderer3d_frame_counts -- --nocapturepassed.cargo bench --bench engine_tick_benchmarks -- --sample-size 10 --warm-up-time 1 --measurement-time 1passed.cargo bench --bench renderer3d_frame_benchmarks -- --sample-size 10 --warm-up-time 1 --measurement-time 1passed.GOUD_BENCH_REAL_WGPU_SHADOW=1 cargo bench --bench renderer3d_real_wgpu_shadow_benchmarks -- --sample-size 10 --warm-up-time 1 --measurement-time 1passed locally on macOS.python3 scripts/bench-gate.py --quickpassed with 16 tracked benches.Known baseline failure:
cargo teststill fails atgoud_engine/tests/native_main_thread.rs:109(assertion failed: game.end_render()). This was reproduced on currentmainbefore this branch, so it is not introduced here.Code Quality
todo!()orunimplemented!()in production code.#[allow(unused)]in production code.Result; benchmark panics are limited to explicit bench setup failures.Documentation
AGENTS.mdfiles (if architecture changed): N/A.README.md(if user-facing behavior changed): N/A.Breaking Changes
None.
Version Bump
Bump type: none
Justification: Internal benchmark/test infrastructure only.
Security
unsafeblocks — or each one has a// SAFETY:comment and is necessary.cargo deny check)Performance
Recorded expanded CPU-safe baseline in
goud_engine/benches/baselines/criterion_baseline.jsonwith 16 tracked benches.Selected local Criterion captures:
cull_scaling/visible_5k_total_10k:[348.85 us 349.32 us 349.73 us]cull_scaling/visible_5k_total_30k:[477.06 us 482.61 us 488.59 us]cull_scaling/visible_5k_total_100k:[882.73 us 894.86 us 926.77 us]primitive_draw_calls/plane_10k:[623.77 us 624.90 us 626.37 us]primitive_draw_calls/cube_10k:[620.42 us 621.34 us 623.53 us]shadow_record_real_gpu/casters_1400:[18.852 ms 19.132 ms 19.586 ms]shadow_record_real_gpu/casters_5k:[71.606 ms 75.424 ms 79.437 ms]Deployment
N/A for this internal benchmark/test change.
Reviewer Notes
The real-wgpu shadow bench is deliberately separate and opt-in through
GOUD_BENCH_REAL_WGPU_SHADOW=1. It creates a small native window because hidden or occluded wgpu surfaces do not produce presentable frames; the default Criterion suite remains NullBackend-only and CI-safe.