zsh integration: use zsh/zselect for poll-loop sleeps (no fork)#6032
Open
jsamuel1 wants to merge 1 commit into
Open
zsh integration: use zsh/zselect for poll-loop sleeps (no fork)#6032jsamuel1 wants to merge 1 commit into
jsamuel1 wants to merge 1 commit into
Conversation
The git-HEAD and PR-status watchers run one background loop per pane and fork /bin/sleep once per second indefinitely. On a busy machine with many panes this is a measurable, continuous source of process churn — which also multiplies under host security/EDR agents that inspect every exec. Replace the in-shell `sleep` calls with a `_cmux_sleep_cs` helper backed by the `zsh/zselect` builtin (its `-t` timeout sleeps without forking), falling back to /bin/sleep when the module is unavailable. This mirrors the existing zsh/net/unix and zsh/parameter module-over-fork optimizations already in this file. Note: zselect with only -t and no fds returns status 1 on timeout, so the helper calls it then returns 0 explicitly rather than falling through to the /bin/sleep fallback (which would otherwise double the wait). Verified: `zsh -n` passes; zselect -t 100 sleeps ~1.00s fork-free in a backgrounded subshell with redirected std streams; fallback path sleeps correctly when the module is absent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@jsamuel1 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR optimizes timing waits in monitoring loops by replacing ChangesFork-free sleep optimization for polling loops
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 21✅ Passed checks (21 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Greptile SummaryThis PR replaces
Confidence Score: 5/5Safe to merge — the change is a pure performance optimisation of existing sleep calls with no behavioural side-effects. Every modified call site replaces an identical-duration /bin/sleep with _cmux_sleep_cs; the helper handles the zselect status-1-on-timeout edge case correctly with an explicit return 0; the float-arithmetic fallback ($(( $1 / 100.0 ))) is valid zsh and produces correct values for both argument values used (100 and 20); the module availability flag is set once at startup and is inherited correctly by the disowned background subshells; no new polling or synchronisation patterns are introduced. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Shell integration loads] --> B{zmodload zsh/zselect}
B -- success --> C[_CMUX_HAS_ZSELECT=1]
B -- fail --> D[_CMUX_HAS_ZSELECT=0]
C & D --> E[_cmux_sleep_cs N called]
E --> F{_CMUX_HAS_ZSELECT?}
F -- yes --> G[zselect -t N]
G --> H[return 0 explicitly - no fork]
F -- no --> I[sleep via /bin/sleep fallback]
H & I --> J[Caller continues]
subgraph Watchers
K[_cmux_start_git_head_watch poll tick]
L[_cmux_start_pr_poll_loop countdown tick]
M[_cmux_run_pr_probe_with_timeout TERM/KILL grace]
end
J --> Watchers
Reviews (1): Last reviewed commit: "zsh integration: use zsh/zselect for pol..." | Re-trigger Greptile |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
zsh integration: use
zsh/zselectfor poll-loop sleeps (no fork)Problem
The shell integration starts two background watcher loops per pane:
_cmux_start_git_head_watch— polls.git/HEADevery second to refresh the branch/PR display_cmux_start_pr_poll_loop— counts down its poll interval in 1-second stepsBoth loops fork
/bin/sleeponce per second, indefinitely, for the life of the pane. With several panes open this is a continuous, low-level source of process churn. It's especially visible on managed machines where host security/EDR agents (CrowdStrike, FortiDLP, etc.) inspect everyexec— eachsleepfork fans out to every Endpoint Security client.This is the same class of overhead the file already optimizes away for socket sends (
zsh/net/unix, "~0.2ms vs ~3ms for fork+exec") and job-table checks (zsh/parameter).Change
Add a
_cmux_sleep_cshelper (argument in centiseconds) backed by thezsh/zselectbuiltin, whose-ttimeout sleeps without forking. Falls back to/bin/sleepwhen the module is unavailable. Route the five in-shellsleepcalls in the watcher loops through it.Net effect: the steady-state per-pane sleep fork rate drops from ~1/sec to 0.
Subtlety worth a look
zselectwith only-tand no fds returns status 1 on timeout. The helper therefore callszselectand thenreturn 0explicitly — a naivezselect -t "$1" || sleep ...would fall through and double every wait.Testing
zsh -npasses on the modified file.zselect -t 100sleeps ~1.00s fork-free in a backgrounded subshell with redirected std streams + stdin from/dev/null(the watchers' exact context)._CMUX_HAS_ZSELECT=0) sleeps correctly via/bin/sleep._cmux_halt_pr_poll_loop) is unaffected —zselect, likesleep, is interrupted by the signal.I did not wire this into the VM-based test suite in
tests/since those run against a built app; happy to add coverage if you'd like a specific form.Scope
zsh only.
bashhas no equivalent non-forking sleep builtin (macOS ships bash 3.2), socmux-bash-integration.bashis intentionally left unchanged.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Use
zsh/zselectfor watcher loop sleeps to avoid forking\bin\sleep. This removes the steady 1/sec fork per pane and reduces process churn on EDR-heavy machines._cmux_sleep_cs(centiseconds) usingzsh/zselect -t; returns 0 on timeout; falls back to/bin/sleep._cmux_start_git_head_watch,_cmux_start_pr_poll_loop, and probe timeout waits through the helper; preserves signal interruption.Written for commit c37af7c. Summary will update on new commits.
Summary by CodeRabbit