Conversation
…probe (partial #91) Two targeted warm-path fixes identified during #91 profiling: F1 — `spawn_daemon_process` on Windows previously let the daemon inherit every inheritable handle in the CLI process, including the parent shell's stderr pipe (attached via `|`, `2>&1`, or PowerShell's default pipeline). The daemon holds that pipe open for SELF_EVICTION_TIMEOUT (120s), so the shell stays blocked until the timer fires even after the CLI's main() has returned — reproducibly visible as a ~125s stall in PowerShell. Fix: call `SetHandleInformation(GetStdHandle(STD_*_HANDLE), HANDLE_FLAG_INHERIT, 0)` on the CLI's std handles immediately before `cmd.spawn()`. Rust's Command still duplicates the explicit Stdio handles it configured (null/null/log_file) as inheritable for the daemon, so stderr-to-log-file redirection is preserved; only the shell-owned pipe is stripped. F2 — `DaemonClient::new()` now builds its reqwest client with a 100ms `connect_timeout`. When the daemon is not running, TCP connect refusal returns instantly on Windows/Linux, so waiting the full 2s `.timeout()` of each probe just wastes wall-clock. Measured: `fbuild daemon status` cold path dropped from 4.0s to 0.28s (2 probes × ~100ms). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 50 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Summary
Two targeted warm-path fixes identified in the #91 profiling report. Both live in
crates/fbuild-cli/src/daemon_client.rs.F1 — Strip
HANDLE_FLAG_INHERITfrom CLI stdio before spawning daemonspawn_daemon_processon Windows previously let the daemon grandchild inherit every inheritable handle in the CLI — including the parent shell's stderr pipe (attached via|,2>&1, or PowerShell's default pipeline). The daemon holds that pipe open forSELF_EVICTION_TIMEOUT= 120 s, so the shell stays blocked until that timer fires even after the CLI'smain()has returned.Reproducibly visible as a ~125 s stall in PowerShell; adding
2>$null(PS) or2>/dev/null(bash) eliminates the stall and drops total to ~4 s. Exact match to the 120 s eviction timeout was the smoking gun in #91's profiling.Fix: call
SetHandleInformation(GetStdHandle(STD_*_HANDLE), HANDLE_FLAG_INHERIT, 0)on the CLI's std handles immediately beforecmd.spawn(). Rust'sCommandstill duplicates the explicitStdiohandles it configured (null/null/log_file) as inheritable for the daemon, so stderr-to-log-file redirection is preserved; only the shell-owned pipe is stripped. Uses a tinyextern "system"FFI block — no new crate dependencies.F2 — 100ms
connect_timeoutonDaemonClientWhen the daemon is not running, TCP connect refusal returns instantly (<1 ms, verified with
Test-NetConnection). reqwest's per-request.timeout(2s)would nevertheless wait the full 2 s before surfacingECONNREFUSED.DaemonClient::new()is called from two sites (display_daemon_stats_compact+ensure_daemon_running), so the cold path eats 4 s before any work starts.Fix: build the shared client with
ClientBuilder::connect_timeout(Duration::from_millis(100)).Measured
fbuild daemon status(daemon not running, bash)fbuild build ...warm in PowerShell w/ attached stderrTest plan
uv run cargo build -p fbuild-cli --releaseclean.uv run cargo clippy --workspace --all-targets -- -D warningsclean.uv run cargo test -p fbuild-cli— 9/9 pass.daemon status4.0 s → 0.28 s.Scope
display_daemon_stats_compactdedupe) and F4 (SELF_EVICTION_TIMEOUT120 s → 30 s) from the perf(build): investigate warm-pass compilation stall — 30s where cache says <1s #91 report; both are independent cleanups best filed as follow-ups after this lands.Related
tasks/issue-91-report.md).