Skip to content

perf: suppress redundant icon observer callbacks#1081

Merged
steipete merged 2 commits into
steipete:mainfrom
ptstory:perf/icon-observer-signature
May 21, 2026
Merged

perf: suppress redundant icon observer callbacks#1081
steipete merged 2 commits into
steipete:mainfrom
ptstory:perf/icon-observer-signature

Conversation

@ptstory
Copy link
Copy Markdown
Contributor

@ptstory ptstory commented May 21, 2026

Summary

Narrow the icon observation path so updateIcons() only fires when icon-relevant state changes.

The existing render signature inside applyIcon() already avoided redundant icon rendering, but it did not prevent redundant observer work from reaching updateIcons(). This change reduces observer callback and updateIcons() churn before render-signature checking is reached.

Split from #1073 per review feedback. Related: #678.

Not included

Problem

StatusItemController.observeStoreIconChanges() watches UsageStore.iconObservationToken, which registers Swift Observation dependencies on multiple store fields.

isRefreshing toggles on every refresh cycle regardless of icon state. That causes the observer to re-fire and call updateIcons() even when no icon-relevant state changed.

That does not necessarily mean every observer fire performs an expensive render: applyIcon() already has render-signature checks and often skips unchanged icon rendering. The issue fixed here is narrower: unnecessary observer/updateIcons work was still happening before those render-signature checks were reached.

Changes

  • Remove isRefreshing from iconObservationToken because refresh start/finish state is not itself icon-relevant.
  • Add storeIconObservationSignature() to derive a signature from actual icon-relevant state.
  • Guard the icon observer callback so it skips updateIcons() when the derived store icon signature is unchanged.
  • Add lightweight [perf] instrumentation gated behind debugLogLevel=verbose for future diagnosis.

Measurement

Measured with [perf] instrumentation gated by debugLogLevel=verbose.

Scope: startup refresh cycles only.

Metric Before (main) After (this branch)
updateIcons() calls per refresh cycle 9–11 (avg 10) 5–6 (avg 6)
Actual renders per refresh cycle 1 1–2
applyIcon() render-signature skips per refresh cycle 10–12 (avg 11) 6–8 (avg 7)

Scope note

This PR does not claim to fully explain or fix the original 99% CPU spike from #1073. After local account pruning from 7 managed Codex accounts to 3 kept/paid accounts, I can no longer reproduce the sustained CPU peg on demand.

The scoped claim here is narrower: refresh-state churn was causing observeStoreIconChanges() to call updateIcons() even when the icon-relevant store signature had not changed. This PR removes that redundant observer work before render-signature checks are reached.

Testing

  • StatusItemIconObservationSignatureTests
    • verifies signature stability across isRefreshing churn
    • verifies signature stability across status description/timestamp churn when the status indicator is unchanged
    • verifies signature changes when the status indicator changes
  • swiftformat
  • swiftlint --strict
  • make check

ptstory and others added 2 commits May 21, 2026 23:54
Remove isRefreshing from iconObservationToken and add observer-side
signature guard to skip updateIcons() when icon-relevant state is
unchanged.

The existing render signature inside applyIcon() avoided redundant
icon rendering, but not redundant observer work. This change reduces
observer callback churn before render-signature checking is reached.

Before: updateIcons() called 10 times per refresh cycle (1 rendered)
After: updateIcons() called 6 times per refresh cycle (1 rendered)

Split from steipete#1073. Related: steipete#678.
@steipete steipete force-pushed the perf/icon-observer-signature branch from 274c47c to c942861 Compare May 21, 2026 22:56
Copy link
Copy Markdown
Owner

@steipete steipete left a comment

Choose a reason for hiding this comment

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

Reviewed the icon observer suppression path, ran focused tests, lint/check, Codex autoreview, and a release bundle relaunch smoke. Added the maintainer changelog entry and rebased onto current main.

@steipete steipete merged commit d5bf269 into steipete:main May 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants