Skip to content

Refactor polling to event-driven architecture for CPU usage fix#111

Open
onliner10 wants to merge 3 commits into
thdxg:mainfrom
onliner10:fix-cpu-usage-idle-polling
Open

Refactor polling to event-driven architecture for CPU usage fix#111
onliner10 wants to merge 3 commits into
thdxg:mainfrom
onliner10:fix-cpu-usage-idle-polling

Conversation

@onliner10

Copy link
Copy Markdown
Contributor

Replaces the 250ms polling timer with event-driven status tracking using libghostty callbacks.

Problem: Macterm used 10-20% CPU continuously when idle/minimized due to unconditional 250ms process polling timer.

Solution: Event-driven architecture:

  • Status updates flow through libghostty callbacks (onCommandFinished, onProgressStarted, onProgressFinished, onTerminalActivity)
  • Tab names refresh lazily on visibility changes (tabDidBecomeVisible, focusDidChange, app activation)
  • Single deferred work item for quiet settle (3s after last activity) - no repeating timer
  • App activation observers pause/resume activity tracking

Results:

  • Inactive/minimized: ~0% CPU (no timers running)
  • Active/idle: Near 0% CPU (no polling, only event-driven updates)
  • Activity: Updates respond to actual terminal events

Fixes #110

Replaces the 250ms polling timer with event-driven status tracking using
libghostty callbacks. Tab names refresh lazily on visibility changes, and a
single deferred work item handles quiet settle (3s after last activity).

- Removes processNameTimer polling loop entirely
- Status updates flow through onCommandFinished/onProgressStarted/onProgressFinished
- Tab names refresh on tabDidBecomeVisible/focusDidChange + app activation
- scheduleQuietSettle() uses DispatchWorkItem with 3s defer, not repeating
- App activation observers pause/resume activity tracking

Fixes thdxg#110
@github-actions github-actions Bot added area:ui Views, Settings UI area:terminal Terminal surface, ghostty integration labels Jul 1, 2026
…lling

# Conflicts:
#	Macterm/Ghostty/GhosttyCallbacks.swift
#	Macterm/Model/SplitNode.swift
#	MactermTests/Model/TerminalExecutionTrackerTests.swift
@onliner10

Copy link
Copy Markdown
Contributor Author

Updated this PR to be the combined fix for #110.

It now includes:

  • the event-driven status/activity tracking from this PR
  • the app-focus and surface-occlusion behavior from Turn off continuous rendering for panes that are not visible in the app. #115
  • source-aware execution tracking for foreground commands, OSC progress, and render/output activity
  • regression coverage for raw-mode TUIs, render-only updates, progress reports, resumed output, and prompt-redraw false positives

I also measured the larger warmed-tab case discussed in #110:

build/workload active visible inactive visible hidden
origin/main, 70 warmed tabs ~8.28% not sampled ~8.20%
combined #111, 70 warmed tabs ~1.41% ~0.86% ~0.84%

So this appears to fix the high-CPU scaling behavior while keeping the activity indicator reliable.

What do you think about landing this as the combined #110 fix?

@thdxg

thdxg commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Thanks @onliner10 for the thorough benchmarking and for engaging with @jcuhnoio's measurements. To answer your landing-path question: not quite this shape. Here's the plan and why.

  1. The occlusion work lands via #115, under @aryeh's authorship. With the occlusion hunks attributed there, jcuhnoio's interleaved benchmark showed the poll removal alone produced no measurable CPU change — the render loop was the cost. So this PR's remaining ~700 lines have to stand on correctness and power merits (killing the 4-wakeups/sec timer is a legitimate power win), and right now they don't. Please merge main into the branch after #115 lands (repo convention: merge, never rebase) and drop the duplicated occlusion/setAppFocus hunks.

  2. Issues found in review of the event-driven rewrite:

  • Crash: TerminalPane now reads a non-optional @Environment(AppState.self) in onActivityNeedsQuietSettle, but the quick terminal hosts it via NSHostingView(rootView:) without injecting the environment (QuickTerminal.swift:214) — first activity event in the quick terminal with the indicator on will fatalError.
  • Default-config title regression: showTabStatusIndicator defaults to false, and the new refresh triggers are all gated behind it — with defaults, tab titles only update on tab switch or app reactivation. Launching or quitting hx no longer retitles the tab, and the quit dialog can show a stale/dead process name. The 250ms poll these replace is load-bearing for title liveness and programTitle expiry (see the Tab Naming section of CLAUDE.md — which would also need updating).
  • Self-conflict with occlusion: the absorbed occlusion parking stops the render/scrollbar actions that the new event-driven status system depends on for hidden panes — an occluded TUI gets falsely settled to "done", and hidden-tab spinners (the indicator's main purpose) go dark.
  • Dead code: handleTerminalActivity / handleCommandFinished / handleProgressStarted / handleProgressFinished have no callers, tabDidBecomeVisible is only called from tests, lastVisibleTabID is write-only. The described callback layer was never wired; the live path is the pre-existing TerminalPane closures.
  • Scope: the TerminalExecutionTracker rewrite changes product behavior (removes progress-quiesce, inverts three pinned regression tests, weakens the fresh-restore checkmark protection). Some of it may be an improvement, but it needs its own PR with its own argument.
  1. A smaller version of this PR would merge quickly: keep the poll, but gate it — skip ticks while the app is inactive or the window fully occluded. That's a few lines, achieves ~0% when minimized/backgrounded, and preserves all the title-pipeline guarantees. The full event-driven rewrite is welcome afterward as its own PR once the items above are addressed.

Happy to re-review either shape. Thanks again for the benchmark rigor — it moved this issue forward materially.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:state AppState, models, persistence area:terminal Terminal surface, ghostty integration area:tests Test changes area:ui Views, Settings UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MacTerm constantly uses 10–20% CPU when any tab is open, even idle/minimized

2 participants