mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
perf(ui-tui): shimmer loaders share one clock and stop after a bounded period
Review on #20379, finding 5 (Perf). Every ShimmerRows mounted its own 90 ms setInterval — the session panel can show lazy skills AND lazy tools at once, and a lazy watch session stays lazy indefinitely, so an otherwise- idle TUI ran ~22 React state updates per second forever. All shimmer compositions now subscribe to a single module-level clock: one interval regardless of how many skeletons are on screen, updates delivered in one timer callback so React batches them into a single render pass, and the interval is torn down with the last subscriber. Each mount's animation is also bounded (SHIMMER_ANIMATE_MS, 30 s): after the budget the skeleton freezes in place — it still reads as "loading" — and stops costing renders entirely. Tests: fake-timer coverage that N subscribers share one timer in lockstep, the interval stops with the last unsubscribe, and a late subscriber restarts the clock cleanly.
This commit is contained in:
parent
9fb2a63db8
commit
cd24efec61
2 changed files with 130 additions and 9 deletions
|
|
@ -1,8 +1,8 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { renderToScreen } from '../../packages/hermes-ink/src/ink/render-to-screen.js'
|
||||
import { cellAtIndex } from '../../packages/hermes-ink/src/ink/screen.js'
|
||||
import { ShimmerRows, shimmerSegments } from '../components/loaders.js'
|
||||
import { ShimmerRows, shimmerSegments, subscribeShimmerClock } from '../components/loaders.js'
|
||||
|
||||
describe('ShimmerRows leniency (agent-authored calls)', () => {
|
||||
it('accepts a bare row COUNT and derives widths — the generated-code shape', async () => {
|
||||
|
|
@ -50,3 +50,64 @@ describe('shimmerSegments', () => {
|
|||
expect(pre + band + post).toBe(10)
|
||||
})
|
||||
})
|
||||
|
||||
// Review on #20379 (finding 5): independent 90 ms intervals per shimmer
|
||||
// composition meant an idle TUI with two lazy sections repainted ~22x/sec
|
||||
// forever. All compositions now share ONE clock, and the interval exists
|
||||
// only while subscribers do.
|
||||
describe('subscribeShimmerClock', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('drives any number of subscribers from a single interval', () => {
|
||||
const a: number[] = []
|
||||
const b: number[] = []
|
||||
|
||||
const timersBefore = vi.getTimerCount()
|
||||
const unsubA = subscribeShimmerClock(p => a.push(p))
|
||||
const unsubB = subscribeShimmerClock(p => b.push(p))
|
||||
|
||||
// Two subscribers, ONE new timer.
|
||||
expect(vi.getTimerCount()).toBe(timersBefore + 1)
|
||||
|
||||
vi.advanceTimersByTime(300)
|
||||
|
||||
// Same shared phases, in lockstep.
|
||||
expect(a.length).toBeGreaterThan(0)
|
||||
expect(a).toEqual(b)
|
||||
|
||||
unsubA()
|
||||
unsubB()
|
||||
})
|
||||
|
||||
it('stops the interval with the last unsubscribe', () => {
|
||||
const unsubA = subscribeShimmerClock(() => {})
|
||||
const unsubB = subscribeShimmerClock(() => {})
|
||||
const timersRunning = vi.getTimerCount()
|
||||
|
||||
unsubA()
|
||||
// Still one subscriber — clock keeps running.
|
||||
expect(vi.getTimerCount()).toBe(timersRunning)
|
||||
|
||||
unsubB()
|
||||
expect(vi.getTimerCount()).toBe(timersRunning - 1)
|
||||
})
|
||||
|
||||
it('a late subscriber restarts the clock cleanly', () => {
|
||||
const unsubA = subscribeShimmerClock(() => {})
|
||||
|
||||
unsubA()
|
||||
|
||||
const seen: number[] = []
|
||||
const unsubB = subscribeShimmerClock(p => seen.push(p))
|
||||
|
||||
vi.advanceTimersByTime(200)
|
||||
expect(seen.length).toBeGreaterThan(0)
|
||||
unsubB()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -50,17 +50,77 @@ export function Shimmer({
|
|||
)
|
||||
}
|
||||
|
||||
/** Self-ticking phase for shimmer compositions. */
|
||||
export function useShimmerPhase(tickMs = 90): number {
|
||||
const [phase, setPhase] = useState(0)
|
||||
// ── Shared shimmer clock ─────────────────────────────────────────────
|
||||
//
|
||||
// ONE interval drives every mounted shimmer composition (review on #20379,
|
||||
// finding 5: the session panel could mount independent 90 ms intervals for
|
||||
// lazy skills AND lazy tools — ~22 state updates/sec on an otherwise-idle
|
||||
// TUI). Subscribers share the tick; the interval exists only while
|
||||
// subscribers do, and all updates land in one timer callback so React
|
||||
// batches them into a single render pass.
|
||||
|
||||
const TICK_MS = 90
|
||||
|
||||
/** Animation budget per mount. A lazy watch session can stay lazy
|
||||
* indefinitely — after the budget the skeleton freezes in place (still
|
||||
* reads as "loading") instead of repainting forever. */
|
||||
export const SHIMMER_ANIMATE_MS = 30_000
|
||||
|
||||
const clockListeners = new Set<(phase: number) => void>()
|
||||
let clockId: NodeJS.Timeout | null = null
|
||||
let clockPhase = 0
|
||||
|
||||
/** Subscribe to the shared clock (exported for tests). Returns unsubscribe;
|
||||
* the interval stops with the last subscriber. */
|
||||
export function subscribeShimmerClock(fn: (phase: number) => void): () => void {
|
||||
clockListeners.add(fn)
|
||||
|
||||
if (!clockId) {
|
||||
clockId = setInterval(() => {
|
||||
clockPhase += 1
|
||||
|
||||
for (const listener of clockListeners) {
|
||||
listener(clockPhase)
|
||||
}
|
||||
}, TICK_MS)
|
||||
|
||||
clockId.unref?.()
|
||||
}
|
||||
|
||||
return () => {
|
||||
clockListeners.delete(fn)
|
||||
|
||||
if (!clockListeners.size && clockId) {
|
||||
clearInterval(clockId)
|
||||
clockId = null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Phase from the shared clock, bounded: stops advancing (and stops costing
|
||||
* renders) after `animateMs`. */
|
||||
export function useShimmerPhase(animateMs = SHIMMER_ANIMATE_MS): number {
|
||||
const [phase, setPhase] = useState(clockPhase)
|
||||
|
||||
useEffect(() => {
|
||||
const id = setInterval(() => setPhase(p => p + 1), tickMs)
|
||||
const startedAt = Date.now()
|
||||
|
||||
id.unref?.()
|
||||
let unsubscribe: (() => void) | null = subscribeShimmerClock(next => {
|
||||
if (Date.now() - startedAt >= animateMs) {
|
||||
unsubscribe?.()
|
||||
unsubscribe = null
|
||||
|
||||
return () => clearInterval(id)
|
||||
}, [tickMs])
|
||||
return
|
||||
}
|
||||
|
||||
setPhase(next)
|
||||
})
|
||||
|
||||
return () => {
|
||||
unsubscribe?.()
|
||||
unsubscribe = null
|
||||
}
|
||||
}, [animateMs])
|
||||
|
||||
return phase
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue