perf(jellyfin): fetch a detail item once when several callers want it at once
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
This commit is contained in:
@@ -612,6 +612,12 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
|
|||||||
/// (`enableResumable=true`, `disableFirstEpisode=false`) match Plex
|
/// (`enableResumable=true`, `disableFirstEpisode=false`) match Plex
|
||||||
/// OnDeck semantics: returns the resume episode when one exists, or S1E1
|
/// OnDeck semantics: returns the resume episode when one exists, or S1E1
|
||||||
/// when the user hasn't started. Movies and other kinds short-circuit.
|
/// 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
|
@override
|
||||||
Future<({MediaItem? item, MediaItem? onDeckEpisode})> fetchItemWithOnDeck(String id) async {
|
Future<({MediaItem? item, MediaItem? onDeckEpisode})> fetchItemWithOnDeck(String id) async {
|
||||||
final item = await fetchItem(id);
|
final item = await fetchItem(id);
|
||||||
@@ -629,8 +635,33 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
|
|||||||
return (item: item, onDeckEpisode: onDeckEpisode);
|
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<String, Future<MediaItem?>> _inFlightItems = {};
|
||||||
|
|
||||||
@override
|
@override
|
||||||
Future<MediaItem?> fetchItem(String id) async {
|
Future<MediaItem?> fetchItem(String id) {
|
||||||
|
final existing = _inFlightItems[id];
|
||||||
|
if (existing != null) return existing;
|
||||||
|
|
||||||
|
late final Future<MediaItem?> request;
|
||||||
|
request = _fetchItemOnce(id).whenComplete(() {
|
||||||
|
if (identical(_inFlightItems[id], request)) _inFlightItems.remove(id);
|
||||||
|
});
|
||||||
|
_inFlightItems[id] = request;
|
||||||
|
return request;
|
||||||
|
}
|
||||||
|
|
||||||
|
Future<MediaItem?> _fetchItemOnce(String id) async {
|
||||||
final endpoint = '/Users/${_segment(connection.userId)}/Items/${_segment(id)}';
|
final endpoint = '/Users/${_segment(connection.userId)}/Items/${_segment(id)}';
|
||||||
// Contract:
|
// Contract:
|
||||||
// - 200 with parseable Map → MediaItem
|
// - 200 with parseable Map → MediaItem
|
||||||
|
|||||||
@@ -134,6 +134,36 @@ void main() {
|
|||||||
await scoped.fetchLibraries();
|
await scoped.fetchLibraries();
|
||||||
expect(views, 2);
|
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', () {
|
test('buildDirectStreamUrl includes static flag, api_key, and device id', () {
|
||||||
final url = client.buildDirectStreamUrl('item-99');
|
final url = client.buildDirectStreamUrl('item-99');
|
||||||
|
|||||||
Reference in New Issue
Block a user