mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
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 <m4air@m4airs-MacBook-Air.local> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
@@ -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<string, true> {
|
||||
@@ -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<string, number> = { 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)
|
||||
})
|
||||
|
||||
@@ -83,15 +83,20 @@ export function removePaneKeys<T>(
|
||||
record: Record<string, T>,
|
||||
paneKeys: ReadonlySet<string>
|
||||
): Record<string, T> {
|
||||
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<string, T> | 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<T>(
|
||||
|
||||
Reference in New Issue
Block a user