diff --git a/web/src/lib/managers/timeline-manager/day-group.svelte.ts b/web/src/lib/managers/timeline-manager/day-group.svelte.ts index 5869983dc3..e2c124e54c 100644 --- a/web/src/lib/managers/timeline-manager/day-group.svelte.ts +++ b/web/src/lib/managers/timeline-manager/day-group.svelte.ts @@ -4,9 +4,9 @@ import type { CommonLayoutOptions } from '$lib/utils/layout-utils'; import { getJustifiedLayoutFromAssets } from '$lib/utils/layout-utils'; import { plainDateTimeCompare } from '$lib/utils/timeline-util'; -import { SvelteSet } from 'svelte/reactivity'; +import { onCreateDayGroup } from '$lib/managers/timeline-manager/internal/TestHooks.svelte'; import type { MonthGroup } from './month-group.svelte'; -import type { AssetOperation, Direction, MoveAsset, TimelineAsset } from './types'; +import type { AssetOperation, Direction, TimelineAsset } from './types'; import { ViewerAsset } from './viewer-asset.svelte'; export class DayGroup { @@ -31,6 +31,7 @@ export class DayGroup { this.monthGroup = monthGroup; this.day = day; this.groupTitle = groupTitle; + onCreateDayGroup(this); } get top() { @@ -104,15 +105,18 @@ export class DayGroup { runAssetOperation(ids: Set, operation: AssetOperation) { if (ids.size === 0) { return { - moveAssets: [] as MoveAsset[], - processedIds: new SvelteSet(), + moveAssets: [] as TimelineAsset[], + // eslint-disable-next-line svelte/prefer-svelte-reactivity + processedIds: new Set(), unprocessedIds: ids, changedGeometry: false, }; } - const unprocessedIds = new SvelteSet(ids); - const processedIds = new SvelteSet(); - const moveAssets: MoveAsset[] = []; + // eslint-disable-next-line svelte/prefer-svelte-reactivity + const unprocessedIds = new Set(ids); + // eslint-disable-next-line svelte/prefer-svelte-reactivity + const processedIds = new Set(); + const moveAssets: TimelineAsset[] = []; let changedGeometry = false; for (const assetId of unprocessedIds) { const index = this.viewerAssets.findIndex((viewAsset) => viewAsset.id == assetId); @@ -121,6 +125,7 @@ export class DayGroup { } const asset = this.viewerAssets[index].asset!; + // save old time, pre-mutating operation const oldTime = { ...asset.localDateTime }; const opResult = operation(asset); let remove = false; @@ -128,10 +133,12 @@ export class DayGroup { remove = (opResult as { remove: boolean }).remove ?? false; } const newTime = asset.localDateTime; - if (oldTime.year !== newTime.year || oldTime.month !== newTime.month || oldTime.day !== newTime.day) { - const { year, month, day } = newTime; + if ( + !remove && + (oldTime.year !== newTime.year || oldTime.month !== newTime.month || oldTime.day !== newTime.day) + ) { remove = true; - moveAssets.push({ asset, date: { year, month, day } }); + moveAssets.push(asset); } unprocessedIds.delete(assetId); processedIds.add(assetId); diff --git a/web/src/lib/managers/timeline-manager/internal/TestHooks.svelte.ts b/web/src/lib/managers/timeline-manager/internal/TestHooks.svelte.ts new file mode 100644 index 0000000000..9de90cbda0 --- /dev/null +++ b/web/src/lib/managers/timeline-manager/internal/TestHooks.svelte.ts @@ -0,0 +1,16 @@ +import type { DayGroup } from '$lib/managers/timeline-manager/day-group.svelte'; +import type { MonthGroup } from '$lib/managers/timeline-manager/month-group.svelte'; + +let testHooks: TestHooks | undefined = undefined; + +export type TestHooks = { + onCreateMonthGroup(monthGroup: MonthGroup): unknown; + onCreateDayGroup(dayGroup: DayGroup): unknown; +}; + +export const setTestHooks = (hooks: TestHooks) => { + testHooks = hooks; +}; + +export const onCreateMonthGroup = (monthGroup: MonthGroup) => testHooks?.onCreateMonthGroup(monthGroup); +export const onCreateDayGroup = (dayGroup: DayGroup) => testHooks?.onCreateDayGroup(dayGroup); diff --git a/web/src/lib/managers/timeline-manager/month-group.svelte.ts b/web/src/lib/managers/timeline-manager/month-group.svelte.ts index 841eb83219..12b054c69c 100644 --- a/web/src/lib/managers/timeline-manager/month-group.svelte.ts +++ b/web/src/lib/managers/timeline-manager/month-group.svelte.ts @@ -9,7 +9,7 @@ import { fromTimelinePlainDateTime, fromTimelinePlainYearMonth, getTimes, - setDifference, + setDifferenceInPlace, type TimelineDateTime, type TimelineYearMonth, } from '$lib/utils/timeline-util'; @@ -17,11 +17,11 @@ import { import { t } from 'svelte-i18n'; import { get } from 'svelte/store'; -import { SvelteSet } from 'svelte/reactivity'; +import { onCreateMonthGroup } from '$lib/managers/timeline-manager/internal/TestHooks.svelte'; import { DayGroup } from './day-group.svelte'; import { GroupInsertionCache } from './group-insertion-cache.svelte'; import type { TimelineManager } from './timeline-manager.svelte'; -import type { AssetDescriptor, AssetOperation, Direction, MoveAsset, TimelineAsset } from './types'; +import type { AssetDescriptor, AssetOperation, Direction, TimelineAsset } from './types'; import { ViewerAsset } from './viewer-asset.svelte'; export class MonthGroup { @@ -76,6 +76,7 @@ export class MonthGroup { if (loaded) { this.isLoaded = true; } + onCreateMonthGroup(this); } set intersecting(newValue: boolean) { @@ -119,26 +120,29 @@ export class MonthGroup { runAssetOperation(ids: Set, operation: AssetOperation) { if (ids.size === 0) { return { - moveAssets: [] as MoveAsset[], - processedIds: new SvelteSet(), + moveAssets: [] as TimelineAsset[], + // eslint-disable-next-line svelte/prefer-svelte-reactivity + processedIds: new Set(), unprocessedIds: ids, changedGeometry: false, }; } const { dayGroups } = this; let combinedChangedGeometry = false; - let idsToProcess = new SvelteSet(ids); - const idsProcessed = new SvelteSet(); - const combinedMoveAssets: MoveAsset[][] = []; + // eslint-disable-next-line svelte/prefer-svelte-reactivity + const idsToProcess = new Set(ids); + // eslint-disable-next-line svelte/prefer-svelte-reactivity + const idsProcessed = new Set(); + const combinedMoveAssets: TimelineAsset[] = []; let index = dayGroups.length; while (index--) { if (idsToProcess.size > 0) { const group = dayGroups[index]; const { moveAssets, processedIds, changedGeometry } = group.runAssetOperation(ids, operation); if (moveAssets.length > 0) { - combinedMoveAssets.push(moveAssets); + combinedMoveAssets.push(...moveAssets); } - idsToProcess = setDifference(idsToProcess, processedIds); + setDifferenceInPlace(idsToProcess, processedIds); for (const id of processedIds) { idsProcessed.add(id); } @@ -150,7 +154,7 @@ export class MonthGroup { } } return { - moveAssets: combinedMoveAssets.flat(), + moveAssets: combinedMoveAssets, unprocessedIds: idsToProcess, processedIds: idsProcessed, changedGeometry: combinedChangedGeometry, diff --git a/web/src/lib/managers/timeline-manager/timeline-manager.svelte.spec.ts b/web/src/lib/managers/timeline-manager/timeline-manager.svelte.spec.ts index fc50888e77..1539dd56e7 100644 --- a/web/src/lib/managers/timeline-manager/timeline-manager.svelte.spec.ts +++ b/web/src/lib/managers/timeline-manager/timeline-manager.svelte.spec.ts @@ -1,10 +1,14 @@ import { sdkMock } from '$lib/__mocks__/sdk.mock'; +import type { DayGroup } from '$lib/managers/timeline-manager/day-group.svelte'; import { getMonthGroupByDate } from '$lib/managers/timeline-manager/internal/search-support.svelte'; +import { setTestHooks } from '$lib/managers/timeline-manager/internal/TestHooks.svelte'; +import type { MonthGroup } from '$lib/managers/timeline-manager/month-group.svelte'; import { AbortError } from '$lib/utils'; import { fromISODateTimeUTCToObject } from '$lib/utils/timeline-util'; import { type AssetResponseDto, type TimeBucketAssetResponseDto } from '@immich/sdk'; import { timelineAssetFactory, toResponseDto } from '@test-data/factories/asset-factory'; import { tick } from 'svelte'; +import type { MockInstance } from 'vitest'; import { TimelineManager } from './timeline-manager.svelte'; import type { TimelineAsset } from './types'; @@ -299,6 +303,122 @@ describe('TimelineManager', () => { }); }); + describe('ensure efficient timeline operations', () => { + let timelineManager: TimelineManager; + + let month1day1asset1: TimelineAsset, + month1day2asset1: TimelineAsset, + month1day2asset2: TimelineAsset, + month1day3asset1: TimelineAsset, + month2day1asset1: TimelineAsset, + month2day2asset1: TimelineAsset, + month2day2asset2: TimelineAsset; + + type DayMocks = { + layoutFn: MockInstance; + sortAssetsFn: MockInstance; + }; + type MonthMocks = { + sortDayGroupsFn: MockInstance; + }; + + const dayGroups = new Map(); + const monthGroups = new Map(); + + beforeEach(async () => { + timelineManager = new TimelineManager(); + setTestHooks({ + onCreateDayGroup: (dayGroup: DayGroup) => { + dayGroups.set(dayGroup, { + layoutFn: vi.spyOn(dayGroup, 'layout'), + sortAssetsFn: vi.spyOn(dayGroup, 'sortAssets'), + }); + }, + onCreateMonthGroup: (monthGroup: MonthGroup) => { + monthGroups.set(monthGroup, { + sortDayGroupsFn: vi.spyOn(monthGroup, 'sortDayGroups'), + }); + }, + }); + sdkMock.getTimeBuckets.mockResolvedValue([]); + month1day1asset1 = deriveLocalDateTimeFromFileCreatedAt( + timelineAssetFactory.build({ + fileCreatedAt: fromISODateTimeUTCToObject('2024-01-20T12:00:00.000Z'), + }), + ); + month1day2asset1 = deriveLocalDateTimeFromFileCreatedAt( + timelineAssetFactory.build({ + fileCreatedAt: fromISODateTimeUTCToObject('2024-01-15T12:00:00.000Z'), + }), + ); + month1day2asset2 = deriveLocalDateTimeFromFileCreatedAt( + timelineAssetFactory.build({ + fileCreatedAt: fromISODateTimeUTCToObject('2024-01-15T13:00:00.000Z'), + }), + ); + month1day3asset1 = deriveLocalDateTimeFromFileCreatedAt( + timelineAssetFactory.build({ + fileCreatedAt: fromISODateTimeUTCToObject('2024-01-16T12:00:00.000Z'), + }), + ); + month2day1asset1 = deriveLocalDateTimeFromFileCreatedAt( + timelineAssetFactory.build({ + fileCreatedAt: fromISODateTimeUTCToObject('2024-02-16T12:00:00.000Z'), + }), + ); + month2day2asset1 = deriveLocalDateTimeFromFileCreatedAt( + timelineAssetFactory.build({ + fileCreatedAt: fromISODateTimeUTCToObject('2024-02-18T12:00:00.000Z'), + }), + ); + month2day2asset2 = deriveLocalDateTimeFromFileCreatedAt( + timelineAssetFactory.build({ + fileCreatedAt: fromISODateTimeUTCToObject('2024-02-18T13:00:00.000Z'), + }), + ); + + await timelineManager.updateViewport({ width: 1588, height: 1000 }); + timelineManager.upsertAssets([ + month1day1asset1, + month1day2asset1, + month1day2asset2, + month1day3asset1, + month2day1asset1, + month2day2asset1, + month2day2asset2, + ]); + vitest.resetAllMocks(); + }); + it.skip('Not Ready Yet - optimizations not complete: moving asset between months only sorts/layout the affected months once', () => { + // move from 2024-01-15 to 2024-01-16 + timelineManager.updateAssetOperation([month1day2asset1.id], (asset) => { + asset.localDateTime.day = asset.localDateTime.day + 1; + }); + for (const [day, mocks] of dayGroups) { + if (day.day === 15 && day.monthGroup.yearMonth.month === 1) { + // source - should be layout once + expect.soft(mocks.layoutFn).toBeCalledTimes(1); + expect.soft(mocks.sortAssetsFn).toBeCalledTimes(1); + } + if (day.day === 16 && day.monthGroup.yearMonth.month === 1) { + // target - should be layout once + expect.soft(mocks.layoutFn).toBeCalledTimes(1); + expect.soft(mocks.sortAssetsFn).toBeCalledTimes(1); + } + // everything else - should not be layed-out + expect.soft(mocks.layoutFn).toBeCalledTimes(0); + expect.soft(mocks.sortAssetsFn).toBeCalledTimes(0); + } + for (const [_, mocks] of monthGroups) { + // if the day itself did not change, probably no need to sort it + // in the timeline manager, the day-group identity is immutable - you will never + // "move" a whole day to another day - only the assets inside will be moved from + // one to the other. + expect.soft(mocks.sortDayGroupsFn).toBeCalledTimes(0); + } + }); + }); + describe('upsertAssets', () => { let timelineManager: TimelineManager; diff --git a/web/src/lib/managers/timeline-manager/timeline-manager.svelte.ts b/web/src/lib/managers/timeline-manager/timeline-manager.svelte.ts index 66f1d4cc25..f1c0da1160 100644 --- a/web/src/lib/managers/timeline-manager/timeline-manager.svelte.ts +++ b/web/src/lib/managers/timeline-manager/timeline-manager.svelte.ts @@ -15,7 +15,7 @@ import { import { WebsocketSupport } from '$lib/managers/timeline-manager/internal/websocket-support.svelte'; import { CancellableTask } from '$lib/utils/cancellable-task'; import { - setDifference, + setDifferenceInPlace, toTimelineAsset, type TimelineDateTime, type TimelineYearMonth, @@ -30,7 +30,6 @@ import type { AssetDescriptor, AssetOperation, Direction, - MoveAsset, ScrubberMonth, TimelineAsset, TimelineManagerOptions, @@ -326,7 +325,7 @@ export class TimelineManager extends VirtualScrollManager { upsertAssets(assets: TimelineAsset[]) { const notExcluded = assets.filter((asset) => !this.isExcluded(asset)); const notUpdated = this.#updateAssets(notExcluded); - this.addAssetsToSegments([...notUpdated]); + this.addAssetsToSegments(notUpdated); } async findMonthGroupForAsset(id: string) { @@ -403,29 +402,40 @@ export class TimelineManager extends VirtualScrollManager { return randomDay.viewerAssets[randomAssetIndex - accumulatedCount].asset; } + /** + * Executes the given operation against every passed in asset id. + * + * @returns An object with the changed ids, unprocessed ids, and if this resulted + * in changes of the timeline geometry. + */ updateAssetOperation(ids: string[], operation: AssetOperation) { - // eslint-disable-next-line svelte/prefer-svelte-reactivity - return this.#runAssetOperation(new Set(ids), operation); + return this.#runAssetOperation(ids, operation); } - #updateAssets(assets: TimelineAsset[]) { + /** + * Looks up the specified asset from the TimelineAsset using its id, and then updates the + * existing object to match the rest of the TimelineAsset parameter. + + * @returns list of assets that were updated (not found) + */ + #updateAssets(updatedAssets: TimelineAsset[]) { // eslint-disable-next-line svelte/prefer-svelte-reactivity - const lookup = new Map(assets.map((asset) => [asset.id, asset])); - // eslint-disable-next-line svelte/prefer-svelte-reactivity - const { unprocessedIds } = this.#runAssetOperation(new Set(lookup.keys()), (asset) => - updateObject(asset, lookup.get(asset.id)), - ); + const lookup = new Map(); + const ids = []; + for (const asset of updatedAssets) { + ids.push(asset.id); + lookup.set(asset.id, asset); + } + const { unprocessedIds } = this.#runAssetOperation(ids, (asset) => updateObject(asset, lookup.get(asset.id))); const result: TimelineAsset[] = []; - for (const id of unprocessedIds.values()) { + for (const id of unprocessedIds) { result.push(lookup.get(id)!); } return result; } removeAssets(ids: string[]) { - // eslint-disable-next-line svelte/prefer-svelte-reactivity - const { unprocessedIds } = this.#runAssetOperation(new Set(ids), () => ({ remove: true })); - return [...unprocessedIds]; + this.#runAssetOperation(ids, () => ({ remove: true })); } protected createUpsertContext(): GroupInsertionCache { @@ -459,26 +469,26 @@ export class TimelineManager extends VirtualScrollManager { this.updateIntersections(); } - #runAssetOperation(ids: Set, operation: AssetOperation) { - if (ids.size === 0) { + #runAssetOperation(ids: string[], operation: AssetOperation) { + if (ids.length === 0) { // eslint-disable-next-line svelte/prefer-svelte-reactivity - return { processedIds: new Set(), unprocessedIds: ids, changedGeometry: false }; + return { processedIds: new Set(), unprocessedIds: new Set(), changedGeometry: false }; } // eslint-disable-next-line svelte/prefer-svelte-reactivity const changedMonthGroups = new Set(); // eslint-disable-next-line svelte/prefer-svelte-reactivity - let idsToProcess = new Set(ids); + const idsToProcess = new Set(ids); // eslint-disable-next-line svelte/prefer-svelte-reactivity const idsProcessed = new Set(); - const combinedMoveAssets: MoveAsset[][] = []; + const combinedMoveAssets: TimelineAsset[] = []; for (const month of this.months) { if (idsToProcess.size > 0) { const { moveAssets, processedIds, changedGeometry } = month.runAssetOperation(idsToProcess, operation); if (moveAssets.length > 0) { - combinedMoveAssets.push(moveAssets); + combinedMoveAssets.push(...moveAssets); } - idsToProcess = setDifference(idsToProcess, processedIds); + setDifferenceInPlace(idsToProcess, processedIds); for (const id of processedIds) { idsProcessed.add(id); } @@ -488,7 +498,7 @@ export class TimelineManager extends VirtualScrollManager { } } if (combinedMoveAssets.length > 0) { - this.addAssetsToSegments(combinedMoveAssets.flat().map((a) => a.asset)); + this.addAssetsToSegments(combinedMoveAssets); } const changedGeometry = changedMonthGroups.size > 0; for (const month of changedMonthGroups) { diff --git a/web/src/lib/managers/timeline-manager/types.ts b/web/src/lib/managers/timeline-manager/types.ts index 41f2653d2f..ea042d8366 100644 --- a/web/src/lib/managers/timeline-manager/types.ts +++ b/web/src/lib/managers/timeline-manager/types.ts @@ -1,4 +1,4 @@ -import type { TimelineDate, TimelineDateTime, TimelineYearMonth } from '$lib/utils/timeline-util'; +import type { TimelineDateTime, TimelineYearMonth } from '$lib/utils/timeline-util'; import type { AssetStackResponseDto, AssetVisibility } from '@immich/sdk'; export type ViewportTopMonth = TimelineYearMonth | undefined | 'lead-in' | 'lead-out'; @@ -37,9 +37,7 @@ export type TimelineAsset = { longitude?: number | null; }; -export type AssetOperation = (asset: TimelineAsset) => { remove: boolean } | unknown; - -export type MoveAsset = { asset: TimelineAsset; date: TimelineDate }; +export type AssetOperation = (asset: TimelineAsset) => unknown; export interface Viewport { width: number; diff --git a/web/src/lib/utils/timeline-util.ts b/web/src/lib/utils/timeline-util.ts index 3326676f3c..6b50cf71eb 100644 --- a/web/src/lib/utils/timeline-util.ts +++ b/web/src/lib/utils/timeline-util.ts @@ -3,7 +3,6 @@ import { locale } from '$lib/stores/preferences.store'; import { getAssetRatio } from '$lib/utils/asset-utils'; import { AssetTypeEnum, type AssetResponseDto } from '@immich/sdk'; import { DateTime, type LocaleOptions } from 'luxon'; -import { SvelteSet } from 'svelte/reactivity'; import { get } from 'svelte/store'; // Move type definitions to the top @@ -222,8 +221,13 @@ export const plainDateTimeCompare = (ascending: boolean, a: TimelineDateTime, b: return aDateTime.millisecond - bDateTime.millisecond; }; -export function setDifference(setA: Set, setB: Set): SvelteSet { - const result = new SvelteSet(); +export function setDifference(setA: Set, setB: Set): Set { + // Check if native Set.prototype.difference is available (ES2025) + const setWithDifference = setA as unknown as Set & { difference?: (other: Set) => Set }; + if (setWithDifference.difference && typeof setWithDifference.difference === 'function') { + return setWithDifference.difference(setB); + } + const result = new Set(); for (const value of setA) { if (!setB.has(value)) { result.add(value); @@ -231,3 +235,13 @@ export function setDifference(setA: Set, setB: Set): SvelteSet { } return result; } + +/** + * Removes all elements of setB from setA in-place (mutates setA). + */ +export function setDifferenceInPlace(setA: Set, setB: Set): Set { + for (const value of setB) { + setA.delete(value); + } + return setA; +}