diff --git a/docs/architecture/performance-journeys.md b/docs/architecture/performance-journeys.md index a40b3ab9e..191441e17 100644 --- a/docs/architecture/performance-journeys.md +++ b/docs/architecture/performance-journeys.md @@ -184,13 +184,14 @@ harness, which is what the ratchet needs. The main process start `renderer.ipcCallsToFirstCard` counts what the renderer asks of the main process before the first card. -- `PlaylistsService.getAllPlaylists()` shares one pending SQLite read between - concurrent callers: at startup the playlist effect and the XMLTV source - reconciliation both read the inventory, and the second caller joins the - first read and receives a `structuredClone` of its result. A settled read - is never reused, and every SQLite write detaches the pending read, so a - caller that follows a write reads again. `dbGetAppPlaylistMetas` before the - first card: 2 → 1. +- `PlaylistsService.getAllPlaylists()` shares its first SQLite read: at + startup the playlist effect and the XMLTV source reconciliation both read + the inventory, and the second caller joins the first read and receives a + `structuredClone` of its result. Sharing ends when that read settles or a + `PlaylistsService` write starts. It is limited to startup on purpose: + other services write playlists too (the settings reset deletes them + through `DatabaseService`), and while the startup screen is up no such + action can run. `dbGetAppPlaylistMetas` before the first card: 2 → 1. - `reconcileEpgSources` stays before the first card on purpose: its completion bumps `EpgSourceSettingsService.revision()`, the fence that keeps XMLTV lookups from returning data of a removed source. diff --git a/libs/services/src/lib/playlist-migration.spec.ts b/libs/services/src/lib/playlist-migration.spec.ts index ed66a5ac2..a68e974ee 100644 --- a/libs/services/src/lib/playlist-migration.spec.ts +++ b/libs/services/src/lib/playlist-migration.spec.ts @@ -38,7 +38,6 @@ describe('Electron legacy playlist migration', () => { dbService, runtime: { supportsSqlite: true }, electronMigrationPromise: null, - pendingMetas: null, }); return { playlists, dbService, electron, service }; } diff --git a/libs/services/src/lib/playlists.service.inventory-read.spec.ts b/libs/services/src/lib/playlists.service.inventory-read.spec.ts index 800171f83..d0be5779d 100644 --- a/libs/services/src/lib/playlists.service.inventory-read.spec.ts +++ b/libs/services/src/lib/playlists.service.inventory-read.spec.ts @@ -18,6 +18,13 @@ describe('PlaylistsService inventory reads', () => { return { promise, resolve }; } + async function until(ready: () => boolean, what: string) { + for (let turn = 0; !ready(); turn += 1) { + if (turn > 100) throw new Error(`${what} never started`); + await Promise.resolve(); + } + } + function setup() { const reads: ReturnType>[] = []; const electron = { @@ -43,12 +50,11 @@ describe('PlaylistsService inventory reads', () => { dbService: { getAll: jest.fn(() => of([])) }, runtime: { supportsSqlite: true }, electronMigrationPromise: null, - pendingMetas: null, playlistWriteQueues: new Map(), }); const settle = async (index: number, playlists: Playlist[]) => { // The read starts after the memoized migration's awaits. - while (!reads[index]) await Promise.resolve(); + await until(() => !!reads[index], `metadata read ${index}`); reads[index].resolve(playlists); }; return { electron, service, settle }; @@ -85,13 +91,33 @@ describe('PlaylistsService inventory reads', () => { expect(electron.dbGetAppPlaylistMetas).toHaveBeenCalledTimes(2); }); + it('stops sharing once the startup read settled', async () => { + const { electron, service, settle } = setup(); + + const startup = firstValueFrom(service.getAllPlaylists()); + await settle(0, [source('startup')]); + await startup; + // Writes that bypass this service (e.g. the settings reset through + // DatabaseService) are possible from here on, so every caller reads. + const first = firstValueFrom(service.getAllPlaylists()); + const second = firstValueFrom(service.getAllPlaylists()); + await settle(1, [source('one')]); + await settle(2, [source('two')]); + + await expect(first).resolves.toEqual([source('one')]); + await expect(second).resolves.toEqual([source('two')]); + expect(electron.dbGetAppPlaylistMetas).toHaveBeenCalledTimes(3); + }); + it('starts a fresh read for callers that arrive after a write', async () => { const { electron, service, settle } = setup(); const added = source('added'); const staleRead = firstValueFrom(service.getAllPlaylists()); - while (electron.dbGetAppPlaylistMetas.mock.calls.length === 0) - await Promise.resolve(); + await until( + () => electron.dbGetAppPlaylistMetas.mock.calls.length > 0, + 'metadata read 0' + ); await firstValueFrom(service.addPlaylist(added)); const freshRead = firstValueFrom(service.getAllPlaylists()); await settle(0, []); diff --git a/libs/services/src/lib/playlists.service.spec.ts b/libs/services/src/lib/playlists.service.spec.ts index 6b5332a2a..71a4961f6 100644 --- a/libs/services/src/lib/playlists.service.spec.ts +++ b/libs/services/src/lib/playlists.service.spec.ts @@ -60,7 +60,6 @@ describe('PlaylistsService', () => { }, electronMigrationPromise: null, indexedDbMigrationPromise: null, - pendingMetas: null, playlistWriteQueues: new Map(), playlistDeleteCleanups: [], }); diff --git a/libs/services/src/lib/playlists.service.ts b/libs/services/src/lib/playlists.service.ts index 143d7a798..9719348bf 100644 --- a/libs/services/src/lib/playlists.service.ts +++ b/libs/services/src/lib/playlists.service.ts @@ -117,9 +117,11 @@ export class PlaylistsService { inject(PLAYLIST_DELETE_CLEANUP, { optional: true }) ?? []; private electronMigrationPromise: Promise | null = null; // Startup reads the inventory twice at once (the playlist effect and the - // XMLTV source reconciliation); both share one worker round trip. Only a - // pending read is shared, and every SQLite write detaches it. - private pendingMetas: Promise | null = null; + // XMLTV source reconciliation); both share one worker round trip. Only the + // first read is shared, while the startup screen still hides every action + // that writes playlists (other services write them too, e.g. the settings + // reset); it ends when that read settles or a write here starts (null). + private startupMetas?: Promise | null; private indexedDbMigrationPromise: Promise | null = null; private readonly playlistWriteQueues = new Map>(); @@ -401,7 +403,7 @@ export class PlaylistsService { return playlist; } - this.pendingMetas = null; + this.startupMetas = null; if (operationId === undefined) { await electron.dbUpsertAppPlaylist(playlist); } else { @@ -419,7 +421,7 @@ export class PlaylistsService { return playlists; } - this.pendingMetas = null; + this.startupMetas = null; await electron.dbUpsertAppPlaylists(playlists); playlists.forEach((playlist) => this.healthEvidence?.connections.next({ id: playlist._id, playlist })); return playlists; @@ -498,9 +500,9 @@ export class PlaylistsService { getAllPlaylists() { if (this.isElectronStorageAvailable) { - const pending = this.pendingMetas; + const shared = this.startupMetas; // A joining caller gets its own copy of the shared result. - if (pending) return from(pending.then((p) => structuredClone(p))); + if (shared) return from(shared.then((p) => structuredClone(p))); const read = firstValueFrom( this.runOnSqlite(async () => { const electron = this.electronApi; @@ -513,12 +515,11 @@ export class PlaylistsService { ); }) ); - this.pendingMetas = read; - const settle = () => { - if (this.pendingMetas === read) - this.pendingMetas = null; - }; - read.then(settle, settle); + if (shared === undefined) { + this.startupMetas = read; + const end = () => (this.startupMetas = null); + read.then(end, end); + } return from(read); } @@ -573,7 +574,7 @@ export class PlaylistsService { await this.ensureElectronPlaylistMigrations(); const electron = this.electronApi; if (electron) { - this.pendingMetas = null; + this.startupMetas = null; if (options) { const deleted = await this.databaseService.deletePlaylist( @@ -1345,7 +1346,7 @@ export class PlaylistsService { await this.ensureElectronPlaylistMigrations(); const electron = this.electronApi; if (electron) { - this.pendingMetas = null; + this.startupMetas = null; await electron.dbDeleteAllPlaylists(); this.healthEvidence?.connections.next({}); }