-
Notifications
You must be signed in to change notification settings - Fork 3
Throttle persona:state:snapshot — fix 8GB browser memory leak #288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -49,12 +49,17 @@ export const DEFAULT_STATE_CONFIG: StateConfig = { | |||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||
| * PersonaStateManager: Manages internal state and traffic decisions | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
| /** Minimum interval between snapshot emissions per persona (ms) */ | ||||||||||||||||||||||||||||||||
| const SNAPSHOT_THROTTLE_MS = 2000; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export class PersonaStateManager { | ||||||||||||||||||||||||||||||||
| private readonly config: StateConfig; | ||||||||||||||||||||||||||||||||
| private state: PersonaState; | ||||||||||||||||||||||||||||||||
| private readonly personaName: string; | ||||||||||||||||||||||||||||||||
| private readonly personaId?: string; | ||||||||||||||||||||||||||||||||
| private readonly logger?: SubsystemLogger; | ||||||||||||||||||||||||||||||||
| private _lastSnapshotTime = 0; | ||||||||||||||||||||||||||||||||
| private _snapshotPending = false; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| constructor(personaName: string, config: Partial<StateConfig> = {}, personaId?: string) { | ||||||||||||||||||||||||||||||||
| this.personaName = personaName; | ||||||||||||||||||||||||||||||||
|
|
@@ -268,17 +273,42 @@ export class PersonaStateManager { | |||||||||||||||||||||||||||||||
| * Uses DataDaemon.jtagContext for cross-context (server→browser) delivery. | ||||||||||||||||||||||||||||||||
| * Without the context, bare Events.emit() stays server-local. | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||
| * Throttled snapshot emission — max once per SNAPSHOT_THROTTLE_MS. | ||||||||||||||||||||||||||||||||
| * With 15 personas each calling this on every cycle (3-5s) plus rest(), | ||||||||||||||||||||||||||||||||
| * unthrottled emission hit 200+/s and flooded the WebSocket to browser. | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
|
Comment on lines
273
to
+280
|
||||||||||||||||||||||||||||||||
| * Uses DataDaemon.jtagContext for cross-context (server→browser) delivery. | |
| * Without the context, bare Events.emit() stays server-local. | |
| */ | |
| /** | |
| * Throttled snapshot emission — max once per SNAPSHOT_THROTTLE_MS. | |
| * With 15 personas each calling this on every cycle (3-5s) plus rest(), | |
| * unthrottled emission hit 200+/s and flooded the WebSocket to browser. | |
| */ | |
| * Uses DataDaemon.jtagContext for cross-context (server→browser) delivery; | |
| * without the context, bare Events.emit() stays server-local. | |
| * | |
| * Emission is throttled — max once per SNAPSHOT_THROTTLE_MS. With many personas | |
| * each calling this on every cycle (3–5s) plus rest(), unthrottled emission | |
| * can reach 200+/s and flood the WebSocket to the browser. | |
| */ |
Copilot
AI
Mar 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a subtle edge case: when a leading-edge emit passes the throttle check while _snapshotPending is still true from a previously scheduled trailing timer, both the leading emit and the trailing timer will fire, causing a double-emit within milliseconds. This happens when setTimeout fires slightly late and a new call arrives just after the throttle window expires but before the pending timer runs.
To fix, either (a) store the timer ID and clear it when a leading emit fires, or (b) reset _snapshotPending = false at line 297 before calling this.emitSnapshotNow() in the leading path. Option (a) is more robust:
Store the timeout handle (e.g., private _snapshotTimer: ReturnType<typeof setTimeout> | null = null) and call clearTimeout(this._snapshotTimer) before the leading-edge emitSnapshotNow() call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
SNAPSHOT_THROTTLE_MSconstant was inserted between the class-level JSDoc (lines 49-51:PersonaStateManager: Manages internal state and traffic decisions) and theclass PersonaStateManagerdeclaration at line 55. In TypeScript/JSDoc, a/** */comment documents the next declaration — so the class-level JSDoc now documentsSNAPSHOT_THROTTLE_MSinstead of the class, and the class itself loses its documentation. Move the constant above the class JSDoc or inside the class as aprivate static readonlyto preserve the original documentation association.