diff --git a/.changeset/every-vans-stand.md b/.changeset/every-vans-stand.md new file mode 100644 index 00000000000..b2771abc9b3 --- /dev/null +++ b/.changeset/every-vans-stand.md @@ -0,0 +1,5 @@ +--- +'@tanstack/router-core': patch +--- + +Avoid quadratic resource handoff scans when navigating with many cached route matches. diff --git a/packages/router-core/src/load-client.ts b/packages/router-core/src/load-client.ts index 2b5a32094a3..a8b3f17f71c 100644 --- a/packages/router-core/src/load-client.ts +++ b/packages/router-core/src/load-client.ts @@ -1578,7 +1578,7 @@ function publishMatches( router.stores.setMatches(matches) } -function commitMatches( +export function commitMatches( router: CoordinatorRouter, tx: LoadTransaction, matches: LaneMatches<'projected'>, @@ -1641,10 +1641,14 @@ function commitMatches( tx[3 /* matches */] = [] router._cache = cached publishMatches(router, matches) + // Retained cache objects keep their leases; only departing owners need handoff. + const previousMatches = [...previousCached.values(), ...previous] transferMatchResources( router, - [...previousCached.values(), ...previous], - [...matches, ...cached.values()], + previousMatches.filter( + (match: WorkMatch) => match._flight && cached.get(match.id) !== match, + ), + matches, ) if (process.env.NODE_ENV !== 'production') { const handoff = tx[6 /* refresh */]?.[0 /* handoff */] diff --git a/packages/router-core/tests/cache-commit.bench.ts b/packages/router-core/tests/cache-commit.bench.ts new file mode 100644 index 00000000000..67ceebad3e8 --- /dev/null +++ b/packages/router-core/tests/cache-commit.bench.ts @@ -0,0 +1,109 @@ +import { bench, describe, expect } from 'vitest' +import { commitMatches } from '../src/load-client' +import type { AnyRouteMatch, AnyRouter } from '../src' +import type { LoaderFlight, LoadTransaction } from '../src/load-client' + +type Match = AnyRouteMatch & { _flight?: LoaderFlight } + +// Run each family separately with -t to keep GC from other workloads out of +// short operations. This measures commit/cache maintenance, not navigation. +for (const family of ['snapshots', 'mixed flights', 'preload flights']) { + describe.each([0, 4, 10, 100, 1000, 5000])( + `${family}: %i cached matches`, + (size) => { + for (const retainedFraction of [1, 0.5, 0]) { + const retainedCount = Math.floor(size * retainedFraction) + let aborts = 0 + const entries = Array.from({ length: size }, (_, index) => { + const retained = + retainedFraction === 1 || + (retainedFraction === 0.5 && index % 2 === 1) + const hasFlight = + family === 'preload flights' || + (family === 'mixed flights' && index % 10 === 0) + // Keep loader scheduling and AbortController allocation outside this + // ownership benchmark. Real abort events are covered in the unit tests. + const flight: LoaderFlight | undefined = hasFlight + ? [ + Promise.resolve([0, undefined]), + { + abort: () => { + aborts++ + }, + } as AbortController, + 1, + ] + : undefined + const match = { + id: String(index), + routeId: retained ? 'retained' : 'expired', + status: 'success', + updatedAt: 0, + _flight: flight, + } as Match + return { match, flight, retained } + }) + const cached = new Map(entries.map(({ match }) => [match.id, match])) + const departing = entries.filter( + (entry) => !entry.retained && entry.flight, + ) + const matches = Array.from({ length: 4 }, (_, index) => ({ + id: `active-${index}`, + routeId: 'retained', + status: 'success', + })) as Array + const tx = [ + new AbortController(), + 0, + undefined, + [], + 0, + Promise.resolve(), + ] as unknown as LoadTransaction + const router = { + _tx: tx, + _committed: matches, + _cache: cached, + options: {}, + routesById: { + retained: { options: { loader: () => {}, gcTime: Infinity } }, + expired: { options: { loader: () => {}, gcTime: 0 } }, + }, + stores: { setMatches: () => {} }, + } as unknown as AnyRouter + const run = () => { + router._cache = cached + for (const entry of departing) { + entry.match._flight = entry.flight + entry.flight![2] = 1 + } + commitMatches(router, tx, matches) + } + run() + expect(router._cache.size).toBe(retainedCount) + expect(aborts).toBe(departing.length) + for (const entry of entries) { + if (entry.retained) { + expect(router._cache.get(entry.match.id)).toBe(entry.match) + expect(entry.match._flight).toBe(entry.flight) + if (entry.flight) { + expect(entry.flight[2]).toBe(1) + } + } else { + expect(entry.match._flight).toBeUndefined() + } + } + const batch = size <= 100 ? 100 : 1 + bench( + `${retainedFraction * 100}% retained (${batch} commits)`, + () => { + for (let index = 0; index < batch; index++) { + run() + } + }, + { time: 500, warmupTime: 100 }, + ) + } + }, + ) +} diff --git a/packages/router-core/tests/cache-commit.test.ts b/packages/router-core/tests/cache-commit.test.ts new file mode 100644 index 00000000000..63505fa365d --- /dev/null +++ b/packages/router-core/tests/cache-commit.test.ts @@ -0,0 +1,216 @@ +import { describe, expect, test, vi } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute } from '../src' +import { commitMatches } from '../src/load-client' +import { createTestRouter } from './routerTestUtils' +import type { AnyRouteMatch, AnyRouter } from '../src' +import type { LoaderFlight, LoadTransaction } from '../src/load-client' + +type Match = AnyRouteMatch & { _flight?: LoaderFlight } + +function resource(leases = 1): LoaderFlight { + return [Promise.resolve([0, undefined]), new AbortController(), leases] +} + +function match(id: string, flight?: LoaderFlight): Match { + return { + id, + routeId: '/items/$id', + status: 'success', + updatedAt: 0, + _flight: flight, + } as Match +} + +function setup( + previous: Array, + cached: Array, + next = [match('next')], +) { + const tx = [ + new AbortController(), + 0, + undefined, + next, + 0, + Promise.resolve(), + ] as unknown as LoadTransaction + const publish = vi.fn() + const router = { + _tx: tx, + _committed: previous, + _cache: new Map(cached.map((entry) => [entry.id, entry])), + _flights: new Map( + [...previous, ...cached, ...next] + .filter((entry) => entry._flight) + .map((entry) => [entry.id, entry._flight!]), + ), + options: {}, + routesById: { + '/items/$id': { + options: { + loader: () => {}, + gcTime: Infinity, + preloadGcTime: Infinity, + }, + }, + }, + stores: { setMatches: publish }, + } as unknown as AnyRouter + return { + router, + publish, + tx, + next, + commit: () => commitMatches(router, tx, next), + } +} + +describe('commit cache ownership', () => { + test('retains cached identities and releases the committed owner cloned into cache', () => { + const retainedFlight = resource() + const departedFlight = resource() + const retained = match('retained', retainedFlight) + const departed = match('departed', departedFlight) + const { router, commit } = setup([departed], [retained]) + + commit() + + expect(router._cache.get('retained')).toBe(retained) + expect(retained._flight).toBe(retainedFlight) + expect(retainedFlight[2]).toBe(1) + expect(retainedFlight[1].signal.aborted).toBe(false) + expect(router._cache.get('departed')).not.toBe(departed) + expect((router._cache.get('departed') as Match)._flight).toBeUndefined() + expect(departed._flight).toBeUndefined() + expect(departedFlight[1].signal.aborted).toBe(true) + }) + + test('keeps a retained generation alive when a departing generation shares its flight', () => { + const shared = resource(2) + const departed = match('same', shared) + const retained = match('same', shared) + const { router, commit } = setup([departed], [retained]) + + commit() + + expect(router._cache.get('same')).toBe(retained) + expect(departed._flight).toBeUndefined() + expect(retained._flight).toBe(shared) + expect(shared[2]).toBe(1) + expect(shared[1].signal.aborted).toBe(false) + }) + + test('releases the old same-id generation without releasing its replacement', () => { + const oldFlight = resource() + const newFlight = resource() + const old = match('same', oldFlight) + const replacement = match('same', newFlight) + const { router, commit } = setup([], [old], [replacement]) + + commit() + + expect(router._cache.has('same')).toBe(false) + expect(old._flight).toBeUndefined() + expect(oldFlight[1].signal.aborted).toBe(true) + expect(replacement._flight).toBe(newFlight) + expect(newFlight[1].signal.aborted).toBe(false) + expect(router._flights?.get('same')).toBe(newFlight) + }) + + test('detaches all expired owners before abort listeners run, including duplicate objects', () => { + const shared = resource(2) + const first = match('first', shared) + const second = match('second', shared) + first.status = second.status = 'error' + const { router, tx, next, commit } = setup([first], [first, second]) + const onAbort = vi.fn(() => { + expect(first._flight).toBeUndefined() + expect(second._flight).toBeUndefined() + expect(router._cache.size).toBe(0) + expect(router._committed).toBe(next) + expect(tx[3]).toEqual([]) + }) + shared[1].signal.addEventListener('abort', onAbort) + + commit() + + expect(shared[2]).toBe(0) + expect(onAbort).toHaveBeenCalledOnce() + }) + + test('uses cache contents after synchronous publication mutation', () => { + const retainedFlight = resource() + const retained = match('retained', retainedFlight) + const { router, publish, commit } = setup([], [retained]) + publish.mockImplementation(() => router._cache.delete('retained')) + + commit() + + expect(retained._flight).toBeUndefined() + expect(retainedFlight[1].signal.aborted).toBe(true) + }) + + test('uses the captured cache if publication replaces the router cache', () => { + const retainedFlight = resource() + const retained = match('retained', retainedFlight) + const { router, publish, commit } = setup([], [retained]) + publish.mockImplementation(() => { + router._cache = new Map() + }) + + commit() + + expect(router._cache.size).toBe(0) + expect(retained._flight).toBe(retainedFlight) + expect(retainedFlight[1].signal.aborted).toBe(false) + }) +}) + +test('an unrelated navigation retains a fresh preload flight and evicts an expired one', async () => { + let freshSignal: AbortSignal | undefined + let expiredSignal: AbortSignal | undefined + const root = new BaseRootRoute({}) + const home = new BaseRoute({ getParentRoute: () => root, path: '/' }) + const other = new BaseRoute({ getParentRoute: () => root, path: '/other' }) + const freshLoader = vi.fn( + ({ abortController }: { abortController: AbortController }) => { + freshSignal = abortController.signal + return 'fresh data' + }, + ) + const fresh = new BaseRoute({ + getParentRoute: () => root, + path: '/fresh', + loader: freshLoader, + staleTime: Infinity, + preloadStaleTime: Infinity, + preloadGcTime: Infinity, + }) + const expired = new BaseRoute({ + getParentRoute: () => root, + path: '/expired', + loader: ({ abortController }) => { + expiredSignal = abortController.signal + return 'expired data' + }, + preloadGcTime: 0, + }) + const router = createTestRouter({ + routeTree: root.addChildren([home, other, fresh, expired]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + await router.load() + await router.preloadRoute({ to: '/fresh' }) + await router.preloadRoute({ to: '/expired' }) + expect(freshSignal?.aborted).toBe(false) + expect(expiredSignal?.aborted).toBe(false) + + await router.navigate({ to: '/other' }) + + expect(freshSignal?.aborted).toBe(false) + expect(expiredSignal?.aborted).toBe(true) + await router.navigate({ to: '/fresh' }) + expect(freshLoader).toHaveBeenCalledOnce() + expect(router.state.matches.at(-1)?.loaderData).toBe('fresh data') +}) diff --git a/packages/router-core/tests/cache-navigation.bench.ts b/packages/router-core/tests/cache-navigation.bench.ts new file mode 100644 index 00000000000..16650d7c4b4 --- /dev/null +++ b/packages/router-core/tests/cache-navigation.bench.ts @@ -0,0 +1,50 @@ +import { bench, describe, expect } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute } from '../src' +import { createTestRouter } from './routerTestUtils' + +// A public-API cross-check for the isolated commit benchmark. Preload seeding +// is outside the timed loop; replace navigations keep history size stationary. +for (const size of [0, 100, 1000, 5000]) { + const root = new BaseRootRoute({}) + const home = new BaseRoute({ getParentRoute: () => root, path: '/' }) + const other = new BaseRoute({ getParentRoute: () => root, path: '/other' }) + let loads = 0 + const item = new BaseRoute({ + getParentRoute: () => root, + path: '/items/$id', + loader: ({ params }) => { + loads++ + return params.id + }, + staleTime: Infinity, + preloadStaleTime: Infinity, + preloadGcTime: Infinity, + gcTime: Infinity, + }) + const router = createTestRouter({ + routeTree: root.addChildren([home, other, item]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + await router.load() + for (let index = 0; index < size; index++) { + await router.preloadRoute({ + to: '/items/$id', + params: { id: String(index) }, + }) + } + const navigate = async () => { + await router.navigate({ to: '/other', replace: true }) + await router.navigate({ to: '/', replace: true }) + } + await navigate() + expect(loads).toBe(size) + expect(router._cache.size).toBe(size) + expect(router.state.location.pathname).toBe('/') + describe(`${size} cached preloads`, () => { + bench('two unrelated navigations', navigate, { + time: 1000, + warmupTime: 200, + }) + }) +}