Skip to content

fix(macos): redraw the menu-bar label only when what it shows changes - #2889

Open
msff wants to merge 1 commit into
ryanbr:mainfrom
msff:fix/menubar-label-update-storm
Open

msff wants to merge 1 commit into
ryanbr:mainfrom
msff:fix/menubar-label-update-storm

Conversation

@msff

@msff msff commented Nov 1, 2026

Copy link
Copy Markdown

What this PR does

The macOS menu-bar item now redraws only when the heart rate or link state it shows changes, at most once a second, instead of on every published change of the app's model. Before this, NOOP used about a quarter of a CPU core, plus about 15% more WindowServer, for as long as a strap was streaming.

A 5 s sample had the status item's repaint path at the top of the main thread:

-[NSStatusItem _updateReplicants]
  -[NSStatusItem _redrawReplicantSnapshot:sourceView:]
    CALayer renderInContext

Cause. MenuBarExtra renders its label into the status item's image and title in two cases, whether or not the drawing changed:

  • on every update of its scene;
  • on every change the label observes.

Each render makes macOS repaint a snapshot ("replicant") of the item for every menu bar it shows in. On one display, macOS 27.2 keeps 8 replicants of the item, 4 light and 4 dark. Each repaint sets the source button's appearance, and SwiftUI's MenuBarExtraController answers each appearance change with another render.

The label observed AppModel, LiveState and Repository whole, and StrandApp observed AppModel through @StateObject. Between them those objects publish more than a hundred properties, dozens of times a second while a strap streams. Every one of those changes paid the repaint cost.

A minimal standalone MenuBarExtra app reproduces the mechanism. In it, any publish from an App-level @StateObject re-renders the label, even when the label reads nothing from that object.

Fix.

  • MenuBarLabel observes one Equatable value, MenuBarReading: the displayed heart rate and the link state. MenuBarReadingSource publishes a new reading at most once a second, keeps the latest reading, and skips any reading equal to the one on screen (MenuBarReading.drawn).
  • StrandApp holds AppModel through AppModelOwner, which never publishes. That keeps @StateObject's build-once guarantee without re-evaluating the scenes on every model change. The scene body reads only the model's subsystems, which are lets, so observing the model gave the scenes nothing.
  • The popover's heart rate uses the same rule (MenuBarReading.displayHR), so the label and popover show the same number; the label can trail the popover by up to a second.

What the label shows and how it is tinted are unchanged.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation
  • CI / tooling

How it was tested

  • Repro, before and after (standalone swiftc app, MenuBarExtra label driven at 20 publishes/s, top -pid over 60 s on a MacBook Air M2):
    • old shape: 31% CPU
    • new shape: 2% CPU
  • StrandTests/MenuBarReadingTests.swift, 7 tests:
    • the display rule: median, then reported rate, then last R-R
    • the zone boundaries
    • an unchanged reading is not redrawn
    • a burst is drawn at most twice and ends on the latest reading
    • a reading that returns within an interval is not redrawn
    • a steady stream keeps being drawn while it runs
    • a link-only change is drawn
  • Mutants, each caught by at least one test:
    • no removeDuplicates
    • no throttle
    • debounce in place of throttle
    • latest: false
    • the median not preferred
  • Not verified locally:
    • The test Mac has the Command Line Tools but no Xcode, so the tests ran through a local XCTest shim.
    • The label, source and owner were type-checked against stubs at the macOS 13 deployment target.
    • The app target itself was not built here; this relies on app-build.yml.
    • The app has not been re-measured with this change installed.

Known limits

  • Chart-style switch. The dot's tint reads StrandPalette.chartStyle, a global that the main window's .chartStyle(_:) modifier sets as it renders. The label no longer re-renders on every published change. So after a Chart style switch, a label whose reading stays the same, such as linked with no heart rate yet, can keep the old style's colour until its reading next changes. While a heart rate streams, the reading changes within seconds.
  • The popover. While the popover is open, MenuBarContent still observes AppModel, LiveState and Repository whole. It lives in its own window and is only on screen while open; this was not re-sampled.

Unapplied review findings

  • P3 — Strand/MenuBar/MenuBarContent.swift:84 — the popover's heart rate can run up to a second ahead of the throttled label.
    Suggested fix: build MenuBarReadingSource once in AppModelOwner, pass it to MenuBarLabel, inject it into MenuBarContent, and read the popover's number from it. The popover's number would then move at most once a second.
  • Testing gap — MenuBarReadingSource's wiring has no test of its own. A change that dropped model.$bpm or live.$connected from its inputs would pass every test here. No StrandTests test builds an AppModel, because its init starts BLEManager and Repository, so such a test needs a seam first.

From a local review (correctness plus one adversarial reader); run 20261101-220955-5e314cb3.

Checklist

  • Swift package tests pass for any package I touched — no package touched
  • Android unit tests pass if I touched android/ — not touched: Android has no menu-bar extra
  • No new build warnings introduced — not checked: the app target was not built locally (no Xcode)
  • UI changes use only StrandDesign tokens — no hardcoded colors, fonts, or spacing
  • No hardcoded hex frame bytes; protocol facts live in the schema / decoders
  • Follows the conventions in docs/CONTRIBUTING.md
  • I did not commit generated output (Strand.xcodeproj/) or any secrets/keystores

Related issues

Refs #2393. That issue covers a neighbouring idle cost, the main window's Liquid animations rendering while the window is hidden. This PR does not touch them.


Compound Engineering

🤖 Generated with Claude Code

MenuBarExtra renders its label into the status item on every scene
update and on every change the label observes, and each render makes
macOS repaint the item's snapshot in every menu bar. The label observed
AppModel, LiveState and Repository whole, and StrandApp observed AppModel,
so every published change paid that cost: about a quarter of a core plus
15% more WindowServer while a strap streamed.

The label now observes one Equatable MenuBarReading, published at most
once a second and only when it differs from the one on screen. StrandApp
holds AppModel through an owner that never publishes. The popover's
heart rate uses the same display rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DX23876 pushed a commit to DX23876/noop that referenced this pull request Nov 8, 2026
Adopted from ryanbr#2889 (by @msff). The menu-bar label observed
AppModel, LiveState and Repository whole, and the app scene observed
AppModel, so every published change re-rendered the status item, which
macOS repaints in every menu bar. While a strap streamed that cost about
a quarter of a CPU core plus WindowServer time.

The label now observes one MenuBarReading (shown heart rate and link
state), updated at most once a second and only when it changes. The app
scene holds AppModel through an owner that never publishes; its body
reads only the model's subsystems, never its published state.

Fork adaptation: the label no longer carries the fork's
dashboardPresentationScope (it reads no sleep data); the popover keeps
it. The reading is mapped through a closure instead of a function
reference for the fork's strict concurrency checking.

Not compiled here (no Swift toolchain in this environment).

Analysis migration required: no

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AWc14WNvMmSF4RHP6ww4d
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.

1 participant