From a56177745603df0d51aa6cd1452540d7c997eeaf Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sun, 12 Jul 2026 18:56:34 +0200 Subject: [PATCH] perf: reduce state notification fan-out --- lib/profiles/active_profile_provider.dart | 10 ++ lib/profiles/plex_home_service.dart | 4 + lib/providers/download_provider.dart | 73 ++++++------ lib/screens/music/now_playing_screen.dart | 105 ++++++++++-------- .../music/music_playback_service.dart | 17 ++- .../music/music_playback_service_impl.dart | 22 ++-- lib/utils/hierarchical_event_mixin.dart | 13 ++- .../widgets/watch_together_overlay.dart | 11 +- test/profiles/plex_home_service_test.dart | 20 ++++ test/providers/download_provider_test.dart | 46 ++++++++ .../music/music_playback_service_test.dart | 18 ++- 11 files changed, 241 insertions(+), 98 deletions(-) diff --git a/lib/profiles/active_profile_provider.dart b/lib/profiles/active_profile_provider.dart index e75eb35e..ab2ec7ea 100644 --- a/lib/profiles/active_profile_provider.dart +++ b/lib/profiles/active_profile_provider.dart @@ -152,6 +152,7 @@ class ActiveProfileProvider extends ChangeNotifier with DisposableChangeNotifier safeNotifyListeners(); }); _plexHomeSub = _plexHome.stream.listen((cache) { + if (_samePlexHomeUsers(cache, _plexHomeUsers)) return; _plexHomeUsers = cache; _recomputeProfiles(); _resolveActive(); @@ -189,6 +190,15 @@ class ActiveProfileProvider extends ChangeNotifier with DisposableChangeNotifier return true; } + static bool _samePlexHomeUsers(Map> a, Map> b) { + if (a.length != b.length) return false; + for (final entry in a.entries) { + final other = b[entry.key]; + if (other == null || !listEquals(entry.value, other)) return false; + } + return true; + } + void _recomputeProfiles() { _profiles = mergeLocalWithPlexHome( locals: _localProfiles, diff --git a/lib/profiles/plex_home_service.dart b/lib/profiles/plex_home_service.dart index 455b6e48..b3f4aa17 100644 --- a/lib/profiles/plex_home_service.dart +++ b/lib/profiles/plex_home_service.dart @@ -101,6 +101,10 @@ class PlexHomeService { for (final conn in current.whereType()) { final cached = _readCache(conn.id); if (cached == null) continue; + final previous = _byConnection[conn.id]; + if (previous != null && encodePlexHomeUsersCacheJson(previous) == encodePlexHomeUsersCacheJson(cached)) { + continue; + } _byConnection[conn.id] = cached; changed = true; } diff --git a/lib/providers/download_provider.dart b/lib/providers/download_provider.dart index 0d7b5bd5..7bb5dcd7 100644 --- a/lib/providers/download_provider.dart +++ b/lib/providers/download_provider.dart @@ -61,6 +61,7 @@ typedef _MetadataHydrationResult = ({MediaItem? metadata, bool networkFilled, bo /// Provider for managing download state and operations. class DownloadProvider extends ChangeNotifier with DisposableChangeNotifierMixin { + int _batchDeletionDepth = 0; final DownloadManagerService _downloadManager; final AppDatabase _database; final SyncRuleExecutor _syncRuleExecutor; @@ -413,16 +414,15 @@ class DownloadProvider extends ChangeNotifier with DisposableChangeNotifierMixin void _onProgressUpdate(DownloadProgress progress) { appLogger.d('Progress update received: ${progress.globalKey} - ${progress.status} - ${progress.progress}%'); - + final ownedByActiveProfile = _ownsDownloadKey(progress.globalKey); _downloads[progress.globalKey] = progress; - // Sync artwork paths when they are available + // Sync artwork paths when they are available. if (progress.hasArtworkPaths) { _artworkPaths[progress.globalKey] = DownloadedArtwork(thumbPath: progress.thumbPath); } - appLogger.d('Notifying listeners for ${progress.globalKey}'); - safeNotifyListeners(); + if (ownedByActiveProfile) safeNotifyListeners(); } @override @@ -1315,8 +1315,10 @@ class DownloadProvider extends ChangeNotifier with DisposableChangeNotifierMixin } } - /// Delete a downloaded item - Future deleteDownload(String globalKey) async { + /// Delete a downloaded item. + Future deleteDownload(String globalKey) => _deleteDownload(globalKey, notify: true); + + Future _deleteDownload(String globalKey, {required bool notify}) async { try { final meta = _metadata[globalKey]; if (meta != null && @@ -1329,40 +1331,38 @@ class DownloadProvider extends ChangeNotifier with DisposableChangeNotifierMixin final released = await _releaseDownloadForActiveProfile(globalKey); final hasOtherOwners = await _database.hasDownloadOwner(globalKey); if (hasOtherOwners) { - if (meta != null) { + if (notify && meta != null) { DeletionNotifier().notifyDeletedItem(item: meta, isDownloadOnly: true); } - if (released) safeNotifyListeners(); + if (notify && released) safeNotifyListeners(); return; } - // Start deletion (progress will be tracked via stream) await _downloadManager.deleteDownload(globalKey); - - // Remove from local state _downloads.remove(globalKey); _metadata.remove(globalKey); _artworkPaths.remove(globalKey); - // Notify any open screens so they can drop the item from their lists - // immediately instead of waiting for an exit/re-enter. - if (meta != null) { + if (notify && meta != null) { DeletionNotifier().notifyDeletedItem(item: meta, isDownloadOnly: true); } - - safeNotifyListeners(); + if (notify) safeNotifyListeners(); } catch (e) { - // Remove from deletion tracking on error _deletionProgress.remove(globalKey); - safeNotifyListeners(); + if (notify) safeNotifyListeners(); rethrow; } } Future _deleteOwnedContainerDownloads(String globalKey, MediaItem container) async { final descendants = _ownedDescendantEntries(container).toList(); - for (final entry in descendants) { - await deleteDownload(entry.key); + _batchDeletionDepth++; + try { + for (final entry in descendants) { + await _deleteDownload(entry.key, notify: false); + } + } finally { + _batchDeletionDepth--; } DeletionNotifier().notifyDeletedItem(item: container, isDownloadOnly: true); @@ -1383,16 +1383,16 @@ class DownloadProvider extends ChangeNotifier with DisposableChangeNotifierMixin }); } - /// Handle deletion progress updates + /// Handle deletion progress updates. void _onDeletionProgressUpdate(DeletionProgress progress) { if (progress.isComplete) { - // Deletion complete - remove from tracking _deletionProgress.remove(progress.globalKey); } else { - // Update progress _deletionProgress[progress.globalKey] = progress; } - safeNotifyListeners(); + if (_batchDeletionDepth == 0 && _ownsDownloadKey(progress.globalKey)) { + safeNotifyListeners(); + } } /// Get deletion progress for an item @@ -1719,15 +1719,18 @@ class DownloadProvider extends ChangeNotifier with DisposableChangeNotifierMixin Future _loadSyncRules() async { try { - _syncRules.clear(); final profileId = _activeProfileId; - if (profileId == null || profileId.isEmpty) return; + if (profileId == null || profileId.isEmpty) { + _syncRules.clear(); + return; + } await _database.adoptLegacySyncRulesForProfile(profileId); if (_activeProfileId != profileId) return; final rules = await _database.getSyncRules(profileId: profileId); - for (final rule in rules) { - _syncRules[rule.globalKey] = rule; - } + if (_activeProfileId != profileId) return; + _syncRules + ..clear() + ..addEntries(rules.map((rule) => MapEntry(rule.globalKey, rule))); } catch (e) { appLogger.w('Failed to load sync rules', error: e); } @@ -1735,12 +1738,18 @@ class DownloadProvider extends ChangeNotifier with DisposableChangeNotifierMixin Future _loadDownloadOwners() async { try { - _ownedDownloadKeys.clear(); final profileId = _activeProfileId; - if (profileId == null || profileId.isEmpty) return; + if (profileId == null || profileId.isEmpty) { + _ownedDownloadKeys.clear(); + return; + } await _database.adoptLegacyDownloadsForProfile(profileId); if (_activeProfileId != profileId) return; - _ownedDownloadKeys.addAll(await _database.getDownloadOwnerKeysForProfile(profileId)); + final ownedKeys = await _database.getDownloadOwnerKeysForProfile(profileId); + if (_activeProfileId != profileId) return; + _ownedDownloadKeys + ..clear() + ..addAll(ownedKeys); } catch (e) { appLogger.w('Failed to load download ownership', error: e); } diff --git a/lib/screens/music/now_playing_screen.dart b/lib/screens/music/now_playing_screen.dart index e96cfc92..3db197bf 100644 --- a/lib/screens/music/now_playing_screen.dart +++ b/lib/screens/music/now_playing_screen.dart @@ -91,7 +91,6 @@ class _NowPlayingScreenState extends State final FocusNode _lyricsPaneFocusNode = FocusNode(debugLabel: 'now_playing_lyrics_pane'); final GlobalKey _utilityBarKey = GlobalKey(); - bool _overflowFocused = false; bool _poppedForIdle = false; @override @@ -535,35 +534,41 @@ class _NowPlayingScreenState extends State /// sessions. Mono styling matches the seek bar: text-colored active track /// on outline. Widget _buildVolumeCluster(MusicPlaybackService service) { - final tk = tokens(context); - final icon = service.volume <= 0 - ? Symbols.volume_off_rounded - : service.volume < 50 - ? Symbols.volume_down_rounded - : Symbols.volume_up_rounded; - return Row( - mainAxisSize: .min, - children: [ - AppIcon(icon, fill: 1, size: 20, color: tk.textMuted), - SizedBox( - width: 140, - child: SliderTheme( - data: SliderTheme.of(context).copyWith( - trackHeight: 3, - activeTrackColor: tk.text, - inactiveTrackColor: tk.outline, - thumbColor: tk.text, - thumbShape: const RoundSliderThumbShape(enabledThumbRadius: 6), - overlayShape: const RoundSliderOverlayShape(overlayRadius: 12), + return ValueListenableBuilder( + valueListenable: service.volumeListenable, + builder: (context, volume, _) { + final tk = tokens(context); + final icon = volume <= 0 + ? Symbols.volume_off_rounded + : volume < 50 + ? Symbols.volume_down_rounded + : Symbols.volume_up_rounded; + return Row( + mainAxisSize: .min, + children: [ + AppIcon(icon, fill: 1, size: 20, color: tk.textMuted), + SizedBox( + width: 140, + child: SliderTheme( + data: SliderTheme.of(context).copyWith( + trackHeight: 3, + activeTrackColor: tk.text, + inactiveTrackColor: tk.outline, + thumbColor: tk.text, + thumbShape: const RoundSliderThumbShape(enabledThumbRadius: 6), + overlayShape: const RoundSliderOverlayShape(overlayRadius: 12), + ), + child: Slider( + value: volume.clamp(0.0, 100.0), + max: 100, + onChanged: (value) => unawaited(service.setVolume(value, persist: false)), + onChangeEnd: (value) => unawaited(service.setVolume(value)), + ), + ), ), - child: Slider( - value: service.volume.clamp(0.0, 100.0), - max: 100, - onChanged: (value) => unawaited(service.setVolume(value)), - ), - ), - ), - ], + ], + ); + }, ); } @@ -578,26 +583,30 @@ class _NowPlayingScreenState extends State Widget child = button; if (focusable) { - final showFocus = _overflowFocused && InputModeTracker.isKeyboardMode(context); - child = Focus( - focusNode: _overflowFocusNode, - descendantsAreFocusable: false, - onFocusChange: (hasFocus) => setState(() => _overflowFocused = hasFocus), - onKeyEvent: (node, event) { - final backResult = handleBackKeyAction(event, _pop); - if (backResult != KeyEventResult.ignored) return backResult; - return dpadKeyHandler( - onSelect: () => contextMenuKey.currentState?.showContextMenu(context), - onDown: _seekFocusNode.requestFocus, - onUp: () {}, // top of the chain — trap - trapHorizontalEdges: true, - )(node, event); + child = ListenableBuilder( + listenable: _overflowFocusNode, + builder: (context, _) { + final showFocus = _overflowFocusNode.hasFocus && InputModeTracker.isKeyboardMode(context); + return Focus( + focusNode: _overflowFocusNode, + descendantsAreFocusable: false, + onKeyEvent: (node, event) { + final backResult = handleBackKeyAction(event, _pop); + if (backResult != KeyEventResult.ignored) return backResult; + return dpadKeyHandler( + onSelect: () => contextMenuKey.currentState?.showContextMenu(context), + onDown: _seekFocusNode.requestFocus, + onUp: () {}, + trapHorizontalEdges: true, + )(node, event); + }, + child: AnimatedContainer( + duration: FocusTheme.getAnimationDuration(context), + decoration: FocusTheme.textFillFocusDecoration(context, isFocused: showFocus, borderRadius: 20), + child: button, + ), + ); }, - child: AnimatedContainer( - duration: FocusTheme.getAnimationDuration(context), - decoration: FocusTheme.textFillFocusDecoration(context, isFocused: showFocus, borderRadius: 20), - child: button, - ), ); } diff --git a/lib/services/music/music_playback_service.dart b/lib/services/music/music_playback_service.dart index 80f8b3fe..59b41fc1 100644 --- a/lib/services/music/music_playback_service.dart +++ b/lib/services/music/music_playback_service.dart @@ -91,10 +91,11 @@ abstract class MusicPlaybackService extends ChangeNotifier { Future seek(Duration position); - /// Music playback volume, 0–100. Persisted across sessions and applied to - /// every audio player instance; independent of the video player volume. + /// Music playback volume, 0–100. Preview updates are exposed separately so + /// a slider does not notify every service consumer on each drag event. double get volume; - Future setVolume(double volume); + ValueListenable get volumeListenable; + Future setVolume(double volume, {bool persist = true}); void setRepeatMode(MusicRepeatMode mode); void toggleShuffle(); @@ -144,6 +145,7 @@ abstract class MusicPlaybackService extends ChangeNotifier { /// on platforms where it failed to initialize). Keeps every UI consumer /// null-safe without per-call-site feature checks. class StubMusicPlaybackService extends MusicPlaybackService { + final ValueNotifier _volumeNotifier = ValueNotifier(100); @override bool get isAvailable => false; @@ -211,9 +213,11 @@ class StubMusicPlaybackService extends MusicPlaybackService { @override double get volume => 100; + @override + ValueListenable get volumeListenable => _volumeNotifier; @override - Future setVolume(double volume) async {} + Future setVolume(double volume, {bool persist = true}) async {} @override void setRepeatMode(MusicRepeatMode mode) {} @@ -259,4 +263,9 @@ class StubMusicPlaybackService extends MusicPlaybackService { @override Future fetchLyrics(MediaItem track) async => null; + @override + void dispose() { + _volumeNotifier.dispose(); + super.dispose(); + } } diff --git a/lib/services/music/music_playback_service_impl.dart b/lib/services/music/music_playback_service_impl.dart index 92f0762d..50fcc12a 100644 --- a/lib/services/music/music_playback_service_impl.dart +++ b/lib/services/music/music_playback_service_impl.dart @@ -1,5 +1,6 @@ import 'dart:async'; +import 'package:flutter/foundation.dart' show ValueListenable; import 'package:flutter/widgets.dart'; import 'package:os_media_controls/os_media_controls.dart'; @@ -102,6 +103,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO /// (the core is recreated after video claims playback). Falls back to full /// volume when settings aren't bootstrapped (tests). double _volume = SettingsService.instanceOrNull?.read(SettingsService.musicVolume) ?? 100.0; + late final ValueNotifier _volumeNotifier = ValueNotifier(_volume); Player? _player; final List> _playerSubs = []; @@ -842,16 +844,21 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO @override double get volume => _volume; + @override + ValueListenable get volumeListenable => _volumeNotifier; @override - Future setVolume(double volume) async { + Future setVolume(double volume, {bool persist = true}) async { final clamped = volume.clamp(0.0, 100.0); - if (clamped == _volume) return; - _volume = clamped; - notifyListeners(); - final settings = SettingsService.instanceOrNull; - if (settings != null) unawaited(settings.write(SettingsService.musicVolume, clamped)); - await _player?.setVolume(clamped); + if (clamped != _volume) { + _volume = clamped; + _volumeNotifier.value = clamped; + await _player?.setVolume(clamped); + } + if (persist) { + final settings = SettingsService.instanceOrNull; + if (settings != null) await settings.write(SettingsService.musicVolume, clamped); + } } @override @@ -1114,6 +1121,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO } unawaited(_positionController.close()); unawaited(_errorsController.close()); + _volumeNotifier.dispose(); super.dispose(); } } diff --git a/lib/utils/hierarchical_event_mixin.dart b/lib/utils/hierarchical_event_mixin.dart index e4b8f0a8..3d3a47f9 100644 --- a/lib/utils/hierarchical_event_mixin.dart +++ b/lib/utils/hierarchical_event_mixin.dart @@ -28,8 +28,17 @@ mixin HierarchicalEventMixin { this.globalKey == globalKey || parentChain.any((pk) => buildGlobalKey(serverId, pk) == globalKey); /// Check if this event affects any item in a collection. - bool affectsAnyOf(Iterable itemIds) => itemIds.any(affectsItem); + bool affectsAnyOf(Iterable itemIds) { + if (itemIds.contains(itemId)) return true; + return parentChain.any(itemIds.contains); + } /// Check if this event affects any item in a global-key collection. - bool affectsAnyGlobalKey(Iterable globalKeys) => globalKeys.any(affectsGlobalKey); + bool affectsAnyGlobalKey(Iterable globalKeys) { + if (globalKeys.contains(globalKey)) return true; + for (final parentId in parentChain) { + if (globalKeys.contains(buildGlobalKey(serverId, parentId))) return true; + } + return false; + } } diff --git a/lib/watch_together/widgets/watch_together_overlay.dart b/lib/watch_together/widgets/watch_together_overlay.dart index a38ebd36..94ad1582 100644 --- a/lib/watch_together/widgets/watch_together_overlay.dart +++ b/lib/watch_together/widgets/watch_together_overlay.dart @@ -390,12 +390,15 @@ class WaitingForParticipantsIndicator extends StatelessWidget { @override Widget build(BuildContext context) { - return Selector)>( - selector: (_, provider) => (provider.isWaitingForPeers, provider.waitingOnNames), + return Selector( + selector: (_, provider) { + final waiting = provider.isWaitingForPeers; + return (waiting, waiting ? _label(provider.waitingOnNames) : ''); + }, builder: (context, value, child) { - final (isWaiting, names) = value; + final (isWaiting, label) = value; if (!isWaiting) return const SizedBox.shrink(); - return _StatusPill(tvIcon: Symbols.hourglass_empty_rounded, label: _label(names)); + return _StatusPill(tvIcon: Symbols.hourglass_empty_rounded, label: label); }, ); } diff --git a/test/profiles/plex_home_service_test.dart b/test/profiles/plex_home_service_test.dart index cf7097c5..cfa71b10 100644 --- a/test/profiles/plex_home_service_test.dart +++ b/test/profiles/plex_home_service_test.dart @@ -76,6 +76,26 @@ void main() { expect(service.current[acct.id]!.firstWhere((u) => u.admin).uuid, 'admin-uuid'); }); + test('identical refreshes do not emit a second cache snapshot', () async { + service = PlexHomeService( + connections: connections, + profileConnections: profileConnections, + storage: storage, + plexHomeUserFetcher: (_) async => [_user('same-user')], + ); + final acct = _account('plex.same'); + await connections.upsert(acct); + final emissions = >>[]; + final subscription = service.stream.listen(emissions.add); + addTearDown(subscription.cancel); + + await service.refresh(acct); + await service.refresh(acct); + await Future.delayed(Duration.zero); + + expect(emissions, hasLength(2)); + expect(emissions.last[acct.id]!.single.uuid, 'same-user'); + }); test('refresh persists users to SharedPreferences', () async { service = PlexHomeService( connections: connections, diff --git a/test/providers/download_provider_test.dart b/test/providers/download_provider_test.dart index fdecad90..2c1cae7f 100644 --- a/test/providers/download_provider_test.dart +++ b/test/providers/download_provider_test.dart @@ -630,6 +630,52 @@ void main() { p.dispose(); }); + test('deleting an album emits one provider notification for all tracks', () async { + MediaItem track(String id) => testMediaItem( + id: id, + backend: MediaBackend.plex, + kind: MediaKind.track, + title: id, + parentId: 'album-1', + serverId: ServerId('srv'), + ); + final album = testMediaItem( + id: 'album-1', + backend: MediaBackend.plex, + kind: MediaKind.album, + title: 'Album', + serverId: ServerId('srv'), + ); + for (final id in ['t1', 't2']) { + await db.insertDownload( + serverId: ServerId('srv'), + ratingKey: id, + globalKey: 'srv:$id', + type: 'track', + status: DownloadStatus.completed.index, + ); + await db.addDownloadOwner(profileId: 'test-profile', globalKey: 'srv:$id'); + } + final p = DownloadProvider.forTesting(downloadManager: downloadManager, database: db); + await p.ensureInitialized(); + p.debugSeedState( + downloads: { + 'srv:t1': const DownloadProgress(globalKey: 'srv:t1', status: DownloadStatus.completed), + 'srv:t2': const DownloadProgress(globalKey: 'srv:t2', status: DownloadStatus.completed), + }, + metadata: {'srv:album-1': album, 'srv:t1': track('t1'), 'srv:t2': track('t2')}, + ownedDownloadKeys: {'srv:t1', 'srv:t2'}, + ); + var notifications = 0; + p.addListener(() => notifications++); + + await p.deleteDownload(album.globalKey); + + expect(notifications, 1); + expect(p.downloads, isEmpty); + p.dispose(); + }); + test('album aggregates, downloadedAlbums, and per-album track order come from track downloads', () async { MediaItem track(String id, {required int disc, required int number}) => testMediaItem( id: id, diff --git a/test/services/music/music_playback_service_test.dart b/test/services/music/music_playback_service_test.dart index e4f7ddb7..41f4c832 100644 --- a/test/services/music/music_playback_service_test.dart +++ b/test/services/music/music_playback_service_test.dart @@ -88,6 +88,7 @@ class FakePlayer implements Player { final List openedUris = []; final List setNextCalls = []; final List seeks = []; + final List volumes = []; /// Arming these URIs throws, simulating a native setNext failure. final Set failingSetNextUris = {}; @@ -249,7 +250,7 @@ class FakePlayer implements Player { Future addSubtitleTrack({required String uri, String? title, String? language, bool select = false}) async {} @override - Future setVolume(double volume) async {} + Future setVolume(double volume) async => volumes.add(volume); @override Future setRate(double rate) async {} @@ -553,6 +554,21 @@ void main() { h.controls.closeControllers(); }); + test('volume updates notify only the dedicated volume listenable', () async { + await h.playTracks([t1]); + var serviceNotifications = 0; + var volumeNotifications = 0; + h.service.addListener(() => serviceNotifications++); + h.service.volumeListenable.addListener(() => volumeNotifications++); + + await h.service.setVolume(42, persist: false); + + expect(h.service.volume, 42); + expect(h.player.volumes, [42]); + expect(volumeNotifications, 1); + expect(serviceNotifications, 0); + }); + test('playFromList opens the first track and arms the second', () async { await h.playTracks([t1, t2, t3]);