From 8192441f607374ddcd9f24d0bdf3cc839bad2d0e Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:22:45 +0700 Subject: [PATCH] perf(downloads): check album ReplayGain readiness in one pass --- .../album_replaygain_readiness_test.dart | 76 +++++++ lib/providers/download_queue_provider.dart | 1 + .../download_queue_provider_replaygain.dart | 61 +----- lib/services/album_replaygain_readiness.dart | 43 ++++ test/album_replaygain_readiness_test.dart | 191 ++++++++++++++++++ test/support/album_replaygain_benchmark.dart | 90 +++++++++ 6 files changed, 410 insertions(+), 52 deletions(-) create mode 100644 integration_test/album_replaygain_readiness_test.dart create mode 100644 lib/services/album_replaygain_readiness.dart create mode 100644 test/album_replaygain_readiness_test.dart create mode 100644 test/support/album_replaygain_benchmark.dart diff --git a/integration_test/album_replaygain_readiness_test.dart b/integration_test/album_replaygain_readiness_test.dart new file mode 100644 index 00000000..cbc56272 --- /dev/null +++ b/integration_test/album_replaygain_readiness_test.dart @@ -0,0 +1,76 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:integration_test/integration_test.dart'; +import 'package:spotiflac_android/services/album_replaygain_readiness.dart'; + +import '../test/support/album_replaygain_benchmark.dart'; + +void main() { + final binding = IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + final report = { + 'platform': Platform.operatingSystem, + 'note': + 'Five alternating old/new pairs of the queue-readiness decision used by album ReplayGain retriggering. Fixtures have ten tracks per album and mixed blocked/ready/absent albums. Excludes audio scanning and tag I/O; not whole-download speed or FPS. iOS debug is correctness only.', + }; + var completed = 0; + tearDownAll(() { + report['completed_cases'] = completed; + binding.reportData ??= {}; + binding.reportData!['album_replaygain'] = report; + }); + for (final count in [1000, 10000]) { + testWidgets('album readiness for $count queue items', (tester) async { + final fixture = await albumReplayGainFixture(count); + final expected = [ + for (var album = 0; album < count ~/ 10; album++) + if (album % 3 == 2) + legacyAlbumReplayGainKey(fixture.items[album * 10].track), + 'id:absent-multiple', + ]; + final samples = >{'per_album': [], 'one_scan': []}; + for (var sample = 0; sample < 5; sample++) { + for (final bulk in sample.isEven ? [false, true] : [true, false]) { + final watch = Stopwatch()..start(); + final ready = bulk + ? readyReplayGainAlbums(fixture.items, fixture.entries) + : legacyReadyReplayGainAlbums(fixture.items, fixture.entries); + watch.stop(); + samples[bulk ? 'one_scan' : 'per_album']!.add( + watch.elapsedMicroseconds, + ); + expect(ready, expected); + } + } + report['items_$count'] = samples; + completed++; + }); + } + testWidgets('single-album checks preserve early, late and absent decisions', ( + tester, + ) async { + final fixture = await albumReplayGainFixture(10000); + for (final (label, key, expected) in [ + ('first', legacyAlbumReplayGainKey(fixture.items.first.track), true), + ('last', legacyAlbumReplayGainKey(fixture.items.last.track), true), + ('absent', 'id:absent', false), + ]) { + final samples = >{'full_list': [], 'short_circuit': []}; + for (var sample = 0; sample < 5; sample++) { + for (final optimized in sample.isEven ? [false, true] : [true, false]) { + final watch = Stopwatch()..start(); + final blocked = optimized + ? albumReplayGainBlocked(fixture.items, key) + : legacyAlbumReplayGainBlocked(fixture.items, key); + watch.stop(); + samples[optimized ? 'short_circuit' : 'full_list']!.add( + watch.elapsedMicroseconds, + ); + expect(blocked, expected); + } + } + report['single_$label'] = samples; + } + completed++; + }); +} diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index ce57254a..91f83b4e 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -34,6 +34,7 @@ import 'package:spotiflac_android/services/download_motion_artwork_source.dart'; import 'package:spotiflac_android/services/ffmpeg_service.dart'; import 'package:spotiflac_android/services/hires_check_service.dart'; import 'package:spotiflac_android/services/replaygain_service.dart'; +import 'package:spotiflac_android/services/album_replaygain_readiness.dart'; import 'package:spotiflac_android/services/notification_service.dart'; import 'package:spotiflac_android/services/verification_notification.dart'; import 'package:spotiflac_android/utils/logger.dart' hide log; diff --git a/lib/providers/download_queue_provider_replaygain.dart b/lib/providers/download_queue_provider_replaygain.dart index ab731828..330eb5b6 100644 --- a/lib/providers/download_queue_provider_replaygain.dart +++ b/lib/providers/download_queue_provider_replaygain.dart @@ -22,12 +22,7 @@ class _AlbumRgAccumulator { } extension _DownloadQueueReplayGain on DownloadQueueNotifier { - String _albumRgKey(Track track) { - if (track.albumId != null && track.albumId!.isNotEmpty) { - return 'id:${track.albumId}'; - } - return 'name:${track.albumName}|${track.albumArtist ?? ''}'; - } + String _albumRgKey(Track track) => albumReplayGainKey(track); /// Purge a track's stale ReplayGain accumulator entry, dropping the whole /// album accumulator once it becomes empty. @@ -93,31 +88,7 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { final accumulator = _albumRgData[key]; if (accumulator == null || accumulator.entries.isEmpty) return; - // Find queue items for this album that are STILL in the queue. - // Completed tracks may have already been removed by removeItem(), so - // their absence means they finished successfully (not that they're - // still pending). - final albumItemsInQueue = state.items - .where((item) => _albumRgKey(item.track) == key) - .toList(); - - final pending = albumItemsInQueue.where( - (item) => - item.status == DownloadStatus.queued || - item.status == DownloadStatus.downloading || - item.status == DownloadStatus.finalizing, - ); - if (pending.isNotEmpty) return; - - // If any item is failed/skipped, the user might retry it later. - // Don't finalize album RG with partial data — wait until all album - // tracks are either completed (and possibly removed) or retried. - final retryable = albumItemsInQueue.where( - (item) => - item.status == DownloadStatus.failed || - item.status == DownloadStatus.skipped, - ); - if (retryable.isNotEmpty) return; + if (albumReplayGainBlocked(state.items, key)) return; // The accumulator entries represent successfully scanned tracks. Entries // are only added after a successful ReplayGain scan, removed on retry or @@ -151,30 +122,16 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { final settings = ref.read(settingsProvider); if (!settings.embedReplayGain) return; - // Snapshot the keys — _checkAndWriteAlbumReplayGain may mutate the map. - final keys = _albumRgData.keys.toList(); + // Decide readiness once before starting writes, which may remove completed + // accumulators. Avoid scanning the entire queue again for every album. + final keys = readyReplayGainAlbums(state.items, { + for (final entry in _albumRgData.entries) + entry.key: entry.value.entries.length, + }); for (final key in keys) { final acc = _albumRgData[key]; if (acc == null || acc.entries.isEmpty) continue; - // Use the first entry's trackId to find a representative track. - // _checkAndWriteAlbumReplayGain only needs it for _albumRgKey(), so any - // track from the album works. - final albumItems = state.items - .where((item) => _albumRgKey(item.track) == key) - .toList(); - // If there are no items left in queue for this album but we have - // accumulator data, all items were completed and removed. Use a - // synthetic call — we need a Track to call the check, but the items - // are gone. For this case, directly check conditions inline. - if (albumItems.isEmpty) { - // All items removed → no pending/retryable. Trigger computation. - if (acc.entries.length > 1) { - _computeAndWriteAlbumRg(key, acc); - } - continue; - } - final representative = albumItems.first; - _checkAndWriteAlbumReplayGain(representative.track); + _computeAndWriteAlbumRg(key, acc); } } diff --git a/lib/services/album_replaygain_readiness.dart b/lib/services/album_replaygain_readiness.dart new file mode 100644 index 00000000..a169f4db --- /dev/null +++ b/lib/services/album_replaygain_readiness.dart @@ -0,0 +1,43 @@ +import 'package:spotiflac_android/models/download_item.dart'; +import 'package:spotiflac_android/models/track.dart'; + +String albumReplayGainKey(Track track) { + if (track.albumId != null && track.albumId!.isNotEmpty) { + return 'id:${track.albumId}'; + } + return 'name:${track.albumName}|${track.albumArtist ?? ''}'; +} + +/// Failed/skipped requests may still be retried; every non-completed member +/// blocks writing album gain. Completed members may already have been removed. +bool albumReplayGainBlocked(Iterable items, String albumKey) => + items.any( + (item) => + item.status != DownloadStatus.completed && + albumReplayGainKey(item.track) == albumKey, + ); + +/// Keys whose accumulated scans can be finalized, in accumulator order. +/// A present single-entry album is finalized to clear its accumulator; an +/// absent single-entry album is retained, matching the queue's existing policy. +List readyReplayGainAlbums( + Iterable items, + Map entryCounts, +) { + if (entryCounts.isEmpty) return const []; + final present = {}; + final blocked = {}; + for (final item in items) { + final key = albumReplayGainKey(item.track); + if ((entryCounts[key] ?? 0) == 0) continue; + present.add(key); + if (item.status != DownloadStatus.completed) blocked.add(key); + } + return [ + for (final entry in entryCounts.entries) + if (entry.value > 0 && + !blocked.contains(entry.key) && + (entry.value > 1 || present.contains(entry.key))) + entry.key, + ]; +} diff --git a/test/album_replaygain_readiness_test.dart b/test/album_replaygain_readiness_test.dart new file mode 100644 index 00000000..de4dcb75 --- /dev/null +++ b/test/album_replaygain_readiness_test.dart @@ -0,0 +1,191 @@ +import 'dart:math'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/models/download_item.dart'; +import 'package:spotiflac_android/models/track.dart'; +import 'package:spotiflac_android/services/album_replaygain_readiness.dart'; + +import 'support/album_replaygain_benchmark.dart'; + +DownloadItem _item(String album, DownloadStatus status, {String? albumId}) => + DownloadItem( + id: '$album-${status.name}', + track: Track( + id: 'shared-track-id', + name: 'Song', + artistName: 'Artist & Guest', + albumName: album, + albumId: albumId, + albumArtist: 'Artist', + duration: 180, + ), + service: 'example', + createdAt: DateTime.utc(2026), + status: status, + ); + +void main() { + test('album identity keeps ID precedence and literal case/whitespace', () { + final track = _item('Album', DownloadStatus.completed).track; + expect(albumReplayGainKey(track), 'name:Album|Artist'); + expect( + albumReplayGainKey(track.copyWith(albumId: '')), + 'name:Album|Artist', + ); + expect(albumReplayGainKey(track.copyWith(albumId: ' ID ')), 'id: ID '); + expect( + albumReplayGainKey(track.copyWith(albumName: 'album')), + 'name:album|Artist', + ); + expect( + albumReplayGainKey( + const Track( + id: 'missing-artist', + name: 'Song', + artistName: 'Artist', + albumName: 'Album', + duration: 180, + ), + ), + 'name:Album|', + ); + expect(albumReplayGainKey(track.copyWith(albumArtist: '')), 'name:Album|'); + }); + + for (final status in DownloadStatus.values) { + test('$status blocks only its own album until completed', () { + final target = _item('Target', status); + final items = [ + _item('Other', DownloadStatus.failed), + _item('Target', DownloadStatus.completed), + target, + ]; + final key = albumReplayGainKey(target.track); + expect( + albumReplayGainBlocked(items, key), + status != DownloadStatus.completed, + ); + expect( + readyReplayGainAlbums(items, {key: 2}), + status == DownloadStatus.completed ? [key] : isEmpty, + ); + expect(albumReplayGainBlocked(items, 'id:absent'), false); + }); + } + + test( + 'absent, empty and single-entry accumulators retain their policy/order', + () { + final items = [ + _item('Single', DownloadStatus.completed), + _item('Multiple', DownloadStatus.completed), + _item('Empty', DownloadStatus.completed), + ]; + const entries = { + 'id:absent-multiple': 2, + 'name:Single|Artist': 1, + 'id:absent-single': 1, + 'name:Empty|Artist': 0, + 'name:Multiple|Artist': 2, + }; + expect(readyReplayGainAlbums(items, entries), [ + 'id:absent-multiple', + 'name:Single|Artist', + 'name:Multiple|Artist', + ]); + expect(readyReplayGainAlbums([], entries), [ + 'id:absent-multiple', + 'name:Multiple|Artist', + ]); + }, + ); + + test( + 'retry, removal and new snapshots change readiness without stale cache', + () { + final complete = _item('Album', DownloadStatus.completed); + final failed = _item('Album', DownloadStatus.failed); + final key = albumReplayGainKey(complete.track); + final original = [complete, failed]; + expect(readyReplayGainAlbums(original, {key: 2}), isEmpty); + expect(readyReplayGainAlbums([complete], {key: 2}), [key]); + expect( + readyReplayGainAlbums( + [complete, failed.copyWith(status: DownloadStatus.queued)], + {key: 2}, + ), + isEmpty, + ); + expect( + readyReplayGainAlbums( + [complete, failed.copyWith(status: DownloadStatus.completed)], + {key: 2}, + ), + [key], + ); + expect(readyReplayGainAlbums(original, {key: 2}), isEmpty); + }, + ); + + test( + 'empty accumulator set and first blocker avoid unnecessary iteration', + () { + Iterable neverRead() sync* { + throw StateError('No scan should be needed'); + } + + expect(readyReplayGainAlbums(neverRead(), {}), isEmpty); + final first = _item('Album', DownloadStatus.queued); + Iterable firstOnly() sync* { + yield first; + throw StateError('Already blocked'); + } + + expect( + albumReplayGainBlocked(firstOnly(), albumReplayGainKey(first.track)), + true, + ); + }, + ); + + test('2000 mixed snapshots match both original decisions exactly', () { + final random = Random(4817); + for (var iteration = 0; iteration < 2000; iteration++) { + final albums = [ + for (var i = 0; i < 15; i++) + _item( + 'Album ${i % 7}', + DownloadStatus.completed, + albumId: i.isEven ? (i % 4 == 0 ? '$i' : ' $i ') : null, + ).track.copyWith(albumArtist: i % 3 == 0 ? '' : 'Artist ${i % 3}'), + ]; + final items = List.generate( + random.nextInt(180), + (index) => DownloadItem( + id: '$index', + track: albums[random.nextInt(albums.length)], + service: 'example', + createdAt: DateTime.utc(2026), + status: DownloadStatus + .values[random.nextInt(DownloadStatus.values.length)], + ), + ); + final entries = { + for (final track in albums) + legacyAlbumReplayGainKey(track): random.nextInt(4), + 'id:absent-single': 1, + 'id:absent-multiple': 2, + }; + expect( + readyReplayGainAlbums(items, entries), + legacyReadyReplayGainAlbums(items, entries), + ); + for (final key in entries.keys) { + expect( + albumReplayGainBlocked(items, key), + legacyAlbumReplayGainBlocked(items, key), + ); + } + } + }); +} diff --git a/test/support/album_replaygain_benchmark.dart b/test/support/album_replaygain_benchmark.dart new file mode 100644 index 00000000..800774b2 --- /dev/null +++ b/test/support/album_replaygain_benchmark.dart @@ -0,0 +1,90 @@ +import 'dart:isolate'; + +import 'package:spotiflac_android/models/download_item.dart'; +import 'package:spotiflac_android/models/track.dart'; +import 'package:spotiflac_android/providers/download_queue_state.dart'; + +String legacyAlbumReplayGainKey(Track track) { + if (track.albumId != null && track.albumId!.isNotEmpty) { + return 'id:${track.albumId}'; + } + return 'name:${track.albumName}|${track.albumArtist ?? ''}'; +} + +bool legacyAlbumReplayGainBlocked(List items, String key) { + final albumItems = items + .where((item) => legacyAlbumReplayGainKey(item.track) == key) + .toList(); + final pending = albumItems.where( + (item) => + item.status == DownloadStatus.queued || + item.status == DownloadStatus.downloading || + item.status == DownloadStatus.finalizing, + ); + if (pending.isNotEmpty) return true; + final retryable = albumItems.where( + (item) => + item.status == DownloadStatus.failed || + item.status == DownloadStatus.skipped, + ); + return retryable.isNotEmpty; +} + +// Original retrigger/check selection. Return each key that reaches the +// computation, including present single-entry albums that are only cleaned up. +List legacyReadyReplayGainAlbums( + List items, + Map entryCounts, +) { + final ready = []; + for (final key in entryCounts.keys.toList()) { + final count = entryCounts[key]!; + if (count == 0) continue; + final albumItems = items + .where((item) => legacyAlbumReplayGainKey(item.track) == key) + .toList(); + if (albumItems.isEmpty) { + if (count > 1) ready.add(key); + continue; + } + if (!legacyAlbumReplayGainBlocked(items, key)) ready.add(key); + } + return ready; +} + +Future<({List items, Map entries})> +albumReplayGainFixture(int count) => Isolate.run(() { + final items = List.generate(count, (index) { + final album = index ~/ 10; + return DownloadItem( + id: 'item-$index', + track: Track( + id: 'track-$index', + name: 'Track $index — 音楽', + artistName: 'Artist', + albumName: 'Album $album', + albumId: album.isEven ? '$album' : null, + albumArtist: 'Artist', + duration: 180, + ), + service: 'example', + createdAt: DateTime.utc(2026), + status: index % 10 != 9 || album % 3 == 2 + ? DownloadStatus.completed + : album % 3 == 0 + ? DownloadStatus.failed + : DownloadStatus.queued, + ); + }); + final entries = { + for (var i = 0; i < count; i += 10) + legacyAlbumReplayGainKey(items[i].track): 10, + 'id:absent-single': 1, + 'id:absent-multiple': 2, + 'id:empty': 0, + }; + return ( + items: const DownloadQueueState().copyWith(items: items).items, + entries: entries, + ); +});