fix(native): reduce startup probe latency after DA1#241
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughImplements a bounded DA1 drain timeout during terminal startup probing so the probe can exit early after a short DA1 drain window instead of waiting the full budget when XTVERSION never responds. Also updates the tracked vendor submodule commit and changelog entries. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/native/vendor/zireael/src/core/zr_detect.c (1)
868-907:⚠️ Potential issue | 🟠 MajorDA1 drain window is not enforced as a strict 20ms wall-clock cap.
At Line [888]-Line [890],
da1_drain_spent_msonly advances whenn == 0. If reads continue returning bytes, the DA1 drain budget never decreases, so probe time can extend well beyond the intended drain window.💡 Proposed fix (track elapsed time since DA1 observed)
- uint8_t da1_responded = 0u; - uint32_t da1_drain_spent_ms = 0u; + uint8_t da1_responded = 0u; + uint64_t da1_seen_ms = 0u; while (true) { int32_t timeout_ms = zr_detect_read_timeout_slice(start_ms, timeout_spent_ms); if (da1_responded != 0u) { - const int32_t da1_budget_ms = zr_detect_remaining_da1_drain_budget(da1_drain_spent_ms); + const uint64_t now_ms = plat_now_ms(); + const uint32_t da1_elapsed_ms = + (now_ms > da1_seen_ms) ? (uint32_t)(now_ms - da1_seen_ms) : 0u; + const int32_t da1_budget_ms = zr_detect_remaining_da1_drain_budget(da1_elapsed_ms); timeout_ms = zr_detect_clamp_timeout_budget(timeout_ms, da1_budget_ms); } @@ if (n == 0) { timeout_spent_ms += (uint32_t)timeout_ms; - if (da1_responded != 0u) { - da1_drain_spent_ms += (uint32_t)timeout_ms; - } continue; } @@ if (da1_responded == 0u) { zr_detect_parsed_t partial; zr_detect_parsed_reset(&partial); (void)zr_detect_parse_responses(collected, collected_len, &partial); da1_responded = partial.da1_responded; + if (da1_responded != 0u && da1_seen_ms == 0u) { + da1_seen_ms = plat_now_ms(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/native/vendor/zireael/src/core/zr_detect.c` around lines 868 - 907, DA1's 20ms drain budget isn't enforced because da1_drain_spent_ms is only incremented when plat_read_input_timed returns 0; fix by recording the wall-clock time when DA1 is first observed (e.g., add a da1_responded_ts or da1_drain_start_ms set when da1_responded transitions to non-zero inside zr_detect_read loop) and on each loop iteration compute elapsed = now_ms - da1_responded_ts and set da1_drain_spent_ms = (uint32_t)elapsed (or add to it if you prefer monotonic accumulation) before calling zr_detect_remaining_da1_drain_budget/zr_detect_clamp_timeout_budget so the DA1 drain budget decreases based on real elapsed time regardless of whether reads return bytes; update types and initialization to match existing uint32_t time variables and use the same time source used by zr_detect_read_timeout_slice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/native/vendor/zireael/src/core/zr_detect.c`:
- Around line 868-907: DA1's 20ms drain budget isn't enforced because
da1_drain_spent_ms is only incremented when plat_read_input_timed returns 0; fix
by recording the wall-clock time when DA1 is first observed (e.g., add a
da1_responded_ts or da1_drain_start_ms set when da1_responded transitions to
non-zero inside zr_detect_read loop) and on each loop iteration compute elapsed
= now_ms - da1_responded_ts and set da1_drain_spent_ms = (uint32_t)elapsed (or
add to it if you prefer monotonic accumulation) before calling
zr_detect_remaining_da1_drain_budget/zr_detect_clamp_timeout_budget so the DA1
drain budget decreases based on real elapsed time regardless of whether reads
return bytes; update types and initialization to match existing uint32_t time
variables and use the same time source used by zr_detect_read_timeout_slice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2992db7-c327-46bc-9e9c-1be3ecc3a0ef
📒 Files selected for processing (4)
CHANGELOG.mdpackages/native/vendor/VENDOR_COMMIT.txtpackages/native/vendor/zireael/src/core/zr_detect.cvendor/zireael
Summary
Root Cause
zr_detect_probe_terminal()waited up to the full 500ms probe budget when one query (typically XTVERSION) had no response, even if DA1/DA2 arrived quickly. Sincebackend.start()blocks on engine creation, this delayed first render.Fix Details
ZR_DETECT_DA1_DRAIN_TIMEOUT_MSand clamp read timeouts after DA1 is observed.Validation
npm run check:native-vendornpm -w @rezi-ui/native run build:nativenpm -w @rezi-ui/native run test:native:smoke(deep checks skipped in non-TTY CI shell)Dependency
Summary by CodeRabbit