Repository navigation
perf: skip the per-flush stat and mkdir in the event log writer - #14828
Merged
Merged
Conversation
Every cmux event (agent hook, feed item, sidebar status) flushes the durable log. Each flush ran createDirectory, fileExists, open, seek and then attributesOfItem to learn the file size. An attach-only sample of a running 0.64.25 app with active Claude sessions put 353 of the event-log queue's 450 busy samples in attributesOfItem, which made this queue the largest idle CPU consumer in the process. The end-of-file offset from seekToEnd is the size, and the directory and file only need creating when the open fails. A per-flush micro benchmark (one 650-byte line per flush, 3000 flushes, swiftc -O) drops from 430-650 us to 195-265 us of CPU per flush. Rotation behavior is unchanged (scripts/benchmark-event-log-writes.py behavior checks pass, including the 16 MiB crossing case). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughThe event log writer now opens the log through an append helper and reads its current size from the handle’s end-of-file offset. The helper creates the parent directory and file when needed, then retries opening the log. ChangesEvent log append handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue remains; the event log continues to append and rotate as before. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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
|
Merge receipt for |
teamleaderleo
added a commit
to teamleaderleo/cmux
that referenced
this pull request
Sep 26, 2026
Every event flush (agent hooks, feed items, sidebar metadata; about 75 per minute on a busy machine) opened ~/.cmuxterm/events.jsonl, seeked, wrote, and closed it. manaflow-ai#14828 already dropped the per-flush stat and mkdir; this keeps the descriptor open on the event-log queue instead. The log is opened with O_APPEND | O_CREAT, so writes always land at the current end of file even when another cmux process sharing the log appended since the last flush; a seek-then-write handle could overwrite those lines. The rotation size comes from fstat on the open descriptor. The handle is reused only while the path still names the same file (device and inode match) and is reopened after an external rotation, deletion, or write failure. Directory creation still runs only when the open reports a missing parent. Rotation limits are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 of 2 tasks
rustybret
pushed a commit
to rustybret/bmux
that referenced
this pull request
Sep 26, 2026
2123231 fix(cloud): offer Pi in the cloud agent menu and vm.cloud_agent_open (manaflow-ai#14819) 909fcc7 fix(settings): stop promising a Tailscale QR the pairing window no longer shows (manaflow-ai#14817) 5d2bc04 fix(custom-sidebar): render Menu nodes so context-menu submenus appear (manaflow-ai#14808) 45815f7 Keep a collapsed sidebar group folded when the workspace below it closes (manaflow-ai#10169) 4bf0ea0 perf(shell): stop spawning tmux and rm on every prompt when idle (manaflow-ai#14833) 443d050 perf: skip the per-flush stat and mkdir in the event log writer (manaflow-ai#14828)
teamleaderleo
added a commit
that referenced
this pull request
Sep 26, 2026
* test: event-log flushes must reuse one open handle Each durable event flush reopened ~/.cmuxterm/events.jsonl, created its directory, seeked, and stat'ed it. The first test fails until the writer keeps its handle open across flushes. The second pins the safety condition for that change: after another process rotates or deletes the shared log, the next flush lands in the file now at the path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * perf: keep the durable event log open across flushes Every event flush (agent hooks, feed items, sidebar metadata; about 75 per minute on a busy machine) opened ~/.cmuxterm/events.jsonl, seeked, wrote, and closed it. #14828 already dropped the per-flush stat and mkdir; this keeps the descriptor open on the event-log queue instead. The log is opened with O_APPEND | O_CREAT, so writes always land at the current end of file even when another cmux process sharing the log appended since the last flush; a seek-then-write handle could overwrite those lines. The rotation size comes from fstat on the open descriptor. The handle is reused only while the path still names the same file (device and inode match) and is reopened after an external rotation, deletion, or write failure. Directory creation still runs only when the open reports a missing parent. Rotation limits are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Cover external rotation that leaves a replacement log at the path Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.
Summary
Every cmux event (agent hooks, feed items, sidebar status updates) flushes the durable log at
~/.cmuxterm/events.jsonl. Each flush rancreateDirectory,fileExists, open, seek, and thenattributesOfItemto learn the file size. With several Claude sessions running, that stat was the largest idle CPU cost in the app.This change reads the size from the
seekToEnd()offset and only creates the directory and file when the open fails. Rotation logic is unchanged.Evidence
sampleof a running 0.64.25 (build 106) app with active Claude sessions, 10 s: thecom.cmuxterm.event-logqueue was the busiest non-main queue (450 busy samples), and 353 of those were_FileManagerImpl.attributesOfItem, mostly bridging and freeing the attribute dictionary.swiftc -O -DDEBUG, alternating runs): 430 to 650 us of CPU per flush before, 195 to 265 us after. Wall time on that machine was dominated by system load, so CPU time is the number to read.scripts/benchmark-event-log-writes.py --baseline upstream/main: behavior checks pass for both variants, including the 16 MiB rotation crossing.Verification
No local app build (this Mac is not allowed to run them). Relying on CI for compile and the
CmuxEventLogWriterTestssuite.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Cuts idle CPU cost in the durable event log writer by reading the file size from the
seekToEnd()offset instead of a separate per-flush stat, and only creating the directory and file when the open fails. Per-flush CPU drops from 430–650 us to 195–265 us; rotation and write behavior are unchanged.Verification
CmuxEventLogWriterTestsare covered by CI.Written for commit 109a813. Summary will update on new commits.
Summary by CodeRabbit