feat(#380): runtime profiling-capture toggle + setup/plugin-drawGui phase timing - #784
Conversation
…hase timing Completes the engine side of the debug-inspector profiling issue: - profiler.setRecording(?bool): runtime override of the LABELLE_PROFILE env gate. true/false force capture; null defers back to the env var, so the debug inspector can arm live capture while its Performance section is open without killing a user's env-enabled headless dump. - Game.setProfilingCapture / profilingCaptureActive / frameHistory accessors (MiscMixin) — the surface the debug plugin reads/drives. - New timed phases: ScriptRow/PluginRow gain a one-shot 'setup' Stat (recorded unconditionally at boot — two clock reads per unit, once) and PluginRow gains 'draw_gui' (rec-gated, mirrors script drawGui), so the overlay can show the full setup/tick/postTick/drawGui breakdown the issue asked for. dumpProfile resets the new draw_gui window; setup is never reset (boot cost). Zero-cost-when-off is unchanged: dispatch loops still branch on one cached bool per frame; the only unconditional additions are the one-shot setup timings. Tests: inspector_overlay_test — override round-trip (env-restoring), Game capture accessors, frameHistory passthrough, new phase fields. Closes #380 Claude-Session: https://claude.ai/code/session_011szWvquoss1yNX7KWSKCaM
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 33 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Companion UI PR: labelle-toolkit/labelle-assembler#640 (debug plugin panel consuming this API). |
There was a problem hiding this comment.
Code Review
This pull request introduces a runtime override mechanism for the LABELLE_PROFILE environment variable gate, allowing the debug inspector to toggle profiling capture dynamically. It also adds unconditional tracking for boot-time setup costs in both game scripts and plugin systems, as well as tracking for the drawGui phase in plugins. Additionally, new helper methods for retrieving frame history and managing profiling state are exposed, accompanied by comprehensive unit tests. I have no feedback to provide as there are no review comments.
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31d5ee253f
ℹ️ 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".
| setup: Stat = .{}, | ||
| tick: Stat = .{}, | ||
| post_tick: Stat = .{}, | ||
| draw_gui: Stat = .{}, |
There was a problem hiding this comment.
Include the new plugin phases in the overlay collector
The newly recorded plugin setup and draw_gui stats are dropped for callers using the documented overlay API: src/root.zig points consumers at game.pluginProfileRows() plus profiler.collect*Rows, but collectPluginRows() still flattens only tick and post_tick into OverlayRow. In a debug panel built on that public helper, plugin drawGui/setup will always be invisible even though SystemRegistry now records them, so the advertised full phase breakdown only works for consumers that bypass the collector.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in da9225c. OverlayRow now carries one named field per lifecycle phase — setup_ms/tick_ms/post_tick_ms/draw_gui_ms (each + severity) — so collectScriptRows/collectPluginRows surface the new plugin setup and draw_gui stats to any collector consumer. Scripts fill setup/tick/draw_gui (post_tick stays 0); plugins fill all four. The old aux_* secondary view is retained for back-compat, and totalMs now sums the recurring phases (tick+post_tick+draw_gui), still excluding one-shot setup. Tests assert the new named fields on both collectors.
There was a problem hiding this comment.
Re-verified — the collector DOES surface both new plugin phases; this re-anchors onto the PluginRow.draw_gui field decl (line 180), but the collector path is complete:
OverlayRow(scene/src/profiler.zig ~202) has named fieldssetup_ms/setup_severity,post_tick_ms/post_tick_severity,draw_gui_ms/draw_gui_severity(plustick_*).collectPluginRowspopulates ALL four:.setup_ms = nsToMs(e.setup.last_ns),.tick_ms,.post_tick_ms = nsToMs(e.post_tick.last_ns),.draw_gui_ms = nsToMs(e.draw_gui.last_ns)(~lines 258–267).collectScriptRowspopulatessetup/tick/draw_gui(scripts have no postTick).- Test
collectPluginRows: flattens setup + tick + postTick + drawGuiasserts thedebugrow'sdraw_gui_ms == 0.08andsetup_ms == 0.5flow through.
No source drop remains — the plugin setup/draw_gui stats reach any collector consumer. (da9225c)
Verified on a real renderRan a sokol game (three busy-work scripts + the debug plugin) headless-surfaceless with the overlay open, captured via the engine screenshot facility. Panel shows the engine-fed Where the timing hooks landed
Overhead (ReleaseFast, macOS, profiler's own
|
Round-1 review (codex P2, scene/src/profiler.zig): the newly-recorded plugin `setup` and `draw_gui` Stats were dropped by the documented OverlayRow collector — it only exposed `tick` + one `aux` secondary phase, so any consumer reading collectScriptRows/collectPluginRows missed them. OverlayRow now carries one named field per lifecycle phase (setup/tick/post_tick/draw_gui, each ms + severity); scripts populate setup/tick/draw_gui (post_tick stays 0), plugins populate all four. The old `aux_*` secondary view is kept for back-compat (drawGui for scripts, postTick for plugins). totalMs now sums the recurring phases (tick+post_tick+draw_gui) and still excludes one-shot setup. Tests updated to assert the new named fields and the setup-excluded total. Claude-Session: https://claude.ai/code/session_011szWvquoss1yNX7KWSKCaM
What
Ships the last engine-side pieces of the debug-inspector profiling issue. Most of #380 already landed earlier (per-script/per-plugin
LABELLE_PROFILEprofiler in v1.6-era, FPSFrameProfiler+ overlay data API in #733); this PR closes the remaining gaps the live inspector panel needs:profiler.setRecording(?bool)— runtime override of theLABELLE_PROFILEenv gate.true/falseforce per-unit capture on/off;nulldefers back to the env var. This is what lets the debug plugin arm live capture while its Performance section is open — and hand the gate back on close without killing a user's env-enabled headless dump.Game.setProfilingCapture/Game.profilingCaptureActive/Game.frameHistory(MiscMixin + re-exports) — the accessor surface the debug plugin drives/reads.frameHistoryexposes the frame-time ring (ms, oldest-first) for the mini-graph.ScriptRow/PluginRowgain a one-shotsetupStat (timed unconditionally at boot — two clock reads per unit, once; never window-reset) andPluginRowgainsdraw_gui(rec-gated inSystemRegistry.drawGui, mirroring scriptdrawGuitiming). The overlay can now show the full setup/tick/postTick/drawGui breakdown requested in the issue discussion.Zero-cost-when-off
Unchanged: dispatch loops still branch on one cached bool per frame. Measured (ReleaseFast, macOS, same
clock_gettimeprimitivenowNsuses):Stat.record)Tests
test/inspector_overlay_test.zig: override round-trip (restores env baseline), Game capture accessors,frameHistorypassthrough, new phase-field defaults.zig build testgreen (exit-checked).Companion PR
The inspector UI that consumes this lands in the labelle-assembler debug plugin (linked in comments): engine-fed FPS header + mini-graph, sorted per-unit ms tables with severity markers, panel-driven capture arming,
LABELLE_DEBUG_OPEN=1boot-open. Verified live on flying-platform-labelle vialocal:overrides. The plugin is@hasDecl/@hasField-gated, so it degrades gracefully against engines without this PR; the full panel needs an engine release cut from this (next minor, 2.5.0).Follow-ups (not this PR):
labelColoredon core'sGuiInterface+ imgui adapter; text markers (*/!) until then.Systems.tickrow is wanted.Closes #380
https://claude.ai/code/session_011szWvquoss1yNX7KWSKCaM
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.