From a759e8b3c60759658871311aba0f0afdda1790b3 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Tue, 4 Aug 2026 06:39:23 +0200 Subject: [PATCH] perf(jellyfin): fetch a detail item once when several callers want it at once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening a detail screen issued two identical full-detail GETs for the same id, concurrently: `_loadFullMetadata` calls `fetchItemWithOnDeck`, and `_initWatchlistState` calls `fetchExternalIds`, which fetches the same item purely to read `ProviderIds`. Playback start adds three more for its own id. Each of those makes the server rebuild the entire dto — `People`, `Chapters` and `MediaSources` cost a database query apiece and `Trickplay` costs several plus a filesystem stat — so the duplicate is expensive on both ends. `fetchItem` now shares an in-flight request per item id. Single-flight only: once a request settles the next caller re-fetches, so nothing can serve a stale item. Measured on a remote Jellyfin server, 12 interleaved show-detail opens per version: requests 4 -> 3, payload 28.4 KB -> 18.6 KB. Median wall time is unchanged (1394ms -> 1386ms) because the duplicate ran alongside the first rather than behind it; this removes duplicated work, not latency. Two things were tried and rejected because measurement did not support them: starting `/Shows/NextUp` in parallel with the detail fetch (the requests contend rather than overlap — NextUp went from 380ms alone to 1395ms beside it — and it costs a wasted request per movie), and dropping `Trickplay` / `Chapters` from the detail field set (no measurable effect; both are real data the playback path reads). Refs #1784 --- .../jellyfin_client/parts/browse.dart | 33 ++++++++++++++++++- test/services/jellyfin_client_urls_test.dart | 30 +++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/lib/services/jellyfin_client/parts/browse.dart b/lib/services/jellyfin_client/parts/browse.dart index 6302524f..2cff6ccf 100644 --- a/lib/services/jellyfin_client/parts/browse.dart +++ b/lib/services/jellyfin_client/parts/browse.dart @@ -612,6 +612,12 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { /// (`enableResumable=true`, `disableFirstEpisode=false`) match Plex /// OnDeck semantics: returns the resume episode when one exists, or S1E1 /// when the user hasn't started. Movies and other kinds short-circuit. + /// + /// Deliberately still chained rather than fired in parallel off a caller + /// kind hint: measured against a remote server that saved nothing, because + /// the two requests contend rather than overlap (`/Shows/NextUp` went from + /// 380ms alone to 1395ms beside the detail fetch), and it would cost a + /// wasted request on every movie whose hint was absent or wrong. @override Future<({MediaItem? item, MediaItem? onDeckEpisode})> fetchItemWithOnDeck(String id) async { final item = await fetchItem(id); @@ -629,8 +635,33 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { return (item: item, onDeckEpisode: onDeckEpisode); } + /// In-flight `fetchItem` requests, keyed by item id. + /// + /// Opening a detail screen issues two identical full-detail GETs for the + /// same id at the same time — `_loadFullMetadata` and, from + /// `_initWatchlistState`, `fetchExternalIds` — and playback start adds three + /// more. Each one makes the server rebuild the whole dto (People, Chapters + /// and MediaSources are a DB query apiece, Trickplay several), so the + /// duplicates are expensive on both ends (#1784). + /// + /// Single-flight only: once a request settles the next caller re-fetches, so + /// nothing here can serve a stale item. + final Map> _inFlightItems = {}; + @override - Future fetchItem(String id) async { + Future fetchItem(String id) { + final existing = _inFlightItems[id]; + if (existing != null) return existing; + + late final Future request; + request = _fetchItemOnce(id).whenComplete(() { + if (identical(_inFlightItems[id], request)) _inFlightItems.remove(id); + }); + _inFlightItems[id] = request; + return request; + } + + Future _fetchItemOnce(String id) async { final endpoint = '/Users/${_segment(connection.userId)}/Items/${_segment(id)}'; // Contract: // - 200 with parseable Map → MediaItem diff --git a/test/services/jellyfin_client_urls_test.dart b/test/services/jellyfin_client_urls_test.dart index 3d9f71d1..8cdc250d 100644 --- a/test/services/jellyfin_client_urls_test.dart +++ b/test/services/jellyfin_client_urls_test.dart @@ -134,6 +134,36 @@ void main() { await scoped.fetchLibraries(); expect(views, 2); }); + test('concurrent fetchItem calls for one id share a single request', () async { + // Opening a detail screen fires `_loadFullMetadata` and, via + // `_initWatchlistState`, `fetchExternalIds` — both a full-detail GET for + // the same id at the same time. Each makes the server rebuild the whole + // dto (People, Chapters and MediaSources are a DB query apiece). + var detailFetches = 0; + final scoped = JellyfinClient.forTesting( + connection: _conn(), + httpClient: MockClient((req) async { + if (req.url.path == '/Users/user-1/Items/item-1') detailFetches++; + return http.Response( + jsonEncode({'Id': 'item-1', 'Name': 'Item', 'Type': 'Movie'}), + 200, + headers: {'content-type': 'application/json'}, + ); + }), + ); + addTearDown(scoped.close); + + final results = await Future.wait([scoped.fetchItem('item-1'), scoped.fetchItem('item-1')]); + + expect(detailFetches, 1); + expect(results.map((item) => item?.id), ['item-1', 'item-1']); + + // Different ids never share, and a later pass re-fetches — single-flight, + // not a cache, so nothing here can serve a stale item. + await scoped.fetchItem('item-2'); + await scoped.fetchItem('item-1'); + expect(detailFetches, 2); + }); test('buildDirectStreamUrl includes static flag, api_key, and device id', () { final url = client.buildDirectStreamUrl('item-99');