Measure the library: perf suites, a recorded baseline, and targets
tests/perf.c and tests/perf_render.c drive the hot paths hard enough to time them -- pool acquire and release at both ends of the scan, the registry, the state-to-sprite lookup, the per-actor update and render, the physics sweep, an all-pairs collision sweep, the JSON accessors, path resolution, the drawing primitives, text, asset loading, a screenful of tiles, and a whole frame through akgl_game_update. They are registered like any other suite but carry the `perf` label, so `ctest -L perf` runs only them and `-LE perf` leaves them out. Every measurement is held to a budget at roughly ten times the recorded baseline, enforced only in an optimized build at full scale. Three things had to be got right before the numbers meant anything, and each was wrong first: - No error checking inside the clock. PASS and CATCH call akerr_valid_error_address, which walks AKERR_ARRAY_ERROR -- more work than several of the calls being measured. - Flush the renderer before stopping it. SDL batches, so the first version measured queueing, reported a tilemap frame 250 times faster than it is, and paid the real cost at teardown inside SDL_DestroyTexture. - A drawing benchmark needs a raw-SDL control doing the same pixel work with the same access pattern. With one, akgl_tilemap_draw turns out to cost 0.2% of the frame it appeared to own: 16.26 ms against a control's 16.23 ms for the same 1200 blits. The rasterizer is the frame. PERFORMANCE.md records the baseline, the frame budget it adds up to, and what the numbers say -- including that the string pool's acquire is 64x slower full than empty because it is a megabyte of PATH_MAX buffers, that a handled missing-sprite condition costs nine times the update it replaces, that text rasterizes and throws away a texture on every call, and that 28 MB of the library's BSS is one akgl_Tilemap. TODO.md gains a Performance section: five defects the stress tests found that the unit suites do not reach (a tilemap load leaking five pooled strings, two JSON accessors that turn pool exhaustion into a segfault rather than AKGL_ERR_HEAP, akgl_game_update crashing without akgl_game_init, and its actor update sweep running once per tilemap layer), and eighteen targets with today's number and whether it is met. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
183
TODO.md
183
TODO.md
@@ -918,6 +918,189 @@ without coming here first. Ordered by blast radius.
|
||||
the case written and deliberately not asserted; it would become a `TEST_EXPECT_OK`. Until
|
||||
then the header carries the wart as a `@note` pointing here.
|
||||
|
||||
## Performance
|
||||
|
||||
The first measured baseline is in `PERFORMANCE.md`, produced by `tests/perf.c`
|
||||
and `tests/perf_render.c` (`ctest --test-dir build -L perf`). Both suites hold
|
||||
every measurement to a budget set at roughly ten times the recorded baseline, so
|
||||
an algorithmic regression fails the suite rather than being discovered by a
|
||||
player. Read `PERFORMANCE.md` before arguing with anything below — every claim
|
||||
here has a number behind it, and several of the things I expected to be slow are
|
||||
not.
|
||||
|
||||
### Defects the perf suites found
|
||||
|
||||
Ordered by blast radius. Numbering continues the **Defects** list above.
|
||||
|
||||
28. **`akgl_path_relative` leaked an error context on every root-fallback
|
||||
resolution.** `src/util.c:118` took its `ENOENT` branch by `return`ing from
|
||||
inside the `HANDLE` block, which skips the `RELEASE_ERROR` that `FINISH`
|
||||
ends with. One entry of `AKERR_ARRAY_ERROR` was lost per call, and the 129th
|
||||
call hit "Unable to pull an error context from the array!" and **exited the
|
||||
process**. Every tilemap load resolves several paths this way, so a game
|
||||
that loaded fifty levels died in the loader.
|
||||
|
||||
**Fixed.** The branch now records a flag and calls
|
||||
`akgl_path_relative_root` after `FINISH`. `tests/util.c` carries
|
||||
`test_akgl_path_relative_releases_contexts`, which resolves
|
||||
`AKERR_MAX_ARRAY_ERROR * 2` paths through that branch and asserts the pool
|
||||
is where it started; against the old code that test does not fail, it
|
||||
terminates the suite.
|
||||
|
||||
This is the *only* `return` from inside a `HANDLE` block in `src/` — the
|
||||
other candidates use `SUCCEED_RETURN`, which releases correctly. Worth a
|
||||
grep before anyone writes a new one, and worth a line in AGENTS.md's
|
||||
error-handling protocol, which warns about `*_RETURN` inside `ATTEMPT` but
|
||||
not about returning out of `HANDLE`.
|
||||
|
||||
29. **`akgl_tilemap_load` leaks five pooled strings per load.** Measured by
|
||||
counting non-zero `HEAP_STRING` refcounts across load/release cycles: five
|
||||
per cycle, exactly, and `akgl_tilemap_release` gives none of them back. The
|
||||
52nd map load in a process finds the pool empty.
|
||||
|
||||
Blast radius: every level transition. A game with fifty levels, or one that
|
||||
reloads a level on death, hits it. The fix is a sweep of the loader's
|
||||
`CLEANUP` blocks — `akgl_tilemap_load`, `akgl_tilemap_load_layers`,
|
||||
`akgl_tilemap_load_layer_tile`, `akgl_tilemap_load_layer_objects`, and
|
||||
`akgl_tilemap_load_tilesets_each` all claim scratch strings — and it wants a
|
||||
test that asserts the pool is unchanged across a load/release cycle. That is
|
||||
its own commit, not a footnote to a benchmark.
|
||||
|
||||
Until it is fixed, the tilemap-load benchmark in `tests/perf_render.c`
|
||||
reclaims the pool by hand between iterations, and says so.
|
||||
|
||||
30. **Two JSON accessors turn string-pool exhaustion into a segfault.**
|
||||
`akgl_get_json_string_value` ends its `ATTEMPT` with
|
||||
`FINISH(errctx, false)` at `src/json_helpers.c:89`, so a failed
|
||||
`akgl_heap_next_string` is swallowed rather than passed up, and
|
||||
`src/json_helpers.c:91` then `strncpy`s `AKGL_MAX_STRING_LENGTH` bytes
|
||||
through the pointer it never set. `akgl_get_json_array_index_string` has the
|
||||
same pair at `src/json_helpers.c:143` and `src/json_helpers.c:145`. Item 29
|
||||
is what makes this reachable, and this is what item 29 looks like from the
|
||||
outside: not `AKGL_ERR_HEAP`, a crash inside `strncpy` with a NULL
|
||||
destination.
|
||||
|
||||
Fix: `FINISH(errctx, true)` in both. One word each, but it changes what
|
||||
callers see on a path they currently cannot survive, so it wants a test that
|
||||
exhausts the pool deliberately and asserts `AKGL_ERR_HEAP` comes back out of
|
||||
both functions.
|
||||
|
||||
31. **`akgl_game_update` segfaults if `akgl_game_init` did not run.**
|
||||
`src/game.c:182` calls `game.lowfpsfunc()` through the pointer with no NULL
|
||||
check, on every frame where `game.fps` is under 30 — which includes the
|
||||
first frame, before there is a frame rate to compare against. `akgl_game_init`
|
||||
installs the default; nothing else does.
|
||||
|
||||
That matters because `include/akgl/renderer.h` documents the other path on
|
||||
purpose: a host that owns its own window and calls `akgl_render_bind2d`
|
||||
instead of `akgl_render_init2d`. An embedder following that documentation
|
||||
and then calling `akgl_game_update` crashes on frame one. `akbasic` is
|
||||
exactly that embedder.
|
||||
|
||||
Fix: guard the call, or have `akgl_game_updateFPS` install the default when
|
||||
it finds a NULL. `tests/perf_render.c` installs it by hand as a workaround
|
||||
and points here.
|
||||
|
||||
32. **`akgl_game_update` runs the actor update sweep once per tilemap layer.**
|
||||
`src/game.c:617` loops `i` over `AKGL_TILEMAP_MAX_LAYERS` and the actor
|
||||
sweep nested inside it never compares `actor->layer` to `i`, so every live
|
||||
actor's `updatefunc` runs **16 times per frame**. Measured: 68.4 ns per
|
||||
update, 64 actors, so 70 µs of work to do 4.4 µs of work.
|
||||
|
||||
It hides behind the rasterizer today (a 640x480 software frame is 16 ms) and
|
||||
it will not hide behind a GPU backend, where a frame is nearer 2 ms and this
|
||||
is 3.5% of it. Fix: either filter by layer like `akgl_render_2d_draw_world`
|
||||
does, or hoist the update sweep out of the layer loop entirely — updating an
|
||||
actor is not a per-layer operation. The second is almost certainly right,
|
||||
and it is the one that needs a test asserting `updatefunc` runs exactly once
|
||||
per actor per `akgl_game_update`.
|
||||
|
||||
33. **`akgl_heap_release_character` leaks its `state_sprites` property set.**
|
||||
Already documented in `heap.h` and in **Carried over** item 1; the perf
|
||||
suite makes it measurable rather than theoretical. A benchmark that loads
|
||||
the fixture character 10,000 times leaks 10,000 SDL property sets and 20,000
|
||||
sprite references. Cross-referenced here only because the character-load
|
||||
benchmark had to be written around it.
|
||||
|
||||
### Targets
|
||||
|
||||
What a library like this *should* hit. These are not predictions of what the
|
||||
current code does — several are missed today, and each says which. The frame
|
||||
budget throughout is 16.67 ms (60 fps); where a target is stated per-operation
|
||||
it is because that is the number that survives a change of renderer.
|
||||
|
||||
The one that governs the rest: **libakgl's own bookkeeping should never be the
|
||||
reason a frame is late.** Everything the library decides — pool scans, registry
|
||||
lookups, state-to-sprite mapping, physics, visibility, error contexts — should
|
||||
fit in 5% of a frame, leaving 95% for the pixels and the game's own logic. At
|
||||
64 actors and a screenful of tiles that is a ceiling of about 800 µs.
|
||||
|
||||
| # | Target | Today | Verdict |
|
||||
|---|---|---|---|
|
||||
| 1 | Library bookkeeping under 5% of a 60 fps frame at 64 actors + 1200 tiles | ~0.3% (excluding pixel work) | **met** |
|
||||
| 2 | One actor's logic update under 200 ns | 68.4 ns | **met** |
|
||||
| 3 | One actor's render bookkeeping, excluding the blit, under 500 ns | ~250 ns, by subtraction rather than direct measurement | **met, weakly measured** |
|
||||
| 4 | Physics sweep under 25 ns per *live* actor, and proportional to live actors rather than pool size | 19 ns per live actor, but 63.9 ns for an *empty* pool | **partly** |
|
||||
| 5 | Tilemap draw bookkeeping under 100 ns per tile, excluding the blit | ~25 ns | **met** |
|
||||
| 6 | Pool acquire under 100 ns regardless of how full the pool is | 3.9 ns empty, 250.9 ns on the last free string slot | **missed** |
|
||||
| 7 | Pool release proportional to the bytes actually used, not to the slot's capacity | 47.2 ns, a fixed 4 KiB wipe | **missed** |
|
||||
| 8 | Re-drawing an unchanged line of text under 1 µs | 12.6 µs, every frame, no cache | **missed** |
|
||||
| 9 | Zero texture creation or destruction per frame in steady state | one create + one destroy per line of text per frame | **missed** |
|
||||
| 10 | A handled, routine condition costs no more than twice the path that succeeds | 616.5 ns vs 68.4 ns — 9x | **missed** |
|
||||
| 11 | 256 actors simulated, updated and made ready to draw in under 1 ms | ~22 µs at 64 actors (5.8 µs measured logic + estimated render bookkeeping); linear, so ~90 µs extrapolated | **met, untested at that size** |
|
||||
| 12 | Collision for 256 actors under 2 ms without the caller writing a broad phase | no broad phase exists; the naive loop is 1.9 ms at 256 actors | **missed** |
|
||||
| 13 | Level load under 100 ms for a map with 8 tilesets, 4 layers and 64 actors | 11.9 ms for a 2x2 map with one tileset | **unknown at that size** |
|
||||
| 14 | Fixed per-load overhead under 1% of a level load | 11.5% — zeroing 26 MB of `akgl_Tilemap` | **missed** |
|
||||
| 15 | Static footprint under 4 MB in the default configuration | 28 MB, 94% of it one tilemap | **missed** |
|
||||
| 16 | No pool leaks across a load/release cycle of any asset type | tilemap leaks 5 strings per cycle (item 29) | **missed** |
|
||||
| 17 | Pool exhaustion reports `AKGL_ERR_HEAP` and never crashes | item 30 crashes | **missed** |
|
||||
| 18 | Every benchmark held to 10x its recorded baseline, enforced in CI | done, `ctest -L perf` | **met** |
|
||||
|
||||
Notes on the ones worth arguing about:
|
||||
|
||||
- **6 and 7 are the same fix.** The acquire scan is 64x slower on a full string
|
||||
pool than an empty one purely because the pool is a megabyte and the scan
|
||||
touches one refcount per 4 KiB. A free-list index — one `int` per layer,
|
||||
remembering where the last free slot was — takes both to constant time without
|
||||
changing the "no `malloc`" rule at all. That is the change I would make first,
|
||||
and it is worth doing *before* anyone raises `AKGL_MAX_HEAP_*`, because the
|
||||
cost of the current design grows with the ceiling rather than with the usage.
|
||||
|
||||
- **8 and 9 are one cache.** A single entry keyed on (font, string, colour,
|
||||
wrap) would cover the common case — a HUD field that changes once a second —
|
||||
and a four- or eight-entry ring would cover the rest. This is the clearest
|
||||
optimisation in the library and it is maybe forty lines.
|
||||
|
||||
- **10 is a design target, not a speed target.** The answer is not a faster
|
||||
error context. It is that "this character has no sprite for this state" is a
|
||||
question with a boolean answer, and reporting it through `AKERR_KEY` costs
|
||||
nine times the update it replaces. `akgl_character_sprite_get` wants a
|
||||
companion that returns `NULL` without raising.
|
||||
|
||||
- **12 is a scope decision, not a defect.** The library deliberately does not
|
||||
own a broad phase, and at 64 actors the naive all-pairs loop is 0.7% of a
|
||||
frame — genuinely fine. At 256 it is 11%. Either the ceiling stays where it is
|
||||
and this target is dropped, or a uniform grid keyed on tile size goes in. I do
|
||||
not think a spatial index belongs here yet; I think the target belongs on
|
||||
record so that raising `AKGL_MAX_HEAP_ACTOR` is a decision made with the
|
||||
number in front of it.
|
||||
|
||||
- **14 and 15 are the same fix too.** `akgl_Tilemap` is 26 MB because every
|
||||
layer carries a 512x512 `int` grid and every tileset a 65,536-entry offset
|
||||
table, sized for the worst case at compile time. The pool rule does not
|
||||
require *this*: a layer could carry an index into one shared cell arena sized
|
||||
by `AKGL_TILEMAP_MAX_WIDTH * AKGL_TILEMAP_MAX_HEIGHT` once rather than
|
||||
sixteen times, and the offset table could be sized by `tilecount` rather than
|
||||
by the maximum. That is a real refactor with a real ABI break, so it belongs
|
||||
to 0.4 rather than to a patch release — but 28 MB of BSS on a handheld or an
|
||||
ESP32-class target is the difference between fitting and not.
|
||||
|
||||
- **13 is untested and should not stay that way.** The only map fixture in the
|
||||
tree is 2x2 with one tileset, which is why the load benchmark measures a PNG
|
||||
decode and a `memset` rather than a map. A realistic fixture — 128x128, four
|
||||
layers, several tilesets — would make target 13 measurable and would probably
|
||||
find something.
|
||||
|
||||
## Build notes
|
||||
|
||||
The vendored SDL satellite libraries are built into per-project subdirectories
|
||||
|
||||
Reference in New Issue
Block a user