Skip to content

perf(phase 1): low-risk isolated fixes — leaks, bounded cache, main-actor media, micro-opts #653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mehmetefeaytas wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

perf(phase 1): low-risk isolated fixes — leaks, bounded cache, main-actor media, micro-opts #653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
02b130c
perf(shelf): bound thumbnail cache with NSCache (count + cost limits)
mehmetefeaytas Jul 23, 2026
ecf38cf
perf(audio): fix HAL property-listener leak + reuse volume/mute eleme…
mehmetefeaytas Jul 23, 2026
32799c8
perf(stats): fix observer-id bug, ps throttle, single getifaddrs, laz…
mehmetefeaytas Jul 23, 2026
0571428
fix(media): publish playbackState on the main actor; reuse static ISO…
mehmetefeaytas Jul 23, 2026
16c8620
perf(phase-1): low-risk isolated fixes (HAL leak, thumbnail NSCache, …
mehmetefeaytas Jul 23, 2026
3193e1d
fix(perf-phase-1): address CodeRabbit review
mehmetefeaytas Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions DynamicIsland/MediaControllers/AmazonMusicController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ class FilteredNowPlayingController: ObservableObject, MediaControllerProtocol {
/// True only after a stream line explicitly identified the selected app as the now playing source.
private var targetSessionActive = false

/// Reused across every stream update to avoid re-allocating a formatter
/// on each (1+/second) now playing payload.
private static let iso8601Formatter = ISO8601DateFormatter()

init?(bundleIdentifier: String, controllerName: String) {
self.targetBundleIdentifier = bundleIdentifier
self.controllerName = controllerName
Expand Down Expand Up @@ -136,14 +140,18 @@ class FilteredNowPlayingController: ObservableObject, MediaControllerProtocol {
}

func toggleShuffle() async {
MRMediaRemoteSetShuffleModeFunction(playbackState.isShuffled ? 1 : 3)
playbackState.isShuffled.toggle()
await MainActor.run {
MRMediaRemoteSetShuffleModeFunction(playbackState.isShuffled ? 1 : 3)
playbackState.isShuffled.toggle()
}
}

func toggleRepeat() async {
let newRepeatMode = (playbackState.repeatMode == .off) ? 3 : (playbackState.repeatMode.rawValue - 1)
playbackState.repeatMode = RepeatMode(rawValue: newRepeatMode) ?? .off
MRMediaRemoteSetRepeatModeFunction(newRepeatMode)
await MainActor.run {
let newRepeatMode = (playbackState.repeatMode == .off) ? 3 : (playbackState.repeatMode.rawValue - 1)
playbackState.repeatMode = RepeatMode(rawValue: newRepeatMode) ?? .off
MRMediaRemoteSetRepeatModeFunction(newRepeatMode)
}
}

private func setupNowPlayingObserver() async {
Expand Down Expand Up @@ -214,9 +222,12 @@ class FilteredNowPlayingController: ObservableObject, MediaControllerProtocol {
return state
}

private func applyIdleBecauseDifferentSource() {
private func applyIdleBecauseDifferentSource() async {
targetSessionActive = false
playbackState = Self.makeIdlePlaybackState(bundleIdentifier: targetBundleIdentifier)
let idleState = Self.makeIdlePlaybackState(bundleIdentifier: targetBundleIdentifier)
await MainActor.run { [weak self] in
self?.playbackState = idleState
}
}

private func handleAdapterUpdate(_ update: NowPlayingUpdate) async {
Expand All @@ -233,12 +244,12 @@ class FilteredNowPlayingController: ObservableObject, MediaControllerProtocol {

if let source = explicitSource {
if source != targetBundleIdentifier {
applyIdleBecauseDifferentSource()
await applyIdleBecauseDifferentSource()
return
}
targetSessionActive = true
} else if !diff {
applyIdleBecauseDifferentSource()
await applyIdleBecauseDifferentSource()
return
} else if !targetSessionActive {
return
Expand Down Expand Up @@ -276,7 +287,7 @@ class FilteredNowPlayingController: ObservableObject, MediaControllerProtocol {
}

if let dateString = payload.timestamp,
let date = ISO8601DateFormatter().date(from: dateString) {
let date = Self.iso8601Formatter.date(from: dateString) {
newPlaybackState.lastUpdated = date
} else if !diff {
newPlaybackState.lastUpdated = Date()
Expand All @@ -288,7 +299,9 @@ class FilteredNowPlayingController: ObservableObject, MediaControllerProtocol {
newPlaybackState.isPlaying = payload.playing ?? (diff ? self.playbackState.isPlaying : false)
newPlaybackState.bundleIdentifier = targetBundleIdentifier

self.playbackState = newPlaybackState
await MainActor.run { [weak self] in
self?.playbackState = newPlaybackState
}
Comment on lines +302 to +304

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Construct and publish each state snapshot in the same main-actor transaction.

These closures only publish a snapshot; its baseline was read and merged beforehand off-actor. A command/WebSocket/artwork update may modify one field on the main actor before an older full snapshot arrives and overwrites it. Build the next state from self.playbackState inside the same MainActor.run block, or main-actor-isolate the update method.

  • DynamicIsland/MediaControllers/AmazonMusicController.swift#L302-L304: merge the adapter payload from an actor-isolated baseline.
  • DynamicIsland/MediaControllers/NowPlayingController.swift#L270-L272: merge the stream payload from an actor-isolated baseline.
  • DynamicIsland/MediaControllers/AppleMusicController.swift#L165-L168: derive updatedState from an actor-isolated baseline.
  • DynamicIsland/MediaControllers/SpotifyController.swift#L199-L202: derive and publish resolvedState atomically with respect to other state updates.
  • DynamicIsland/MediaControllers/YouTube Music Controller/YouTubeMusicController.swift#L448-L451: prevent full polling/WebSocket responses from reverting incremental updates.
📍 Affects 5 files
  • DynamicIsland/MediaControllers/AmazonMusicController.swift#L302-L304 (this comment)
  • DynamicIsland/MediaControllers/NowPlayingController.swift#L270-L272
  • DynamicIsland/MediaControllers/AppleMusicController.swift#L165-L168
  • DynamicIsland/MediaControllers/SpotifyController.swift#L199-L202
  • DynamicIsland/MediaControllers/YouTube Music Controller/YouTubeMusicController.swift#L448-L451
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@DynamicIsland/MediaControllers/AmazonMusicController.swift` around lines 302
- 304, Construct and publish each media state snapshot atomically on the main
actor by reading the current playback state and merging the incoming payload
inside the same update transaction. Apply this to AmazonMusicController.swift
(302-304), NowPlayingController.swift (270-272), AppleMusicController.swift
(165-168), SpotifyController.swift (199-202), and YouTubeMusicController.swift
(448-451); preserve incremental updates while preventing stale full responses
from overwriting newer fields.

}
}

Expand Down
5 changes: 4 additions & 1 deletion DynamicIsland/MediaControllers/AppleMusicController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ class AppleMusicController: MediaControllerProtocol {
}

updatedState.lastUpdated = Date()
self.playbackState = updatedState
let finalState = updatedState
await MainActor.run { [weak self] in
self?.playbackState = finalState
}
}

// MARK: - Private Methods
Expand Down
28 changes: 19 additions & 9 deletions DynamicIsland/MediaControllers/NowPlayingController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ final class NowPlayingController: ObservableObject, MediaControllerProtocol {
private var lastMusicItem:
(title: String, artist: String, album: String, duration: TimeInterval, artworkData: Data?)?

/// Reused across every stream update to avoid re-allocating a formatter
/// on each (1+/second) now playing payload.
private static let iso8601Formatter = ISO8601DateFormatter()

// MARK: - Media Remote Functions
private let mediaRemoteBundle: CFBundle
private let MRMediaRemoteSendCommandFunction: @convention(c) (Int, AnyObject?) -> Void
Expand Down Expand Up @@ -134,15 +138,19 @@ final class NowPlayingController: ObservableObject, MediaControllerProtocol {

func toggleShuffle() async {
// MRMediaRemoteSendCommandFunction(6, nil)
MRMediaRemoteSetShuffleModeFunction(playbackState.isShuffled ? 1 : 3)
playbackState.isShuffled.toggle()
await MainActor.run {
MRMediaRemoteSetShuffleModeFunction(playbackState.isShuffled ? 1 : 3)
playbackState.isShuffled.toggle()
}
}

func toggleRepeat() async {
// MRMediaRemoteSendCommandFunction(7, nil)
let newRepeatMode = (playbackState.repeatMode == .off) ? 3 : (playbackState.repeatMode.rawValue - 1)
playbackState.repeatMode = RepeatMode(rawValue: newRepeatMode) ?? .off
MRMediaRemoteSetRepeatModeFunction(newRepeatMode)
await MainActor.run {
let newRepeatMode = (playbackState.repeatMode == .off) ? 3 : (playbackState.repeatMode.rawValue - 1)
playbackState.repeatMode = RepeatMode(rawValue: newRepeatMode) ?? .off
MRMediaRemoteSetRepeatModeFunction(newRepeatMode)
}
}

// MARK: - Setup Methods
Expand Down Expand Up @@ -243,7 +251,7 @@ final class NowPlayingController: ObservableObject, MediaControllerProtocol {
}

if let dateString = payload.timestamp,
let date = ISO8601DateFormatter().date(from: dateString) {
let date = Self.iso8601Formatter.date(from: dateString) {
newPlaybackState.lastUpdated = date
} else if !diff {
newPlaybackState.lastUpdated = Date()
Expand All @@ -258,8 +266,10 @@ final class NowPlayingController: ObservableObject, MediaControllerProtocol {
payload.bundleIdentifier ??
(diff ? self.playbackState.bundleIdentifier : "")
)

self.playbackState = newPlaybackState

await MainActor.run { [weak self] in
self?.playbackState = newPlaybackState
}
}
}

Expand Down
8 changes: 6 additions & 2 deletions DynamicIsland/MediaControllers/SpotifyController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class SpotifyController: MediaControllerProtocol {
self?.artistMetadataFetchTask?.cancel()
self?.artistMetadataFetchTask = nil
self?.currentArtistTrackURI = nil
if let self {
Task { @MainActor [weak self] in
guard let self else { return }
var updatedState = self.playbackState
updatedState.liveArtworkURL = nil
self.playbackState = updatedState
Expand Down Expand Up @@ -195,7 +196,10 @@ class SpotifyController: MediaControllerProtocol {
artistMetadataFetchTask = nil
}

playbackState = state
let resolvedState = state
await MainActor.run { [weak self] in
self?.playbackState = resolvedState
}

if !trackURI.isEmpty {
if cachedArtistResult == nil {
Expand Down
101 changes: 70 additions & 31 deletions DynamicIsland/MediaControllers/YouTube Music Controller/YouTubeMusicController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ final class YouTubeMusicController: MediaControllerProtocol {

func updatePlaybackInfo() async {
guard isActive() else {
resetPlaybackState()
await MainActor.run { [weak self] in
self?.resetPlaybackState()
}
return
}

Expand Down Expand Up @@ -161,8 +163,10 @@ final class YouTubeMusicController: MediaControllerProtocol {
await webSocketClient?.disconnect()
webSocketClient = nil
}

resetPlaybackState()

await MainActor.run { [weak self] in
self?.resetPlaybackState()
}
}

private func initializeIfAppActive() async {
Expand Down Expand Up @@ -237,33 +241,49 @@ final class YouTubeMusicController: MediaControllerProtocol {
}
guard let newPosition = position else { return }

var copied = playbackState
copied.currentTime = newPosition
copied.lastUpdated = Date()
playbackState = copied
await MainActor.run { [weak self] in
guard let self else { return }
var copied = self.playbackState
copied.currentTime = newPosition
copied.lastUpdated = Date()
self.playbackState = copied
}

case .repeatChanged:
guard let data = message.extractData() else { return }
var copy = playbackState

var newRepeatMode: RepeatMode? = nil
if let repeatStr = data["repeat"] as? String {
switch repeatStr.uppercased() {
case "NONE": copy.repeatMode = .off
case "ALL": copy.repeatMode = .all
case "ONE": copy.repeatMode = .one
case "NONE": newRepeatMode = .off
case "ALL": newRepeatMode = .all
case "ONE": newRepeatMode = .one
default: break
}
}
copy.lastUpdated = Date()
playbackState = copy

await MainActor.run { [weak self] in
guard let self else { return }
var copy = self.playbackState
if let newRepeatMode { copy.repeatMode = newRepeatMode }
copy.lastUpdated = Date()
self.playbackState = copy
}

case .shuffleChanged:
guard let data = message.extractData() else { return }
var copy = playbackState
if let shuffle = data["shuffle"] as? Bool { copy.isShuffled = shuffle }
else if let shuffle = data["isShuffled"] as? Bool { copy.isShuffled = shuffle }
copy.lastUpdated = Date()
playbackState = copy

var newShuffle: Bool? = nil
if let shuffle = data["shuffle"] as? Bool { newShuffle = shuffle }
else if let shuffle = data["isShuffled"] as? Bool { newShuffle = shuffle }

await MainActor.run { [weak self] in
guard let self else { return }
var copy = self.playbackState
if let newShuffle { copy.isShuffled = newShuffle }
copy.lastUpdated = Date()
self.playbackState = copy
}

case .volumeChanged:
break
Expand Down Expand Up @@ -339,24 +359,40 @@ final class YouTubeMusicController: MediaControllerProtocol {
)
// Lightweight endpoint-specific parsing
if endpoint == "/shuffle" {
if let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], let shuffleState = json["state"] as? Bool {
playbackState.isShuffled = shuffleState
} else {
playbackState.isShuffled = !playbackState.isShuffled
var shuffleState: Bool? = nil
if let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], let state = json["state"] as? Bool {
shuffleState = state
}
await MainActor.run { [weak self] in
guard let self else { return }
if let shuffleState {
self.playbackState.isShuffled = shuffleState
} else {
self.playbackState.isShuffled = !self.playbackState.isShuffled
}
}
} else if endpoint == "/repeat-mode" {
var mode: String? = nil
if let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any] {
if let mode = json["mode"] as? String { updateRepeatMode(mode) }
mode = json["mode"] as? String
}
if let mode {
await MainActor.run { [weak self] in
self?.updateRepeatMode(mode)
}
}
} else if endpoint == "/switch-repeat" {
// Find next repeat mode
let nextMode: RepeatMode
switch playbackState.repeatMode {
case .off: nextMode = .all
case .all: nextMode = .one
case .one: nextMode = .off
await MainActor.run { [weak self] in
guard let self else { return }
// Find next repeat mode
let nextMode: RepeatMode
switch self.playbackState.repeatMode {
case .off: nextMode = .all
case .all: nextMode = .one
case .one: nextMode = .off
}
self.playbackState.repeatMode = nextMode
}
playbackState.repeatMode = nextMode
} else if refresh && webSocketClient == nil {
try? await Task.sleep(for: .milliseconds(100))
await updatePlaybackInfo()
Expand Down Expand Up @@ -409,7 +445,10 @@ final class YouTubeMusicController: MediaControllerProtocol {
}

// Always update - removed comparison since PlaybackState doesn't conform to Equatable
playbackState = newState
let resolvedState = newState
await MainActor.run { [weak self] in
self?.playbackState = resolvedState
}

artworkFetchTask?.cancel()
artworkFetchTask = nil
Expand Down
Loading