From 48ba08976ad2292b2a6bb2ffe4f097448dd78d9b Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sun, 20 Sep 2026 17:21:11 +0700 Subject: [PATCH] perf(bridge): reduce metadata copies and decode large JSON off the UI isolate Keep decoded cache objects private and copy only at public return boundaries. Decode large metadata and persisted JSON in a worker, and read spill files in the decoding isolate with cleanup on failure. Protect replacement in-flight requests from stale cleanup. Cover nested mutation isolation, coalescing, invalidation, restoration, spill cleanup and retries. --- lib/services/platform_bridge.dart | 55 +++-- test/platform_bridge_custom_search_test.dart | 3 + test/platform_bridge_lookup_cache_test.dart | 206 +++++++++++++++++++ 3 files changed, 248 insertions(+), 16 deletions(-) create mode 100644 test/platform_bridge_lookup_cache_test.dart diff --git a/lib/services/platform_bridge.dart b/lib/services/platform_bridge.dart index 5087004c..9ef33af3 100644 --- a/lib/services/platform_bridge.dart +++ b/lib/services/platform_bridge.dart @@ -22,6 +22,11 @@ bool isForegroundServiceStartNotAllowed(Object error) { Object? _decodeJsonInBackground(String json) => jsonDecode(json); String _encodeJsonInBackground(Object? value) => jsonEncode(value); +Object? _decodeJsonFileInBackground(String path) { + final contents = File(path).readAsStringSync(); + return contents.isEmpty ? null : jsonDecode(contents); +} + class LibraryScanNDJSONFile { final File file; final int expectedCount; @@ -292,7 +297,7 @@ class PlatformBridge { dynamic args, ]) async { final result = await _channel.invokeMethod(method, args); - return _decodeRequiredMapResult(result, method); + return _decodeRequiredMapResultAsync(result, method); } static Future ensureInstallMarker() async { @@ -327,13 +332,15 @@ class PlatformBridge { if (generation == _lookupCacheGeneration) { _putCachedMap(cache, cacheKey, value, ttl, persistentCacheKey); } - return _copyStringMap(value); + return value; }(); inFlight[cacheKey] = future; try { return _copyStringMap(await future); } finally { - inFlight.remove(cacheKey); + if (identical(inFlight[cacheKey], future)) { + inFlight.remove(cacheKey); + } } } @@ -375,7 +382,8 @@ class PlatformBridge { cache.remove(cache.keys.first); } cache[key] = _BridgeCacheEntry( - value: _copyStringMap(value), + // Loader results stay private; every caller receives its own deep copy. + value: value, expiresAt: DateTime.now().add(ttl), ); _scheduleLookupCachePersist( @@ -448,25 +456,28 @@ class PlatformBridge { try { final prefs = await SharedPreferences.getInstance(); if (generation != _lookupCacheGeneration) return; - _restorePersistentCache( + await _restorePersistentCache( prefs, _metadataPersistentCacheKey, _metadataCache, + generation, ); } catch (e) { _log.w('Failed to load bridge lookup cache: $e'); } } - static void _restorePersistentCache( + static Future _restorePersistentCache( SharedPreferences prefs, String prefsKey, Map target, - ) { + int generation, + ) async { final raw = prefs.getString(prefsKey); if (raw == null || raw.isEmpty) return; - final decoded = jsonDecode(raw); + final decoded = await _decodeJsonStringAsync(raw); + if (generation != _lookupCacheGeneration) return; if (decoded is! Map) return; final now = DateTime.now(); @@ -484,7 +495,7 @@ class PlatformBridge { if (!expiresAt.isAfter(now)) continue; target[key] = _BridgeCacheEntry( - value: _copyStringMap(Map.from(value)), + value: Map.from(value), expiresAt: expiresAt, ); } @@ -1496,7 +1507,7 @@ class PlatformBridge { 'getProviderMetadata returned null for $providerId:$resourceType:$resourceId', ); } - return _decodeRequiredMapResult(result, 'getProviderMetadata'); + return _decodeRequiredMapResultAsync(result, 'getProviderMetadata'); }, ); } @@ -1996,7 +2007,7 @@ class PlatformBridge { final result = await _channel.invokeMethod('handleURLWithExtension', { 'url': url, }); - final decoded = _decodeNullableMapResult( + final decoded = await _decodeNullableMapResultAsync( result, 'handleURLWithExtension', ); @@ -2049,7 +2060,7 @@ class PlatformBridge { cache.remove(cache.keys.first); } cache[key] = _BridgeCacheEntry( - value: _copyStringMap(value), + value: value, expiresAt: DateTime.now().add(ttl), ); } @@ -2079,7 +2090,7 @@ class PlatformBridge { cache.remove(cache.keys.first); } cache[key] = _BridgeListCacheEntry( - value: _copyMapList(value), + value: value, expiresAt: DateTime.now().add(ttl), ); } @@ -2386,9 +2397,9 @@ class PlatformBridge { if (result is Map && result[_jsonResultFileKey] is String) { final file = File(result[_jsonResultFileKey] as String); try { - final contents = await file.readAsString(); - if (contents.isEmpty) return null; - return await _decodeJsonStringAsync(contents); + // Read and decode where the result is built, so the UI isolate never + // retains a second copy of the entire spill file's text. + return await compute(_decodeJsonFileInBackground, file.path); } finally { try { await file.delete(); @@ -2449,6 +2460,18 @@ class PlatformBridge { ); } + static Future?> _decodeNullableMapResultAsync( + dynamic result, + String method, + ) async { + final decoded = await _decodeJsonResultAsync(result); + if (decoded == null) return null; + if (decoded is Map) return decoded.cast(); + throw FormatException( + 'Expected nullable map result from $method, got ${decoded.runtimeType}', + ); + } + static List _decodeRequiredListResult( dynamic result, String method, diff --git a/test/platform_bridge_custom_search_test.dart b/test/platform_bridge_custom_search_test.dart index 611d0b29..355241f8 100644 --- a/test/platform_bridge_custom_search_test.dart +++ b/test/platform_bridge_custom_search_test.dart @@ -23,6 +23,7 @@ void main() { 'artist_name': 'Artist', 'album_name': 'Album', 'duration': 123000, + 'external_links': {'example': 'https://example.invalid/track/$index'}, }, ); final json = jsonEncode(rows); @@ -42,6 +43,8 @@ void main() { ); expect(result, rows); result.first['name'] = 'Changed by caller'; + (result.first['external_links'] as Map)['example'] = + 'Changed by caller'; final cached = await PlatformBridge.customSearchWithExtension( 'decode-test-$format', 'query', diff --git a/test/platform_bridge_lookup_cache_test.dart b/test/platform_bridge_lookup_cache_test.dart new file mode 100644 index 00000000..1e4eac4e --- /dev/null +++ b/test/platform_bridge_lookup_cache_test.dart @@ -0,0 +1,206 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:spotiflac_android/services/platform_bridge.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + const channel = MethodChannel('com.zarz.spotiflac/backend'); + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + + setUp(() async { + SharedPreferences.setMockInitialValues({}); + messenger.setMockMethodCallHandler(channel, (_) async => null); + await PlatformBridge.clearTrackCache(); + }); + + tearDown(() async { + messenger.setMockMethodCallHandler(channel, (_) async => null); + await PlatformBridge.clearTrackCache(); + messenger.setMockMethodCallHandler(channel, null); + }); + + Map collection([int count = 2]) => { + 'type': 'album', + 'album_info': {'name': 'Album 音楽'}, + 'track_list': List.generate( + count, + (index) => { + 'id': 'track-$index', + 'name': 'Lagu $index — 日本語', + 'external_links': {'example': 'https://example.invalid/track/$index'}, + }, + ), + }; + + Future> load() => + PlatformBridge.getProviderMetadata('example', 'album', 'collection'); + + void mutate(Map value) { + (value['album_info'] as Map)['name'] = 'Changed'; + final first = (value['track_list'] as List).first as Map; + (first['external_links'] as Map)['example'] = 'Changed'; + } + + for (final count in [2, 3000]) { + test( + 'coalesced and cached metadata isolate nested mutations ($count)', + () async { + final value = collection(count); + final response = Completer(); + final started = Completer(); + var calls = 0; + messenger.setMockMethodCallHandler(channel, (call) async { + expect(call.method, 'getProviderMetadata'); + calls++; + if (!started.isCompleted) started.complete(); + return response.future; + }); + final first = load(); + await started.future; + final second = load(); + await Future.delayed(Duration.zero); + response.complete(jsonEncode(value)); + final results = await Future.wait([first, second]); + expect(results[0], value); + mutate(results[0]); + expect(results[1], value); + mutate(results[1]); + final cached = await load(); + expect(cached, value); + mutate(cached); + expect(await load(), value); + expect(calls, 1); + }, + ); + } + + test( + 'a cleared request cannot remove a replacement in-flight lookup', + () async { + final requests = >[]; + messenger.setMockMethodCallHandler(channel, (call) async { + if (call.method == 'clearTrackCache') return null; + expect(call.method, 'getProviderMetadata'); + final pending = Completer(); + requests.add(pending); + return pending.future; + }); + final old = load(); + await Future.delayed(Duration.zero); + expect(requests, hasLength(1)); + await PlatformBridge.clearTrackCache(); + final replacement = load(); + await Future.delayed(Duration.zero); + expect(requests, hasLength(2)); + requests[0].complete('{"version":"old"}'); + expect(await old, {'version': 'old'}); + final joined = load(); + await Future.delayed(Duration.zero); + // Complete unexpected calls too, so a failed assertion leaves no waiters. + for (final request in requests.skip(1)) { + request.complete('{"version":"new"}'); + } + expect(await replacement, {'version': 'new'}); + expect(await joined, {'version': 'new'}); + expect(requests, hasLength(2)); + expect(await load(), {'version': 'new'}); + }, + ); + + test( + 'large persisted metadata restores without exposing its stored objects', + () async { + final value = collection(3000); + final prefs = await SharedPreferences.getInstance(); + await prefs.setString( + 'bridge_metadata_lookup_cache_v1', + jsonEncode({ + 'example:album:collection': { + 'expires_at': DateTime.now() + .add(const Duration(minutes: 5)) + .millisecondsSinceEpoch, + 'value': value, + }, + }), + ); + messenger.setMockMethodCallHandler(channel, (call) async { + fail('Restored metadata must not query native: ${call.method}'); + }); + final restored = await load(); + expect(restored, value); + mutate(restored); + expect(await load(), value); + }, + ); + + for (final nullable in [false, true]) { + test( + 'large spill file is decoded and deleted (nullable=$nullable)', + () async { + final directory = await Directory.systemTemp.createTemp( + 'bridge-decode-', + ); + addTearDown(() => directory.delete(recursive: true)); + final file = File('${directory.path}/response.json'); + final value = collection(3000); + await file.writeAsString(jsonEncode(value)); + messenger.setMockMethodCallHandler( + channel, + (_) async => {'__json_file': file.path}, + ); + final result = nullable + ? await PlatformBridge.handleURLWithExtension( + 'https://example.invalid/album', + ) + : await load(); + expect(result, value); + expect(await file.exists(), isFalse); + mutate(result!); + final cached = nullable + ? await PlatformBridge.handleURLWithExtension( + 'https://example.invalid/album', + ) + : await load(); + expect(cached, value); + }, + ); + } + + test('invalid spill file is deleted and does not poison retry', () async { + final directory = await Directory.systemTemp.createTemp('bridge-invalid-'); + addTearDown(() => directory.delete(recursive: true)); + final file = File('${directory.path}/response.json'); + await file.writeAsString('{broken'); + var calls = 0; + messenger.setMockMethodCallHandler( + channel, + (_) async => + ++calls == 1 ? {'__json_file': file.path} : jsonEncode(collection()), + ); + await expectLater(load(), throwsFormatException); + expect(await file.exists(), isFalse); + expect(await load(), collection()); + expect(calls, 2); + }); + + test( + 'nullable URL responses preserve null and reject encoded scalar strings', + () async { + for (final raw in ['null', jsonEncode(jsonEncode(collection()))]) { + messenger.setMockMethodCallHandler(channel, (_) async => raw); + expect( + await PlatformBridge.handleURLWithExtension( + 'https://example.invalid/value', + ), + isNull, + ); + } + }, + ); +}