feat(debug): engine-fed FPS counter + sorted per-unit profiler tables (labelle-engine#380) - #640
Conversation
… (labelle-engine#380) Rewires the inspector's Performance section onto the engine's profiler APIs (engine PR labelle-engine#380 follow-up of #733): - FPS header now reads Game.frameStats()/frameTimeMs() — the always-on engine FrameProfiler — replacing the 0.16-migration stub that faked a 16ms counter (FPS was hardwired ~62.5 since std.time.milliTimestamp went away). Mini-graph reads Game.frameHistory() (real dt ring). - Per-unit tables: Game.scriptProfileRows()/pluginProfileRows() typed rows replace the stale hand-rolled @ptrCast layouts (which no longer matched the engine's Stat-based rows). Scripts and Plugins render as tables sorted by per-frame cost (tick+post+gui, setup excluded), columns per lifecycle phase, '-' for never-ran, severity markers (* >1ms, ! >5ms — text until GuiInterface grows colored labels), per-group ms totals. - Live capture without env var: arms Game.setProfilingCapture(true) while the panel + Show Performance are visible, hands back null on close so a user's LABELLE_PROFILE headless dump keeps running. - LABELLE_DEBUG_OPEN=1 opens the inspector at boot (headless screenshot verification; skips the F12). - All engine reads @hasDecl/@hasField-gated: against an engine without the API (< 2.5) the section degrades gracefully, older row layouts read missing phases as 0. Tests: sorting excludes setup, severity marks, phase-cell formatting, old-row tolerance. Requires labelle-engine with Game.frameStats/scriptProfileRows for the full panel (2.4.0 + labelle-engine#380 follow-up; degrades below that). Claude-Session: https://claude.ai/code/session_011szWvquoss1yNX7KWSKCaM
|
Engine API side: labelle-toolkit/labelle-engine#784. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 43 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: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors the performance and FPS tracking in plugins/debug/src/root.zig to utilize the engine's built-in profilers, introducing structured performance tables for scripts and plugins along with corresponding unit tests. The reviewer identified three compatibility and UI issues: first, if a performance table is collapsed, the total time incorrectly displays as 0.00ms because the summation is skipped; second, game.frameTimeMs() is called without a comptime check, risking compilation failures on older engines; and third, game.pluginProfileRows() similarly lacks a @hasDecl check, which could also break builds on older engine versions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| var total_ns: u64 = 0; | ||
| if (Gui.beginTable(title, cols)) { | ||
| Gui.tableNextRow(); | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label("name"); | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label("tick"); | ||
| if (show_post) { | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label("post"); | ||
| } | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label("gui"); | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label("setup"); | ||
|
|
||
| for (rows) |r| { | ||
| total_ns += r.frameNs(); | ||
| Gui.tableNextRow(); | ||
| var name_buf: [96]u8 = undefined; | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label(std.fmt.bufPrintZ(&name_buf, "{s}", .{r.name}) catch "?"); | ||
| var cell: [24]u8 = undefined; | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label(fmtPhase(&cell, r.tick_ns)); | ||
| if (show_post) { | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label(fmtPhase(&cell, r.post_ns)); | ||
| } | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label(fmtPhase(&cell, r.gui_ns)); | ||
| _ = Gui.tableNextColumn(); | ||
| Gui.label(fmtPhase(&cell, r.setup_ns)); | ||
| } | ||
| Gui.endTable(); | ||
| } |
There was a problem hiding this comment.
If Gui.beginTable returns false (e.g., when the table is collapsed or clipped), the loop that sums total_ns is skipped entirely. This causes the "Total" label printed outside the table to incorrectly display 0.00ms. Summing the total_ns before the beginTable check ensures the total remains accurate regardless of the table's visibility state.
var total_ns: u64 = 0;
for (rows) |r| total_ns += r.frameNs();
if (Gui.beginTable(title, cols)) {
Gui.tableNextRow();
_ = Gui.tableNextColumn();
Gui.label("name");
_ = Gui.tableNextColumn();
Gui.label("tick");
if (show_post) {
_ = Gui.tableNextColumn();
Gui.label("post");
}
_ = Gui.tableNextColumn();
Gui.label("gui");
_ = Gui.tableNextColumn();
Gui.label("setup");
for (rows) |r| {
Gui.tableNextRow();
var name_buf: [96]u8 = undefined;
_ = Gui.tableNextColumn();
Gui.label(std.fmt.bufPrintZ(&name_buf, "{s}", .{r.name}) catch "?");
var cell: [24]u8 = undefined;
_ = Gui.tableNextColumn();
Gui.label(fmtPhase(&cell, r.tick_ns));
if (show_post) {
_ = Gui.tableNextColumn();
Gui.label(fmtPhase(&cell, r.post_ns));
}
_ = Gui.tableNextColumn();
Gui.label(fmtPhase(&cell, r.gui_ns));
_ = Gui.tableNextColumn();
Gui.label(fmtPhase(&cell, r.setup_ns));
}
Gui.endTable();
}
There was a problem hiding this comment.
Fixed in 2544c55. total_ns is now summed unconditionally from the rows BEFORE the beginTable branch, so the Total footer is correct even when the table body is collapsed/clipped (beginTable returns false).
| const st = game.frameStats(); | ||
| var fps_buf: [64]u8 = undefined; | ||
| Gui.label(std.fmt.bufPrintZ(&fps_buf, "FPS: {d:.0} | Frame: {d:.1}ms", .{ st.fps, game.frameTimeMs() }) catch "?"); |
There was a problem hiding this comment.
While frameStats is gated at comptime, game.frameTimeMs() is called directly without a check. If an older engine version implements frameStats but lacks frameTimeMs, compilation will fail. Adding a comptime @hasDecl check for frameTimeMs ensures graceful degradation and prevents compile breaks.
const st = game.frameStats();
const frame_ms = if (comptime @hasDecl(Game, "frameTimeMs")) game.frameTimeMs() else 0.0;
var fps_buf: [64]u8 = undefined;
Gui.label(std.fmt.bufPrintZ(&fps_buf, "FPS: {d:.0} | Frame: {d:.1}ms", .{ st.fps, frame_ms }) catch "?");
There was a problem hiding this comment.
Fixed in 2544c55. frameStats and frameTimeMs are now folded under a single capability check — !(@hasDecl(Game, "frameStats") and @hasDecl(Game, "frameTimeMs")) — so an engine exposing one but not the other still compiles (degrades to the 'n/a' label).
There was a problem hiding this comment.
This call site IS gated — it's the same single frameTimeMs() call the :134 fix folded; the line moved to 159 after the insertTopRow addition shifted the file. drawFpsHeader early-returns at the top under one capability check: if (comptime !(@hasDecl(Game, "frameStats") and @hasDecl(Game, "frameTimeMs"))) { Gui.label("FPS: n/a …"); return; } — so by line 159 both accessors are guaranteed present. grep -n frameTimeMs shows exactly one call (line 159) plus the comment on 152; there is no second ungated site. (2544c55)
There was a problem hiding this comment.
Re-anchor of the single frameTimeMs() call already gated (round 2, 2544c55): drawFpsHeader early-returns under one check gating BOTH frameStats and frameTimeMs, so the call is only reached when both are present. grep -n frameTimeMs shows exactly one call site. No change needed.
| { | ||
| const src = game.pluginProfileRows(); | ||
| const n = @min(src.len, MAX_PERF_ROWS); | ||
| for (src[0..n], 0..) |r, i| rows_buf[i] = .{ | ||
| .name = r.name, | ||
| .setup_ns = phaseNs(r, "setup"), | ||
| .tick_ns = phaseNs(r, "tick"), | ||
| .post_ns = phaseNs(r, "post_tick"), | ||
| .gui_ns = phaseNs(r, "draw_gui"), | ||
| }; | ||
| drawPerfGroup(Gui, "Plugins", rows_buf[0..n], true); | ||
| } |
There was a problem hiding this comment.
The outer function drawPerfTables only checks for the presence of scriptProfileRows. Calling game.pluginProfileRows() without a corresponding @hasDecl check can cause compilation failures on older engines that might only support script profiling. Wrapping the plugin profiling block in a comptime check ensures robust compatibility.
if (comptime @hasDecl(Game, "pluginProfileRows")) {
const src = game.pluginProfileRows();
const n = @min(src.len, MAX_PERF_ROWS);
for (src[0..n], 0..) |r, i| rows_buf[i] = .{
.name = r.name,
.setup_ns = phaseNs(r, "setup"),
.tick_ns = phaseNs(r, "tick"),
.post_ns = phaseNs(r, "post_tick"),
.gui_ns = phaseNs(r, "draw_gui"),
};
drawPerfGroup(Gui, "Plugins", rows_buf[0..n], true);
}
There was a problem hiding this comment.
Fixed in 2544c55. drawPerfTables now gates each accessor independently: the scripts block is under @hasDecl(Game, "scriptProfileRows") and the plugins block under @hasDecl(Game, "pluginProfileRows"), so an engine exposing only one still compiles.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27fbba2311
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const n = @min(src.len, MAX_PERF_ROWS); | ||
| for (src[0..n], 0..) |r, i| rows_buf[i] = .{ |
There was a problem hiding this comment.
Select top profiler rows before truncating
When a game has more than 64 scripts, this slices to the first 64 rows before sorting, so an expensive script registered later is never copied into rows_buf and the table is only sorted within that prefix; the same pattern below affects plugins. Please sort/select across the full source rows and only then cap the displayed list.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2544c55. Replaced the truncate-before-sort with a streaming insertTopRow that keeps the top MAX_PERF_ROWS by per-frame cost in one pass, so the priciest rows survive regardless of source order (late-registered hot scripts included). Added a regression test: fill the cap with cheap rows, then insert the priciest ones AFTER the buffer is full — they land at the front and the smallest is evicted.
Verification (on-screen panel)Built a sokol demo with three busy-work scripts + the debug plugin, ran headless surfaceless (sokol true-surfaceless capture), overlay opened via
Known wart → follow-up (assembler codegen)The per-plugin rows label the plugin by its tuple index ( Overhead: ~51 ns per dispatched unit while capturing, ~2 ns when off (engine PR has the microbench). |
…pluginProfileRows Round-1 review fixes on the inspector Performance section: - Top-N truncation before sort (codex P2): drawPerfTables sliced the first MAX_PERF_ROWS source rows THEN sorted, so with >64 scripts the priciest (registered later) never appeared. Replaced with a streaming insertTopRow that keeps the top-N by per-frame cost regardless of source order — sort-then-truncate semantics in one pass. Regression test: cap+ rows with the priciest arriving AFTER the buffer is full lands at the front. - Total skipped when table collapses (gemini): total_ns was summed inside the beginTable body but the Total label prints outside, so a collapsed/clipped table showed a stale/zero total. Total is now summed unconditionally from the rows before the render branch. - frameTimeMs unguarded (gemini): game.frameTimeMs() was called while only frameStats was @hasDecl-gated. Both now fold under one capability check. - pluginProfileRows unguarded (gemini): drawPerfTables gated only on scriptProfileRows but called pluginProfileRows. Each accessor is now gated independently, so an engine exposing only one still compiles. Plugin zig build test green. Claude-Session: https://claude.ai/code/session_011szWvquoss1yNX7KWSKCaM
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2544c55f62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // if `beginTable` returns false (collapsed / clipped) the row loop | ||
| // is skipped, and a total computed inside it would print stale/zero. | ||
| var total_ns: u64 = 0; | ||
| for (rows) |r| total_ns += r.frameNs(); |
There was a problem hiding this comment.
Sum profiler totals before capping rows
When a game has more than MAX_PERF_ROWS scripts or plugins, rows is already the capped top-64 slice built by insertTopRow, so the Total Scripts/Total Plugins footer omits every hidden row. That under-reports total frame cost exactly in larger projects; accumulate the total while iterating all game.*ProfileRows() before truncating the displayed rows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in dd66d1d. drawPerfTables now accumulates the full total (and full unit count) across EVERY source row, before the top-N cap, and passes both to drawPerfGroup; the footer prints that full total and, when truncated, notes (top N of M). drawPerfGroup no longer re-sums its capped rows. Regression test perf total sums ALL units, not just the displayed top-N: MAX_PERF_ROWS+10 equal-cost units → full total counts all of them and strictly exceeds the displayed top-N sum.
Round-2 review (codex, root.zig:235): the Total footer summed only the top-N rows kept by insertTopRow, so with more than MAX_PERF_ROWS scripts/plugins the total undercounted (it dropped the cost of every truncated unit). drawPerfTables now accumulates the full total (and full unit count) across EVERY source row, before the top-N cap, and passes both to drawPerfGroup. The footer prints the full total and, when truncated, notes "(top N of M)" so the display set vs the accounted set is unambiguous. drawPerfGroup no longer re-sums its (capped) rows. Regression test: MAX_PERF_ROWS+10 equal-cost units -> full total counts all of them and strictly exceeds the sum of the displayed top-N. Plugin zig build test green. Claude-Session: https://claude.ai/code/session_011szWvquoss1yNX7KWSKCaM
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd66d1de00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const Game = @TypeOf(game.*); | ||
| var rows_buf: [MAX_PERF_ROWS]PerfRow = undefined; | ||
|
|
||
| if (comptime @hasDecl(Game, "scriptProfileRows")) { |
There was a problem hiding this comment.
Gate profiler tables when capture cannot be armed
When running with the repo's default engine 2.5.0 and no LABELLE_PROFILE, Game already has scriptProfileRows/pluginProfileRows but lacks setProfilingCapture, so syncProfilingCapture returns without enabling recording. This check still enters the table-rendering path and shows - cells and 0.00ms totals as if they were real live timings, even though nothing is being sampled. Please gate the tables on capture support or LABELLE_PROFILE in this compatibility path, or show the fallback hint instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2b927f6. drawPerfTables now gates up front on capture being genuinely armable AND active:
- Engine lacking
setProfilingCapture/profilingCaptureActive(< 2.6, e.g. 2.5 which has the row accessors but not the capture-control API): printsprofiling capture unavailable - requires engine >= 2.6.0and returns — no more wall of-/0.00ms. - Capture-capable but recording currently off: prints
profiling capture inactiveand returns. - Only when
game.profilingCaptureActive()is true does it render the tables.
Test drawPerfTables gates on capture armability: a label-capturing stub Gui drives it against three mock engines (old/no-API → 'unavailable', new/active → tables shown, new/idle → 'inactive'), asserting the tables appear only when active.
Round-3 review (codex P2, root.zig:198): against an engine that exposes scriptProfileRows/pluginProfileRows (>= 2.4) but NOT setProfilingCapture (< 2.6), syncProfilingCapture is a no-op, nothing enables recording, and every row reads back 0 — so the tables rendered a wall of `-`/`0.00ms` that reads as a broken panel. drawPerfTables now gates up front: - No setProfilingCapture/profilingCaptureActive (< 2.6): print "profiling capture unavailable - requires engine >= 2.6.0" and return. - Capture-capable but recording currently off: print "profiling capture inactive" and return. - Only when capture is genuinely active does it render the tables. Test: a label-capturing stub Gui drives drawPerfTables against three mock engines (old/no-API, new/active, new/idle) asserting the right line shows and the tables appear only when active. Plugin zig build test green. Claude-Session: https://claude.ai/code/session_011szWvquoss1yNX7KWSKCaM
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b927f6795
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (comptime !@hasDecl(Game, "setProfilingCapture") or !@hasDecl(Game, "profilingCaptureActive")) { | ||
| Gui.label("profiling capture unavailable - requires engine >= 2.6.0"); | ||
| return; |
There was a problem hiding this comment.
Preserve env-enabled profiling on 2.5 engines
When a game is pinned to the released engine that already has scriptProfileRows/pluginProfileRows but not the new capture-control methods, launching with LABELLE_PROFILE=1 still produces real per-unit rows; this unconditional early return now hides them behind the “requires engine >= 2.6.0” message. Please allow the no-capture-control compatibility path to render when the env gate is already enabled, and only suppress the zero-row/no-env case.
Useful? React with 👍 / 👎.
| updateFpsTracking(); | ||
| // Keep the engine's live per-unit capture in step with panel | ||
| // visibility (runs even when hidden, so closing disarms it). | ||
| syncProfilingCapture(game, debug_visible and show_perf); |
There was a problem hiding this comment.
Skip capture on wasm profiler builds
On wasm/editor-preview builds, the engine profiler clock returns 0, but this still forces setProfilingCapture(true) whenever the panel is open; profilingCaptureActive() then reports true and drawPerfTables renders rows of -/0.00ms instead of the fallback, even though no real per-unit timings can be sampled. Gate this override to targets with a usable profiler clock, or avoid rendering the tables in that context.
Useful? React with 👍 / 👎.
What
Rewires the debug inspector's Performance section onto the engine's profiler APIs — the UI half of labelle-engine#380 (engine data side shipped in labelle-engine#733 + labelle-engine#784).
Game.frameStats()/frameTimeMs()(engine's always-onFrameProfiler). The previous code was a Zig-0.16-migration stub faking a fixed 16ms counter, so the FPS readout had been hardwired to ~62.5 sincestd.time.milliTimestampwent away. The mini frame-time graph now rendersGame.frameHistory()(real dt ring, newest 40 frames).Game.scriptProfileRows()/pluginProfileRows()typed rows replace the stale hand-rolled@ptrCastlayouts (which silently mismatched the engine's currentStat-based rows). Scripts and Plugins render as tables sorted by per-frame cost (tick+postTick+drawGui; one-shot setup excluded from the sort key but shown in its own column),-for never-ran phases, severity markers (*>1ms,!>5ms) and per-group totals.Game.setProfilingCapture(true)while the panel + Show Performance are visible, hands backnullon close so a user'sLABELLE_PROFILEheadless dump keeps running.LABELLE_DEBUG_OPEN=1opens the inspector at boot (headless screenshot verification, skips the F12).All engine reads are
@hasDecl/@hasField-gated: against an engine without the new API the section degrades to a hint label, and older row layouts read missing phases as 0 — no compile break for games pinning older engines.Verification
Ran on flying-platform-labelle (bgfx desktop) via
local:overrides for this plugin + the engine PR branch: inspector opens at boot, FPS header live, per-script/per-plugin tables populate, and the profiler's 120-frame log dumps fire purely from the panel arming capture (noLABELLE_PROFILEset). Capture overhead measured at ~51 ns per dispatched unit while the panel is open (~2.3 µs/frame at FP scale), ~2 ns when closed.Plugin tests: sorting excludes setup, severity marks, phase-cell formatting, old-row tolerance (
zig build testinplugins/debug, exit-checked).Pins
Full panel needs a labelle-engine release cut from labelle-engine#784 (next minor). Degrades gracefully below that. Follow-ups tracked on the engine PR: colored severity labels (needs
labelColoredon core's GuiInterface + imgui adapter), per-language-script breakdown via labelle-scripting.https://claude.ai/code/session_011szWvquoss1yNX7KWSKCaM
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.