From 4958534d21cef6317002a935e4ae64a418351b06 Mon Sep 17 00:00:00 2001 From: vrazuvaev Date: Fri, 27 Mar 2026 16:20:10 +0100 Subject: [PATCH 1/3] fix perf regression on auto eviction --- packages/apollo-forest-run/src/ForestRun.ts | 2 +- .../apollo-forest-run/src/ForestRunCompat.ts | 2 +- .../src/__tests__/eviction.test.ts | 16 +- .../src/__tests__/helpers/forest.ts | 1 + packages/apollo-forest-run/src/cache/read.ts | 2 +- packages/apollo-forest-run/src/cache/store.ts | 149 ++++++------------ packages/apollo-forest-run/src/cache/write.ts | 2 +- .../apollo-forest-run/src/forest/addTree.ts | 42 ++++- .../apollo-forest-run/src/forest/types.ts | 7 + .../src/forest/updateForest.ts | 2 +- 10 files changed, 115 insertions(+), 110 deletions(-) diff --git a/packages/apollo-forest-run/src/ForestRun.ts b/packages/apollo-forest-run/src/ForestRun.ts index 7743aa6d7..cae86828e 100644 --- a/packages/apollo-forest-run/src/ForestRun.ts +++ b/packages/apollo-forest-run/src/ForestRun.ts @@ -578,7 +578,7 @@ export class ForestRun< ); logUpdateStats(this.env, activeTransaction.changelog, watchesToNotify); } - maybeEvictOldData(this.env, this.store, activeTransaction); + maybeEvictOldData(this.env, this.store); return result as T; } diff --git a/packages/apollo-forest-run/src/ForestRunCompat.ts b/packages/apollo-forest-run/src/ForestRunCompat.ts index eab232d16..d42845977 100644 --- a/packages/apollo-forest-run/src/ForestRunCompat.ts +++ b/packages/apollo-forest-run/src/ForestRunCompat.ts @@ -47,7 +47,7 @@ export class ForestRunCompat extends ForestRun { ); const operationResult = { data: write.result ?? {} }; const tree = indexTree(this.env, operation, operationResult); - replaceTree(this.store.dataForest, tree); + replaceTree(this.env, this.store.dataForest, tree); } return this; } diff --git a/packages/apollo-forest-run/src/__tests__/eviction.test.ts b/packages/apollo-forest-run/src/__tests__/eviction.test.ts index 842bc7fc9..3f643248d 100644 --- a/packages/apollo-forest-run/src/__tests__/eviction.test.ts +++ b/packages/apollo-forest-run/src/__tests__/eviction.test.ts @@ -152,7 +152,10 @@ it("allows partitioned eviction", () => { maxOperationCount: 1, autoEvict: false, partitionConfig: { - partitionKey: (o) => o.operation.debugName, + partitionKey: (o) => + ["query a", "query b"].includes(o.operation.debugName) + ? o.operation.debugName + : null, partitions: { "query a": { maxOperationCount: 1 }, "query b": { maxOperationCount: 2 }, @@ -260,9 +263,12 @@ it("allows partitioned eviction", () => { }); it("should warn exactly once for the same warning", () => { + const calls: unknown[] = []; const mockConsole = { ...console, - warn: jest.fn(), + warn: (...args: unknown[]) => { + calls.push(args); + }, }; const cache = new ForestRun({ @@ -295,11 +301,11 @@ it("should warn exactly once for the same warning", () => { cache.gc(); - expect(mockConsole.warn).toHaveBeenCalledTimes(1); - expect(mockConsole.warn).toHaveBeenCalledWith( + expect(calls.length).toBe(1); + expect(calls[0]).toEqual([ "partition_not_configured", 'Partition "query TestA" is not configured in partitionConfig. Using default partition instead.', - ); + ]); // notably: "query TestB" is NOT logged }); diff --git a/packages/apollo-forest-run/src/__tests__/helpers/forest.ts b/packages/apollo-forest-run/src/__tests__/helpers/forest.ts index 1a8fc4807..4018d906c 100644 --- a/packages/apollo-forest-run/src/__tests__/helpers/forest.ts +++ b/packages/apollo-forest-run/src/__tests__/helpers/forest.ts @@ -36,6 +36,7 @@ export function createTestForest(): IndexedForest { operationsByNodes: new Map(), operationsWithErrors: new Set(), operationsByName: new Map(), + operationsByPartitions: new Map(), deletedNodes: new Set(), }; } diff --git a/packages/apollo-forest-run/src/cache/read.ts b/packages/apollo-forest-run/src/cache/read.ts index 7eccbdc09..627315d85 100644 --- a/packages/apollo-forest-run/src/cache/read.ts +++ b/packages/apollo-forest-run/src/cache/read.ts @@ -264,7 +264,7 @@ function growOutputTree( } if (!dataTree) { dataTree = growDataTree(env, forest, operation); - addTree(forest, dataTree); + addTree(env, forest, dataTree); } const tree = applyTransformations( env, diff --git a/packages/apollo-forest-run/src/cache/store.ts b/packages/apollo-forest-run/src/cache/store.ts index 916bf275c..1e84e2de7 100644 --- a/packages/apollo-forest-run/src/cache/store.ts +++ b/packages/apollo-forest-run/src/cache/store.ts @@ -3,8 +3,6 @@ import { DataForest, DataTree, OptimisticLayer, - ResolvedPartition, - ResolvedPartitionOptions, ResultTree, Store, Transaction, @@ -17,51 +15,20 @@ import { TypeName, } from "../descriptor/types"; import { assert } from "../jsutils/assert"; -import { IndexedTree } from "../forest/types"; +import { IndexedTree, DefaultPartition } from "../forest/types"; import { NodeChunk } from "../values/types"; import { operationCacheKey } from "./descriptor"; const EMPTY_ARRAY = Object.freeze([]); - -const DEFAULT_PARTITION = "__default__"; - -function resolvePartition( - env: CacheEnv, - store: Store, - tree: IndexedTree, -): ResolvedPartition { - const cached = store.partitions.get(tree); - if (cached !== undefined) { - return cached; - } - const { partitions, partitionKey } = env.partitionConfig; - const key = partitionKey(tree); - let partition: string; - let options: ResolvedPartitionOptions; - if (key != null && partitions[key]) { - partition = key; - options = partitions[key]; - } else { - partition = DEFAULT_PARTITION; - options = partitions[DEFAULT_PARTITION]; - if (key != null) { - env.logger?.warnOnce( - "partition_not_configured", - `Partition "${key}" is not configured in partitionConfig. Using default partition instead.`, - ); - } - } - const result: ResolvedPartition = { partition, options }; - store.partitions.set(tree, result); - return result; -} +const DEFAULT_PARTITION: DefaultPartition = "__default__"; export function createStore(_: CacheEnv): Store { const dataForest: DataForest = { trees: new Map(), operationsByNodes: new Map>(), - operationsWithErrors: new Set(), operationsByName: new Map(), + operationsByPartitions: new Map(), + operationsWithErrors: new Set(), extraRootIds: new Map(), layerTag: null, // not an optimistic layer readResults: new Map(), @@ -100,36 +67,38 @@ export function touchOperation( store.atime.set(operation.id, env.now()); } -function affectedPartitionsHaveAutoEvict( +function shouldAutoEvictAtLeastOnePartition( env: CacheEnv, store: Store, - transaction: Transaction, ): boolean { - for (const entry of transaction.changelog) { - if ("incoming" in entry) { - const { options } = resolvePartition(env, store, entry.incoming); - if (options.autoEvict) { - return true; - } + const partitions = env.partitionConfig.partitions; + + for (const [partition, ops] of store.dataForest.operationsByPartitions) { + const partitionConfig = + partitions[partition] ?? partitions[DEFAULT_PARTITION]; + + assert(partitionConfig); + + if ( + partitionConfig.autoEvict && + ops.size >= partitionConfig.maxOperationCount * 2 + ) { + return true; } } return false; } -export function maybeEvictOldData( - env: CacheEnv, - store: Store, - transaction: Transaction, -): void { +export function maybeEvictOldData(env: CacheEnv, store: Store): void { if (store.pendingEviction != null) { return; } - if (!affectedPartitionsHaveAutoEvict(env, store, transaction)) { + if (!shouldAutoEvictAtLeastOnePartition(env, store)) { return; } store.pendingEviction = env.scheduleAutoEviction(() => { store.pendingEviction = null; - evictOldData(env, store, true); + evictOldData(env, store); }); } @@ -138,55 +107,34 @@ export function cancelPendingEviction(store: Store): void { store.pendingEviction = null; } -export function evictOldData( - env: CacheEnv, - store: Store, - isAutoEvict?: boolean, -): OperationId[] { +export function evictOldData(env: CacheEnv, store: Store): OperationId[] { const { dataForest, atime } = store; - const partitionsOps = new Map< - string, - { options: ResolvedPartitionOptions; operations: OperationId[] } - >(); + const evictedIds: OperationId[] = []; + const partitions = env.partitionConfig?.partitions ?? {}; - for (const tree of dataForest.trees.values()) { - if (!canEvict(env, store, tree)) { - continue; // Skip non-evictable operations entirely - } - - const { partition, options } = resolvePartition(env, store, tree); - let partitionOps = partitionsOps.get(partition); - if (!partitionOps) { - partitionOps = { options, operations: [] }; - partitionsOps.set(partition, partitionOps); + // Process each partition + for (const [partition, ops] of dataForest.operationsByPartitions) { + if (!partitions[partition]) { + env.logger?.warnOnce( + "partition_not_configured", + `Partition "${partition}" is not configured in partitionConfig. Using default partition instead.`, + ); } - partitionOps.operations.push(tree.operation.id); - } + const partitionConfig = + partitions[partition] ?? partitions[DEFAULT_PARTITION]; - const toEvict: OperationId[] = []; + assert(partitionConfig); - // Process each partition - for (const { - options: partitionConf, - operations: evictableOperationIds, - } of partitionsOps.values()) { - // During auto-eviction, respect per-partition autoEvict settings - if (isAutoEvict) { - if (!partitionConf.autoEvict) { - continue; - } - } + const maxCount = partitionConfig.maxOperationCount; - const maxCount = partitionConf.maxOperationCount; - - if (!maxCount || evictableOperationIds.length <= maxCount) { + if (!maxCount || ops.size <= maxCount) { continue; // No eviction needed for this partition } // Sort by access time (LRU) and determine how many to evict - evictableOperationIds.sort( + const evictableOperationIds = [...ops].sort( (a, b) => (atime.get(a) ?? 0) - (atime.get(b) ?? 0), ); @@ -195,17 +143,20 @@ export function evictOldData( // Keep only the ones to evict without array copying w/ slice evictableOperationIds.length = evictCount; - toEvict.push(...evictableOperationIds); - } + // Remove evicted operations + for (const opId of evictableOperationIds) { + const tree = store.dataForest.trees.get(opId); + assert(tree); + if (!canEvict(env, store, tree)) { + continue; // Skip non-evictable operations entirely + } + removeDataTree(store, tree, partition); + } - // Remove evicted operations - for (const opId of toEvict) { - const tree = store.dataForest.trees.get(opId); - assert(tree); - removeDataTree(store, tree); + evictedIds.push(...evictableOperationIds); } - return toEvict; + return evictedIds; } function canEvict(env: CacheEnv, store: Store, resultTree: DataTree) { @@ -234,6 +185,7 @@ export function createOptimisticLayer( operationsByNodes: new Map(), operationsWithErrors: new Set(), operationsByName: new Map(), + operationsByPartitions: new Map(), extraRootIds: new Map(), readResults: new Map(), mutations: new Set(), @@ -394,11 +346,14 @@ function removeDataTree( atime, }: Store, { operation }: ResultTree, + partition: string, ) { assert(!watches.has(operation)); dataForest.trees.delete(operation.id); dataForest.readResults.delete(operation); dataForest.operationsWithErrors.delete(operation); + dataForest.operationsByName.get(operation.name ?? "")?.delete(operation.id); + dataForest.operationsByPartitions.get(partition)?.delete(operation.id); optimisticReadResults.delete(operation); partialReadResults.delete(operation); operations.get(operation.document)?.delete(operationCacheKey(operation)); diff --git a/packages/apollo-forest-run/src/cache/write.ts b/packages/apollo-forest-run/src/cache/write.ts index 4b8f49de3..c4cf7ee17 100644 --- a/packages/apollo-forest-run/src/cache/write.ts +++ b/packages/apollo-forest-run/src/cache/write.ts @@ -180,7 +180,7 @@ export function write( // Note: even with existingResult === undefined the tree for this operation may still exist in the cache // (when existingResult is resolved with a different key descriptor due to key variables) // TODO: replace with addTree and add a proper check for keyVariables - replaceTree(targetForest, modifiedIncomingResult); + replaceTree(env, targetForest, modifiedIncomingResult); } appendAffectedOperationsFromOtherLayers( diff --git a/packages/apollo-forest-run/src/forest/addTree.ts b/packages/apollo-forest-run/src/forest/addTree.ts index 500548878..bd006bcdd 100644 --- a/packages/apollo-forest-run/src/forest/addTree.ts +++ b/packages/apollo-forest-run/src/forest/addTree.ts @@ -1,24 +1,41 @@ -import { IndexedForest, IndexedTree } from "./types"; +import { + DefaultPartition, + ForestEnv, + IndexedForest, + IndexedTree, +} from "./types"; import { assert } from "../jsutils/assert"; import { getOrCreate } from "../jsutils/map"; const newSet = () => new Set(); -export function addTree(forest: IndexedForest, tree: IndexedTree) { +const DEFAULT_PARTITION: DefaultPartition = "__default__"; + +export function addTree( + env: ForestEnv, + forest: IndexedForest, + tree: IndexedTree, +) { const { trees } = forest; assert(!trees.has(tree.operation.id)); trees.set(tree.operation.id, tree); trackTreeNodes(forest, tree); trackOperationName(forest, tree); + trackPartitions(env, forest, tree); } -export function replaceTree(forest: IndexedForest, tree: IndexedTree) { +export function replaceTree( + env: ForestEnv, + forest: IndexedForest, + tree: IndexedTree, +) { const { trees } = forest; trees.set(tree.operation.id, tree); trackTreeNodes(forest, tree); trackOperationName(forest, tree); + trackPartitions(env, forest, tree); } export function trackTreeNodes(forest: IndexedForest, tree: IndexedTree) { @@ -34,3 +51,22 @@ function trackOperationName(forest: IndexedForest, tree: IndexedTree) { if (!name) return; getOrCreate(forest.operationsByName, name, newSet).add(tree.operation.id); } + +function trackPartitions( + env: ForestEnv, + forest: IndexedForest, + tree: IndexedTree, +) { + const partitionKey = + env.partitionConfig?.partitionKey(tree) ?? DEFAULT_PARTITION; + + const operationIds = getOrCreate( + forest.operationsByPartitions, + partitionKey, + newSet, + ); + if (tree.prev) { + operationIds.delete(tree.prev.operation.id); + } + operationIds.add(tree.operation.id); +} diff --git a/packages/apollo-forest-run/src/forest/types.ts b/packages/apollo-forest-run/src/forest/types.ts index 8f3de4954..e124740cb 100644 --- a/packages/apollo-forest-run/src/forest/types.ts +++ b/packages/apollo-forest-run/src/forest/types.ts @@ -125,6 +125,7 @@ export type IndexedForest = { operationsByNodes: Map>; // May contain false positives operationsWithErrors: Set; // May contain false positives operationsByName: Map>; // operationName → operation IDs + operationsByPartitions: Map>; // partition key => operation IDs deletedNodes: Set; }; @@ -238,4 +239,10 @@ export type ForestEnv = { // History feature flags historyConfig?: HistoryConfig; + + partitionConfig?: { + partitionKey: (operation: IndexedTree) => string | null; + }; }; + +export type DefaultPartition = "__default__"; diff --git a/packages/apollo-forest-run/src/forest/updateForest.ts b/packages/apollo-forest-run/src/forest/updateForest.ts index 3e9a5f7cb..afd77cb6e 100644 --- a/packages/apollo-forest-run/src/forest/updateForest.ts +++ b/packages/apollo-forest-run/src/forest/updateForest.ts @@ -59,9 +59,9 @@ export function updateAffectedTrees( ); } + replaceTree(env, forest, result.updatedTree); // Reset previous tree state on commit result.updatedTree.prev = null; - replaceTree(forest, result.updatedTree); } return allUpdated; } From cdaf4e772d7839180948b32e6208847366e31c6a Mon Sep 17 00:00:00 2001 From: vrazuvaev Date: Fri, 27 Mar 2026 16:53:48 +0100 Subject: [PATCH 2/3] Change files --- ...lo-forest-run-906abf62-5b94-4c5d-999d-2f429024c6f8.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@graphitation-apollo-forest-run-906abf62-5b94-4c5d-999d-2f429024c6f8.json diff --git a/change/@graphitation-apollo-forest-run-906abf62-5b94-4c5d-999d-2f429024c6f8.json b/change/@graphitation-apollo-forest-run-906abf62-5b94-4c5d-999d-2f429024c6f8.json new file mode 100644 index 000000000..699e43455 --- /dev/null +++ b/change/@graphitation-apollo-forest-run-906abf62-5b94-4c5d-999d-2f429024c6f8.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix perf regression on auto eviction", + "packageName": "@graphitation/apollo-forest-run", + "email": "vrazuvaev@microsoft.com_msteamsmdb", + "dependentChangeType": "patch" +} From 7c2a6efa9e9360e26e0b5902e0277cdbb0d463a1 Mon Sep 17 00:00:00 2001 From: vrazuvaev Date: Thu, 9 Apr 2026 13:14:59 +0200 Subject: [PATCH 3/3] fix(apollo-forest-run): skip autoEvict:false partitions during auto-eviction evictOldData now respects per-partition autoEvict flag when triggered automatically Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/__tests__/eviction.test.ts | 52 +++++++++++++++++++ packages/apollo-forest-run/src/cache/store.ts | 12 ++++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/packages/apollo-forest-run/src/__tests__/eviction.test.ts b/packages/apollo-forest-run/src/__tests__/eviction.test.ts index 3f643248d..b0e8b0800 100644 --- a/packages/apollo-forest-run/src/__tests__/eviction.test.ts +++ b/packages/apollo-forest-run/src/__tests__/eviction.test.ts @@ -511,6 +511,58 @@ it("gc() evicts all partitions regardless of per-partition autoEvict", () => { ).toEqual({ bar: 1 }); }); +it("auto-evicting partition B does not evict partition A with autoEvict: false", () => { + const cache = new ForestRun({ + maxOperationCount: 2, + autoEvict: true, + partitionConfig: { + partitionKey: (o) => o.operation.debugName, + partitions: { + "query a": { maxOperationCount: 2, autoEvict: false }, + "query b": { maxOperationCount: 1, autoEvict: true }, + }, + }, + }); + const a = gql` + query a($i: Int) { + foo(i: $i) + } + `; + const b = gql` + query b($i: Int) { + bar(i: $i) + } + `; + + // Fill partition A beyond maxOperationCount (but autoEvict: false) + cache.write({ query: a, variables: { i: 0 }, result: { foo: 0 } }); + cache.write({ query: a, variables: { i: 1 }, result: { foo: 1 } }); + cache.write({ query: a, variables: { i: 2 }, result: { foo: 2 } }); + + // Fill partition B to trigger auto-eviction (2 ops >= maxOperationCount * 2 = 2) + cache.write({ query: b, variables: { i: 0 }, result: { bar: 0 } }); + cache.write({ query: b, variables: { i: 1 }, result: { bar: 1 } }); + + // Partition A should NOT be evicted (autoEvict: false) + expect( + cache.read({ query: a, variables: { i: 0 }, optimistic: true }), + ).toEqual({ foo: 0 }); + expect( + cache.read({ query: a, variables: { i: 1 }, optimistic: true }), + ).toEqual({ foo: 1 }); + expect( + cache.read({ query: a, variables: { i: 2 }, optimistic: true }), + ).toEqual({ foo: 2 }); + + // Partition B should be auto-evicted (autoEvict: true, over threshold) + expect( + cache.read({ query: b, variables: { i: 0 }, optimistic: true }), + ).toEqual(null); + expect( + cache.read({ query: b, variables: { i: 1 }, optimistic: true }), + ).toEqual({ bar: 1 }); +}); + it("unconfigured operations fall to default partition and inherit global autoEvict", () => { // autoEvict: false only applies to __default__ partition and partitions // that don't specify their own autoEvict diff --git a/packages/apollo-forest-run/src/cache/store.ts b/packages/apollo-forest-run/src/cache/store.ts index 1e84e2de7..eec06f1da 100644 --- a/packages/apollo-forest-run/src/cache/store.ts +++ b/packages/apollo-forest-run/src/cache/store.ts @@ -98,7 +98,7 @@ export function maybeEvictOldData(env: CacheEnv, store: Store): void { } store.pendingEviction = env.scheduleAutoEviction(() => { store.pendingEviction = null; - evictOldData(env, store); + evictOldData(env, store, true); }); } @@ -107,7 +107,11 @@ export function cancelPendingEviction(store: Store): void { store.pendingEviction = null; } -export function evictOldData(env: CacheEnv, store: Store): OperationId[] { +export function evictOldData( + env: CacheEnv, + store: Store, + isAutoEvict?: boolean, +): OperationId[] { const { dataForest, atime } = store; const evictedIds: OperationId[] = []; @@ -127,6 +131,10 @@ export function evictOldData(env: CacheEnv, store: Store): OperationId[] { assert(partitionConfig); + if (isAutoEvict && !partitionConfig.autoEvict) { + continue; + } + const maxCount = partitionConfig.maxOperationCount; if (!maxCount || ops.size <= maxCount) {