mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
perf(renderer): skip runtime status map churn on unchanged re-probes (#13771)
setRuntimeEnvironmentStatus always cloned runtimeStatusByEnvironmentId, so a re-probe returning an identical status still invalidated every subscriber. It now returns the state unchanged when the computed entry matches the stored one. checkedAt is excluded from the comparison: it is Date.now() per probe and no consumer reads it, so including it would make the guard dead code. Generation advance and the disconnect toasts still run on every call. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -0,0 +1,12 @@
|
||||
import type { RuntimeEnvironmentStatus } from './runtime-status'
|
||||
import { persistedUIValuesEqual } from '../../../../shared/persisted-ui-equality'
|
||||
|
||||
// Why: `checkedAt` is dropped — no consumer reads it, and a per-probe timestamp
|
||||
// would make every entry unequal and defeat the caller's identity guard.
|
||||
// Everything else, including fields added later, is compared structurally.
|
||||
export function runtimeEnvironmentStatusesEqual(
|
||||
{ checkedAt: _leftCheckedAt, ...left }: RuntimeEnvironmentStatus,
|
||||
{ checkedAt: _rightCheckedAt, ...right }: RuntimeEnvironmentStatus
|
||||
): boolean {
|
||||
return persistedUIValuesEqual(left, right)
|
||||
}
|
||||
@@ -201,6 +201,110 @@ describe('runtime-status slice', () => {
|
||||
).toBe('client-device')
|
||||
})
|
||||
|
||||
it('still learns a paired device id when the re-probed status is unchanged', () => {
|
||||
const store = createSliceStore()
|
||||
// Why: the status arrives before the catalog, so only the second (identical) probe
|
||||
// can carry the id onto the environment — the map guard must not swallow it.
|
||||
const status = makeStatus({ pairedDeviceId: 'client-device' })
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', { status, checkedAt: 1 })
|
||||
store.getState().setRuntimeEnvironments([makeEnvironment()])
|
||||
const before = store.getState().runtimeStatusByEnvironmentId
|
||||
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', { status, checkedAt: 2 })
|
||||
|
||||
expect(store.getState().runtimeEnvironments[0].pairedDeviceId).toBe('client-device')
|
||||
expect(store.getState().runtimeStatusByEnvironmentId).toBe(before)
|
||||
})
|
||||
|
||||
it('keeps the map reference when a re-probe returns an identical status', () => {
|
||||
const store = createSliceStore()
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 })
|
||||
const before = store.getState().runtimeStatusByEnvironmentId
|
||||
const entryBefore = before.get('env-a')
|
||||
const generationBefore = getRuntimeEnvironmentConnectionGeneration('env-a')
|
||||
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 2 })
|
||||
|
||||
expect(store.getState().runtimeStatusByEnvironmentId).toBe(before)
|
||||
expect(store.getState().runtimeStatusByEnvironmentId.get('env-a')).toBe(entryBefore)
|
||||
expect(getRuntimeEnvironmentConnectionGeneration('env-a')).toBe(generationBefore)
|
||||
})
|
||||
|
||||
it('keeps the map reference when an unreachable server is re-probed', () => {
|
||||
const store = createSliceStore()
|
||||
store.setState({ runtimeEnvironments: [makeEnvironment()] })
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 1 })
|
||||
const before = store.getState().runtimeStatusByEnvironmentId
|
||||
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 2 })
|
||||
|
||||
expect(store.getState().runtimeStatusByEnvironmentId).toBe(before)
|
||||
expect(toast.warning).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('replaces the map when any status field changes, nested ones included', () => {
|
||||
const store = createSliceStore()
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', {
|
||||
status: makeStatus({ liveTabCount: 1, capabilities: ['a'] }),
|
||||
checkedAt: 1
|
||||
})
|
||||
const afterFirst = store.getState().runtimeStatusByEnvironmentId
|
||||
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', {
|
||||
status: makeStatus({ liveTabCount: 2, capabilities: ['a'] }),
|
||||
checkedAt: 2
|
||||
})
|
||||
const afterScalarChange = store.getState().runtimeStatusByEnvironmentId
|
||||
expect(afterScalarChange).not.toBe(afterFirst)
|
||||
expect(afterScalarChange.get('env-a')?.checkedAt).toBe(2)
|
||||
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', {
|
||||
status: makeStatus({ liveTabCount: 2, capabilities: ['a', 'b'] }),
|
||||
checkedAt: 3
|
||||
})
|
||||
const afterNestedChange = store.getState().runtimeStatusByEnvironmentId
|
||||
expect(afterNestedChange).not.toBe(afterScalarChange)
|
||||
expect(afterNestedChange.get('env-a')?.status?.capabilities).toEqual(['a', 'b'])
|
||||
})
|
||||
|
||||
it('still writes and toasts across null and non-null transitions', () => {
|
||||
const store = createSliceStore()
|
||||
store.setState({ runtimeEnvironments: [makeEnvironment()] })
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 })
|
||||
const connected = store.getState().runtimeStatusByEnvironmentId
|
||||
const generationConnected = getRuntimeEnvironmentConnectionGeneration('env-a')
|
||||
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 2 })
|
||||
const disconnected = store.getState().runtimeStatusByEnvironmentId
|
||||
expect(disconnected).not.toBe(connected)
|
||||
expect(disconnected.get('env-a')?.status).toBeNull()
|
||||
expect(toast.warning).toHaveBeenCalledTimes(1)
|
||||
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 3 })
|
||||
const reconnected = store.getState().runtimeStatusByEnvironmentId
|
||||
expect(reconnected).not.toBe(disconnected)
|
||||
expect(getRuntimeEnvironmentConnectionGeneration('env-a')).toBe(generationConnected + 1)
|
||||
expect(reconnected.get('env-a')?.connectionGeneration).toBe(generationConnected + 1)
|
||||
expect(toast.dismiss).toHaveBeenCalledWith('runtime-environment-disconnected:env-a')
|
||||
})
|
||||
|
||||
it('replaces the map when the same status arrives under a new runtime id', () => {
|
||||
const store = createSliceStore()
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', {
|
||||
status: makeStatus({ runtimeId: 'runtime-a' }),
|
||||
checkedAt: 1
|
||||
})
|
||||
const before = store.getState().runtimeStatusByEnvironmentId
|
||||
|
||||
store.getState().setRuntimeEnvironmentStatus('env-a', {
|
||||
status: makeStatus({ runtimeId: 'runtime-b' }),
|
||||
checkedAt: 2
|
||||
})
|
||||
|
||||
expect(store.getState().runtimeStatusByEnvironmentId).not.toBe(before)
|
||||
expect(store.getState().runtimeStatusByEnvironmentId.get('env-a')?.connectionGeneration).toBe(2)
|
||||
})
|
||||
|
||||
it('does not toast when the first probe finds a saved server offline', () => {
|
||||
const store = createSliceStore()
|
||||
store.setState({ runtimeEnvironments: [makeEnvironment()] })
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { StateCreator } from 'zustand'
|
||||
import type { AppState } from '../types'
|
||||
import type { PublicKnownRuntimeEnvironment } from '../../../../shared/runtime-environments'
|
||||
import type { RuntimeStatus } from '../../../../shared/runtime-types'
|
||||
import { runtimeEnvironmentStatusesEqual } from './runtime-environment-status-equality'
|
||||
import {
|
||||
clearRecentRuntimeCompatibilityFailure,
|
||||
clearRuntimeCompatibilityCache
|
||||
@@ -21,6 +22,8 @@ import { refreshRuntimeEnvironmentStatus } from './runtime-status-refresh'
|
||||
export type RuntimeEnvironmentStatus = {
|
||||
status: RuntimeStatus | null
|
||||
appVersion?: string | null
|
||||
/** When the stored status was last *observed to change*; an unchanged re-probe
|
||||
* is dropped rather than rewritten, so this is not a probe-freshness clock. */
|
||||
checkedAt: number
|
||||
connectionGeneration?: number
|
||||
}
|
||||
@@ -186,7 +189,6 @@ export const createRuntimeStatusSlice: StateCreator<AppState, [], [], RuntimeSta
|
||||
clearRecentRuntimeCompatibilityFailure(environmentId, status.status)
|
||||
}
|
||||
set((s) => {
|
||||
const next = new Map(s.runtimeStatusByEnvironmentId)
|
||||
const sessionEnded = status.status === null && previous?.status != null
|
||||
const connectionChanged =
|
||||
status.status !== null &&
|
||||
@@ -200,10 +202,13 @@ export const createRuntimeStatusSlice: StateCreator<AppState, [], [], RuntimeSta
|
||||
if (activeEnvironmentId === environmentId && (sessionEnded || connectionChanged)) {
|
||||
bumpProviderRuntimeSessionGeneration()
|
||||
}
|
||||
next.set(environmentId, {
|
||||
...status,
|
||||
connectionGeneration
|
||||
})
|
||||
const nextEntry = { ...status, connectionGeneration }
|
||||
const currentEntry = s.runtimeStatusByEnvironmentId.get(environmentId)
|
||||
// Why: an unchanged re-probe must not invalidate every Map subscriber. Real
|
||||
// transitions change `status` or advance `connectionGeneration`, so they still write.
|
||||
const statusUnchanged = Boolean(
|
||||
currentEntry && runtimeEnvironmentStatusesEqual(currentEntry, nextEntry)
|
||||
)
|
||||
const environmentIndex = pairedDeviceId
|
||||
? s.runtimeEnvironments.findIndex((environment) => environment.id === environmentId)
|
||||
: -1
|
||||
@@ -214,9 +219,15 @@ export const createRuntimeStatusSlice: StateCreator<AppState, [], [], RuntimeSta
|
||||
index === environmentIndex ? { ...environment, pairedDeviceId } : environment
|
||||
)
|
||||
: s.runtimeEnvironments
|
||||
const environmentsChanged = runtimeEnvironments !== s.runtimeEnvironments
|
||||
if (statusUnchanged && !environmentsChanged) {
|
||||
return s
|
||||
}
|
||||
return {
|
||||
runtimeStatusByEnvironmentId: next,
|
||||
...(runtimeEnvironments !== s.runtimeEnvironments ? { runtimeEnvironments } : {})
|
||||
runtimeStatusByEnvironmentId: statusUnchanged
|
||||
? s.runtimeStatusByEnvironmentId
|
||||
: new Map(s.runtimeStatusByEnvironmentId).set(environmentId, nextEntry),
|
||||
...(environmentsChanged ? { runtimeEnvironments } : {})
|
||||
}
|
||||
})
|
||||
if (options?.suppressDisconnectToast) {
|
||||
|
||||
Reference in New Issue
Block a user