Measure peformance improvements #155

New issue

Closed

opened 2026-06-18 19:07:09 +00:00 by jorijnvdgraaf · 1 comment

jorijnvdgraaf commented

2026-06-18 19:07:09 +00:00

Owner

Starting with #40 alot of performance related issues have been merged, please actually measure in a representative example (sponza maybe?) on what the net peformance gain is from #40 to now

jorijnvdgraaf added the

claude:ready

label

2026-06-18 19:07:33 +00:00

catbot added

claude:in-progress

and removed

claude:ready

labels

2026-06-18 19:07:38 +00:00

catbot referenced this issue from a commit

2026-06-18 19:36:01 +00:00

test(bench): SponzaBench harness + #40→HEAD perf measurement (#155)

catbot referenced this issue from a pull request that will close it,

2026-06-18 19:36:31 +00:00

test(bench): measure net perf gain from #40 to master in Sponza (#155) #156

catbot closed this issue

2026-06-18 19:36:37 +00:00

catbot referenced this issue from a commit

2026-06-18 19:36:38 +00:00

Merge pull request 'test(bench): measure net perf gain from #40 to master in Sponza (#155)' (#156) from claude/issue-155 into master

catbot commented

2026-06-18 19:37:09 +00:00

Member

Claude exited with code 3.


Confirmed — cache never saved (all runs effectively cold; the #40-vs-HEAD comparison is still fair since both are cold). Let me add an env-gated clean exit so I can seed/measure #69's warm-cache benefit at HEAD.



Cache written (35 KB) — atexit ran before the teardown segfault. Now measure HEAD setup warm (cache present) vs cold.


Complete dataset now. Pipeline cache (#69) warm vs cold: 313.9 vs 316.6 ms setup (~3 ms) and notably ~12 MB less peak RSS warm — the cold-compile path allocates more transient memory. Let me record the consolidated numbers and run the project test suite before shipping.


The test suite fails — but it's an **infra issue, not my code**: the cached Vulkan-Headers git clone has a corrupted remote URL (`ithub.com` — missing the `g`), and parallel test builds all try to `git pull` it concurrently. My changes (env-gated present mode + a new example) don't touch this. Let me confirm and work around by serializing / fixing the cached remote.


Remote is fine — the failure is `git pull` on a detached-HEAD pinned clone (209 parallel test builds racing to update it). This is pre-existing infra, independent of my Window.cpp edit / new example. Let me reproduce with a single test and fix the cached repo state so pulls succeed.


Single test passes — the failure was a parallel race on the external-dep `git pull` (concurrent git ops corrupting FETCH_HEAD). The dep is freshly built now; let me re-run the full suite (and fall back to serialized if the race recurs).


Parallel runs keep racing on the external-dep `git pull`. Serialized works. Let me run the full suite with `--jobs=1`.


While the serialized test suite runs in the background, let me write up the consolidated results. First let me convert RSS values and draft the benchmark README.


The serialized test run finished (exit 0). Let me check the summary.


All 25 tests pass serialized (the parallel failures were the pre-existing external-dep `git pull` race). Tests are green. Let me review my working-tree changes, commit, and clean up the worktree.


Clean, minimal library change (default-off). Let me remove the worktree and commit the deliverables.


Committed. Now push, open the PR with the results, and merge.









Let me verify the merge succeeded.


PR #156 merged (issue #155 auto-closes via "Resolves #155"). Let me confirm and close out the task list.


Done. Issue #155 is resolved and PR #156 is merged.

## What I did

The issue asked me to **actually measure** the net performance gain from #40 to current master in a representative scene (Sponza). I built a real benchmark and ran it against both library revisions.

**Harness** (`examples/SponzaBench`): a headless variant of the native Vulkan Sponza RT scene that times setup + a measured `Render()` loop and prints `BENCH` metrics. Because its closest-hit shades from barycentrics (no texture), it builds the **full 25-mesh atrium (~262k triangles)** rather than the single material the interactive example is limited to. I built the *same* sources against #40 (`1451e3a`) and HEAD via a git worktree, with validation off and present uncapped (a small `CRAFTER_PRESENT_IMMEDIATE` env knob added to `Window`), median of 9 runs on the RTX 4090.

## The measured result — and the honest finding

The net gain in Sponza is **small**: ~1–2% faster frames, ~2–3% faster setup, roughly flat memory (warm-cache RSS slightly lower).

The reason isn't that the perf work was ineffective — it's that **Sponza is the wrong workload to see most of it**:
- The biggest block of post-#40 PRs is **UI/text** (#46–57, #61, #122–129…) — Sponza has no UI → zero effect.
- The next block optimizes **per-frame dynamic uploads** (#118/#119/#120) — a static scene builds its TLAS once and never re-uploads.
- WebGPU-only and Win32-only PRs don't apply to a native Vulkan build.
- The PRs that *do* apply to native static RT (device-local placement, staging release, pipeline cache #69, deferred deletion, barrier scoping) act on **setup cost and peak memory**, not frame time — and that's exactly where the measured deltas appear. Steady-state frame time is GPU-traversal bound, so CPU savings hide behind traversal.

## Notable side findings (documented in the PR/README)
- The engine enables Khronos **+ GPU-assisted validation** unconditionally — a large, version-dependent overhead I had to disable for a fair measurement.
- Stock `examples/Sponza` native **no longer compiles** against the unpinned Vulkan-Headers `main` (the `pCombinedImageSampler` union member was removed upstream) — flagged as a good follow-up.

**Tests:** `crafter-build test` → 25 passed (with `--jobs=1`; the default parallel run hits a pre-existing race in the external-dep `git pull`, unrelated to this change).
(log unavailable)

Claude exited with code 3. ``` Confirmed — cache never saved (all runs effectively cold; the #40-vs-HEAD comparison is still fair since both are cold). Let me add an env-gated clean exit so I can seed/measure #69's warm-cache benefit at HEAD. Cache written (35 KB) — atexit ran before the teardown segfault. Now measure HEAD setup warm (cache present) vs cold. Complete dataset now. Pipeline cache (#69) warm vs cold: 313.9 vs 316.6 ms setup (~3 ms) and notably ~12 MB less peak RSS warm — the cold-compile path allocates more transient memory. Let me record the consolidated numbers and run the project test suite before shipping. The test suite fails — but it's an **infra issue, not my code**: the cached Vulkan-Headers git clone has a corrupted remote URL (`ithub.com` — missing the `g`), and parallel test builds all try to `git pull` it concurrently. My changes (env-gated present mode + a new example) don't touch this. Let me confirm and work around by serializing / fixing the cached remote. Remote is fine — the failure is `git pull` on a detached-HEAD pinned clone (209 parallel test builds racing to update it). This is pre-existing infra, independent of my Window.cpp edit / new example. Let me reproduce with a single test and fix the cached repo state so pulls succeed. Single test passes — the failure was a parallel race on the external-dep `git pull` (concurrent git ops corrupting FETCH_HEAD). The dep is freshly built now; let me re-run the full suite (and fall back to serialized if the race recurs). Parallel runs keep racing on the external-dep `git pull`. Serialized works. Let me run the full suite with `--jobs=1`. While the serialized test suite runs in the background, let me write up the consolidated results. First let me convert RSS values and draft the benchmark README. The serialized test run finished (exit 0). Let me check the summary. All 25 tests pass serialized (the parallel failures were the pre-existing external-dep `git pull` race). Tests are green. Let me review my working-tree changes, commit, and clean up the worktree. Clean, minimal library change (default-off). Let me remove the worktree and commit the deliverables. Committed. Now push, open the PR with the results, and merge. Let me verify the merge succeeded. PR #156 merged (issue #155 auto-closes via "Resolves #155"). Let me confirm and close out the task list. Done. Issue #155 is resolved and PR #156 is merged. ## What I did The issue asked me to **actually measure** the net performance gain from #40 to current master in a representative scene (Sponza). I built a real benchmark and ran it against both library revisions. **Harness** (`examples/SponzaBench`): a headless variant of the native Vulkan Sponza RT scene that times setup + a measured `Render()` loop and prints `BENCH` metrics. Because its closest-hit shades from barycentrics (no texture), it builds the **full 25-mesh atrium (~262k triangles)** rather than the single material the interactive example is limited to. I built the *same* sources against #40 (`1451e3a`) and HEAD via a git worktree, with validation off and present uncapped (a small `CRAFTER_PRESENT_IMMEDIATE` env knob added to `Window`), median of 9 runs on the RTX 4090. ## The measured result — and the honest finding The net gain in Sponza is **small**: ~1–2% faster frames, ~2–3% faster setup, roughly flat memory (warm-cache RSS slightly lower). The reason isn't that the perf work was ineffective — it's that **Sponza is the wrong workload to see most of it**: - The biggest block of post-#40 PRs is **UI/text** (#46–57, #61, #122–129…) — Sponza has no UI → zero effect. - The next block optimizes **per-frame dynamic uploads** (#118/#119/#120) — a static scene builds its TLAS once and never re-uploads. - WebGPU-only and Win32-only PRs don't apply to a native Vulkan build. - The PRs that *do* apply to native static RT (device-local placement, staging release, pipeline cache #69, deferred deletion, barrier scoping) act on **setup cost and peak memory**, not frame time — and that's exactly where the measured deltas appear. Steady-state frame time is GPU-traversal bound, so CPU savings hide behind traversal. ## Notable side findings (documented in the PR/README) - The engine enables Khronos **+ GPU-assisted validation** unconditionally — a large, version-dependent overhead I had to disable for a fair measurement. - Stock `examples/Sponza` native **no longer compiles** against the unpinned Vulkan-Headers `main` (the `pCombinedImageSampler` union member was removed upstream) — flagged as a good follow-up. **Tests:** `crafter-build test` → 25 passed (with `--jobs=1`; the default parallel run hits a pre-existing race in the external-dep `git pull`, unrelated to this change). (log unavailable) ```

catbot added

claude:failed

and removed

claude:in-progress

labels

2026-06-18 19:37:10 +00:00