From 5408439e07cc0d661973cd5faccc87203c31bb71 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:36:40 -0700 Subject: [PATCH] perf(renderer): skip the resume cache sweep publication when nothing changed Every eviction helper already returns its input reference untouched when it evicts nothing, so the opening sweep in refreshAllGitHub can compare identities and return the previous state. Window resume then stops waking every store subscriber on a no-op sweep. Folds in the remaining unique optimization from #13711, which this PR supersedes on the lookup-indexing side. Co-authored-by: Orca --- src/renderer/src/store/slices/github.test.ts | 40 ++++++++++++++++++++ src/renderer/src/store/slices/github.ts | 31 ++++++++++----- 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index 8b19eb6993e..4b7d19ad284 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -5428,6 +5428,46 @@ describe('createGitHubSlice.refreshAllGitHub', () => { resetRemoteRuntimeMocks() }) + it('publishes no store update when the cache sweep changes nothing', () => { + const store = createTestStore() + store.setState({ + repos: [{ id: 'repo-1', path: '/repo', name: 'repo', kind: 'git' }], + groupBy: 'repo', + worktreeCardProperties: ['comment'], + rightSidebarOpen: false, + worktreesByRepo: { 'repo-1': [makePRRefreshWorktree()] } + } as unknown as Partial) + let publications = 0 + const unsubscribe = store.subscribe(() => { + publications += 1 + }) + + store.getState().refreshAllGitHub() + unsubscribe() + + expect(publications).toBe(0) + }) + + it('still clears populated comments when no workspace refresh is needed', () => { + const store = createTestStore() + store.setState({ + commentsCache: { cached: { data: [], fetchedAt: 1 } }, + groupBy: 'repo', + worktreeCardProperties: ['comment'], + rightSidebarOpen: false + } as unknown as Partial) + let publications = 0 + const unsubscribe = store.subscribe(() => { + publications += 1 + }) + + store.getState().refreshAllGitHub() + unsubscribe() + + expect(store.getState().commentsCache).toEqual({}) + expect(publications).toBe(1) + }) + it('skips repo and worktree identity reads when no GitHub decoration is visible', () => { const store = createTestStore() const repoId = 'repo-idle' diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 97c0f3c7b31..5b16b72796c 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -4433,15 +4433,28 @@ export const createGitHubSlice: StateCreator = (s refreshAllGitHub: () => { // Clear comments cache; evict stale entries to bound long-session growth across repos/branches. - set((s) => ({ - commentsCache: {}, - prCache: evictStaleEntries(s.prCache), - issueCache: evictStaleEntries(s.issueCache), - checksCache: evictStaleEntries(s.checksCache), - workItemsCache: evictStaleEntries(s.workItemsCache), - projectViewCache: evictStaleEntries(s.projectViewCache), - prRefreshStates: pruneExpiredPRRefreshStates(s.prRefreshStates) - })) + set((s) => { + const next = { + commentsCache: Object.keys(s.commentsCache).length === 0 ? s.commentsCache : {}, + prCache: evictStaleEntries(s.prCache), + issueCache: evictStaleEntries(s.issueCache), + checksCache: evictStaleEntries(s.checksCache), + workItemsCache: evictStaleEntries(s.workItemsCache), + projectViewCache: evictStaleEntries(s.projectViewCache), + prRefreshStates: pruneExpiredPRRefreshStates(s.prRefreshStates) + } + // Why: each eviction helper returns its input untouched when nothing changed, so an + // unchanged sweep can return `s` and avoid waking every subscriber on window resume. + return next.commentsCache === s.commentsCache && + next.prCache === s.prCache && + next.issueCache === s.issueCache && + next.checksCache === s.checksCache && + next.workItemsCache === s.workItemsCache && + next.projectViewCache === s.projectViewCache && + next.prRefreshStates === s.prRefreshStates + ? s + : next + }) // Why: don't prune prRequestGenerations here — deleting a live generation makes its response look stale.