fix(renderer): release retired store snapshots from selector caches (#23187)

Co-authored-by: m4air <m4air@m4airs-Air.localdomain>
This commit is contained in:
OrcaWin
2026-09-26 12:46:48 -07:00
committed by GitHub
co-authored by m4air
parent a85057ef48
commit f4557bfef1
4 changed files with 65 additions and 5 deletions
@@ -56,6 +56,39 @@ function review(number: number): HostedReviewInfo {
}
describe('parent PR checks projection selector', () => {
it.each([false, true])(
'releases unrelated store data while keeping the projection cached (replacement=%s)',
async (replacement) => {
if (typeof globalThis.gc !== 'function') {
throw new Error('Run with the repository Vitest --expose-gc config')
}
const select = createParentPrChecksProjectionSelector({
worktrees: [worktree(0)],
repos: [repo()],
settings: null,
refreshOutcomes: new Map()
})
if (replacement) {
select({ hostedReviewCache: {}, prCache: {}, checksCache: {} })
}
const caches = { hostedReviewCache: {}, prCache: {}, checksCache: {} }
function selectRetiredState(): WeakRef<object> {
const unrelatedData = { content: new Uint8Array(1024 * 1024) }
const state = { ...caches, unrelatedData }
select(state)
return new WeakRef(unrelatedData)
}
const retired = selectRetiredState()
const projection = select(caches)
for (let round = 0; round < 3; round++) {
await new Promise<void>((resolve) => setImmediate(resolve))
globalThis.gc()
}
expect(retired.deref()).toBeUndefined()
expect(select(caches)).toBe(projection)
}
)
it('does not inspect tracked keys when cache map references are unchanged', () => {
const cacheRead = vi.fn()
const observedCache = new Proxy(
@@ -18,6 +18,15 @@ type CacheDependency = {
value: unknown
}
function selectCacheReferences(state: ReviewCacheState): ReviewCacheState {
// Pick narrows the type, not the object received from the store.
return {
hostedReviewCache: state.hostedReviewCache,
prCache: state.prCache,
checksCache: state.checksCache
}
}
function trackCacheReads<K extends ReviewCacheName>(
state: ReviewCacheState,
cacheName: K,
@@ -74,7 +83,7 @@ export function createParentPrChecksProjectionSelector(
}
if (dependenciesAreCurrent(state, cached.cacheReferences, cached.dependencies)) {
// Why: adopting unrelated replacement maps keeps later store notifications O(1).
cached.cacheReferences = state
cached.cacheReferences = selectCacheReferences(state)
return cached.projection
}
}
@@ -88,7 +97,7 @@ export function createParentPrChecksProjectionSelector(
prCache: trackCacheReads(state, 'prCache', dependencies),
checksCache: trackCacheReads(state, 'checksCache', dependencies)
})
cached = { cacheReferences: state, dependencies, projection }
cached = { cacheReferences: selectCacheReferences(state), dependencies, projection }
return projection
}
}
@@ -54,6 +54,23 @@ beforeEach(() => {
})
describe('selectTerminalPaneHostState memo', () => {
it('releases the last store snapshot when no terminal selects again', async () => {
if (typeof globalThis.gc !== 'function') {
throw new Error('Run with the repository Vitest --expose-gc config')
}
function selectRetiredState(): WeakRef<AppState> {
const state = makeState()
selectTerminalPaneHostState(state, LOCAL_WORKTREE)
return new WeakRef(state)
}
const retired = selectRetiredState()
for (let round = 0; round < 3; round++) {
await new Promise<void>((resolve) => setImmediate(resolve))
globalThis.gc()
}
expect(retired.deref()).toBeUndefined()
})
it('returns the same object across an unrelated store write', () => {
const first = makeState()
const before = selectTerminalPaneHostState(first, LOCAL_WORKTREE)
@@ -78,7 +78,8 @@ function isSameHostState(a: TerminalPaneHostState, b: TerminalPaneHostState): bo
)
}
let cachedState: AppState | null = null
// Identity checks must not keep a retired store alive after the last pane unmounts.
let cachedState: WeakRef<AppState> | null = null
let cachedByWorktreeId = new Map<string, TerminalPaneHostState>()
let previousByWorktreeId = new Map<string, TerminalPaneHostState>()
@@ -100,10 +101,10 @@ export function selectTerminalPaneHostState(
state: AppState,
worktreeId: string
): TerminalPaneHostState {
if (state !== cachedState) {
if (state !== cachedState?.deref()) {
previousByWorktreeId = cachedByWorktreeId
cachedByWorktreeId = new Map()
cachedState = state
cachedState = new WeakRef(state)
}
const cached = cachedByWorktreeId.get(worktreeId)
if (cached) {