From bf1e1b9004d38a37f2c1677802dc5a45904c19cf Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Wed, 9 Sep 2026 03:23:38 -0700 Subject: [PATCH] perf: probe requested pane keys instead of enumerating records (#19494) * perf: probe requested pane keys instead of enumerating records * perf(agent-status): drop the requested-key array from pane removal Probing the pane keys still beat enumerating the record, but materializing the requested set allocated on every call including the common no-match path, where a dozen records are swept per retirement. Copy lazily on first match instead, and cover the set-disagreement and prototype-key cases. --------- Co-authored-by: m4air Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> --- .../agent-status-pane-keyed-records.test.ts | 49 ++++++++++++++++++- .../slices/agent-status-pane-keyed-records.ts | 19 ++++--- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts b/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts index 5b327561637..02d0e9e56ff 100644 --- a/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts +++ b/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts @@ -3,7 +3,8 @@ import { RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX, RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX, boundRecentlyClosedAgentStatusTabIds, - boundRecentlyRetiredAgentStatusPaneKeys + boundRecentlyRetiredAgentStatusPaneKeys, + removePaneKeys } from './agent-status-pane-keyed-records' function keyRecord(keys: readonly string[]): Record { @@ -108,3 +109,49 @@ describe('boundRecentlyClosedAgentStatusTabIds', () => { expect(next.fresh).toBe(true) }) }) + +it('does not enumerate unrelated pane records when removing absent keys', () => { + let enumerations = 0 + const record = new Proxy( + Object.fromEntries(Array.from({ length: 1000 }, (_, index) => [`tab-${index}:leaf`, index])), + { + ownKeys(target) { + enumerations += 1 + return Reflect.ownKeys(target) + } + } + ) + for (let index = 0; index < 200; index += 1) { + expect(removePaneKeys(record, new Set(['absent:leaf']))).toBe(record) + } + expect(enumerations).toBe(0) +}) + +it('removes enumerable undefined values without removing inherited or hidden keys', () => { + const record = { visible: undefined } + Object.defineProperty(record, 'hidden', { value: 1, enumerable: false }) + expect(removePaneKeys(record, new Set(['hidden', 'toString']))).toBe(record) + expect(Object.hasOwn(removePaneKeys(record, new Set(['visible'])), 'visible')).toBe(false) +}) + +// Probing the requested keys only matches the old record-enumeration when the two sets +// disagree in both directions, and the store relies on the unchanged case staying identical. +it('drops every requested key that is present and keeps the reference when none are', () => { + const base = { a: 1, b: 2, c: 3 } + expect(removePaneKeys({ ...base }, new Set(['a', 'b', 'c', 'd', 'e']))).toEqual({}) + expect(removePaneKeys({ ...base }, new Set(['b', 'missing']))).toEqual({ a: 1, c: 3 }) + const disjoint = { ...base } + expect(removePaneKeys(disjoint, new Set(['x', 'y']))).toBe(disjoint) + const noKeys = { ...base } + expect(removePaneKeys(noKeys, new Set())).toBe(noKeys) + const empty = {} + expect(removePaneKeys(empty, new Set(['a']))).toBe(empty) +}) + +it('leaves surviving keys in their original order and does not pollute the prototype', () => { + const record: Record = { z: 1, y: 2, x: 3, w: 4 } + expect(Object.keys(removePaneKeys(record, new Set(['x', 'z'])))).toEqual(['y', 'w']) + const plain = { safe: 1 } + expect(removePaneKeys(plain, new Set(['__proto__', 'constructor']))).toBe(plain) + expect(Object.getPrototypeOf(plain)).toBe(Object.prototype) +}) diff --git a/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts b/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts index f9199140c07..3acf5258f2c 100644 --- a/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts +++ b/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts @@ -83,15 +83,20 @@ export function removePaneKeys( record: Record, paneKeys: ReadonlySet ): Record { - const matchingKeys = Object.keys(record).filter((key) => paneKeys.has(key)) - if (matchingKeys.length === 0) { - return record - } - const next = { ...record } - for (const key of matchingKeys) { + // Probe the requested keys instead of enumerating the record, and copy only once a key + // actually matches: pane retirement calls this across a dozen records that usually hold + // none of the retired keys, so the no-op path must stay allocation-free and keep returning + // the same reference. `propertyIsEnumerable` is own-only, so inherited keys such as + // `toString` or `__proto__` are never deletable. + let next: Record | null = null + for (const key of paneKeys) { + if (!Object.prototype.propertyIsEnumerable.call(record, key)) { + continue + } + next ??= { ...record } delete next[key] } - return next + return next ?? record } export function removePaneKeysByTabPrefix(