Skip to content

Stop the Touch Bar snapshot doing I/O on the main thread - #138

Open
offyotto wants to merge 3 commits into
mainfrom
app/touchbar-snapshot-off-main
Open

offyotto wants to merge 3 commits into
mainfrom
app/touchbar-snapshot-off-main

Conversation

@offyotto

@offyotto offyotto commented Nov 2, 2026

Copy link
Copy Markdown
Owner

Why

CoreMonTouchBarController.makeSnapshot() runs on the main thread:

  • on every 10 s refresh;
  • once per widget in configure(_:) whenever the bar is rebuilt. startAppTouchBar() runs when Core-Monitor becomes active and again shortly after any other app is activated, so app switching triggers it too.

Each snapshot made two system calls on the main thread:

  • storageUsagePercent() called URL.resourceValues for the home volume's capacity. SystemMonitor already samples the same figure off the main thread as diskStats.usagePercent, using the same formula: used = total − available for important usage.
  • currentWiFiName() called CWWiFiClient.shared().interface()?.ssid(). Nothing reads TouchBarSystemSnapshot.wifiName.

Change

  • ssdPct now comes from systemMonitor.diskStats.usagePercent.
  • The unused wifiName field, currentWiFiName(), storageUsagePercent() and the controller's import CoreWLAN are removed.
  • The Wi-Fi status widget (SWifiItem) has its own CWWiFiClient and is unchanged.

The SSD bar now follows SystemMonitor's disk cache rather than reading the volume live. Because it refreshes every 10 s anyway, the value should look the same.

Verified

  • xcodebuild test -scheme Core-Monitor -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO passes locally on Xcode 27.0: TEST SUCCEEDED, 0 failures.

Not done: I did not check this on Touch Bar hardware.

Every Touch Bar refresh, and every widget configured when the bar is
rebuilt on app activation, built a snapshot on the main thread that:

- queried the home volume's capacity with URL.resourceValues, although
  SystemMonitor already samples the same figure off the main thread as
  diskStats.usagePercent (used = total - available for important usage);
- asked CoreWLAN for the current SSID, a value no widget reads. The Wi-Fi
  status widget has its own CWWiFiClient and is unaffected.

The snapshot now uses diskStats.usagePercent and the unused wifiName field
is removed.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T14:11:32.553522Z 55b4e14 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@gitar-bot
gitar-bot Bot enabled auto-merge (squash) November 2, 2026 14:11

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitar has auto-approved this PR (configure)

@gitar-bot gitar-bot Bot added the gitar-approved Added by Gitar label Nov 2, 2026
@gitar-bot

gitar-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

🟡 Medium risk · Using cached disk usage may make the Touch Bar’s SSD value stale; the excluded GroupViews.swift deletion has unclear impact.

Moves the Touch Bar snapshot's SSD usage to the cached SystemMonitor disk stats and removes the unused Wi-Fi name lookup, so makeSnapshot() no longer does I/O on the main thread. No issues found.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Approved

For more verification details, comment gitar display:verbose.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

This branch has not been deployed

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

Labels

gitar-approved Added by Gitar

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant