Skip to content

perf: optimize audio engine callback removal with Set #219

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.

Sign up for GitHub

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

Open
ysdede wants to merge 1 commit into
base: master
Choose a base branch
from
Open

perf: optimize audio engine callback removal with Set #219

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions src/lib/audio/AudioEngine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,19 @@ export class AudioEngine implements IAudioEngine {

private currentEnergy: number = 0;

private segmentCallbacks: Array<(segment: AudioSegment) => void> = [];
private segmentCallbacks = new Set<(segment: AudioSegment) => void>();

// Fixed-window streaming state (v3 token streaming mode)
private windowCallbacks: Array<{
private windowCallbacks = new Set<{
windowDuration: number;
overlapDuration: number;
triggerInterval: number;
callback: (audio: Float32Array, startTime: number) => void;
lastWindowEnd: number; // Frame offset of last window end
}> = [];
}>();
Comment on lines +42 to +48
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For improved readability and maintainability, consider extracting the inline type for windowCallbacks into a named interface. This makes the code cleaner and the type reusable.

For example, you could define an interface at the top of the file or before the class:

interface WindowCallbackEntry {
    windowDuration: number;
    overlapDuration: number;
    triggerInterval: number;
    callback: (audio: Float32Array, startTime: number) => void;
    lastWindowEnd: number;
}

And then use it here:

private windowCallbacks = new Set<WindowCallbackEntry>();


// Resampled audio chunk callbacks (for mel worker, etc.)
private audioChunkCallbacks: Array<(chunk: Float32Array) => void> = [];
private audioChunkCallbacks = new Set<(chunk: Float32Array) => void>();

// SMA buffer for energy calculation
private energyHistory: number[] = [];
Expand All @@ -75,7 +75,7 @@ export class AudioEngine implements IAudioEngine {
};

// Subscribers for visualization updates
private visualizationCallbacks: Array<(data: Float32Array, metrics: AudioMetrics, bufferEndTime: number) => void> = [];
private visualizationCallbacks = new Set<(data: Float32Array, metrics: AudioMetrics, bufferEndTime: number) => void>();
private lastVisualizationNotifyTime: number = 0;
private readonly VISUALIZATION_NOTIFY_INTERVAL_MS = 33; // ~30fps in foreground
private readonly VISUALIZATION_NOTIFY_HIDDEN_INTERVAL_MS = 250; // Lower cadence for hidden tab
Expand Down Expand Up @@ -440,9 +440,9 @@ export class AudioEngine implements IAudioEngine {
}

onSpeechSegment(callback: (segment: AudioSegment) => void): () => void {
this.segmentCallbacks.push(callback);
this.segmentCallbacks.add(callback);
return () => {
this.segmentCallbacks = this.segmentCallbacks.filter((cb) => cb !== callback);
this.segmentCallbacks.delete(callback);
};
}

Expand All @@ -463,10 +463,10 @@ export class AudioEngine implements IAudioEngine {
callback,
lastWindowEnd: 0, // Will be set on first chunk
};
this.windowCallbacks.push(entry);
this.windowCallbacks.add(entry);

return () => {
this.windowCallbacks = this.windowCallbacks.filter((e) => e !== entry);
this.windowCallbacks.delete(entry);
};
}

Expand All @@ -476,9 +476,9 @@ export class AudioEngine implements IAudioEngine {
* Returns an unsubscribe function.
*/
onAudioChunk(callback: (chunk: Float32Array) => void): () => void {
this.audioChunkCallbacks.push(callback);
this.audioChunkCallbacks.add(callback);
return () => {
this.audioChunkCallbacks = this.audioChunkCallbacks.filter((cb) => cb !== callback);
this.audioChunkCallbacks.delete(callback);
};
Comment on lines 478 to 482
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Unsubscribe can skip callbacks 🐞 Bug ✓ Correctness

During callback dispatch, unsubscribing now mutates the same Set being iterated, which can cause
later callbacks in the same notify cycle to be skipped. Previously, unsubscription reassigned a new
array (filter) so the in-flight iteration over the old array would continue unaffected.
Agent Prompt 
### Issue description
Callback unsubscribe now uses `Set.delete()` which mutates the same collection being iterated during dispatch. This can skip callbacks later in the same notify cycle (behavioral regression vs the old array+reassignment approach).

### Issue Context
In `AudioEngine`, event dispatch iterates over Sets directly (audio chunks, window callbacks, segments, visualization). Unsubscribe closures delete from those Sets.

### Fix Focus Areas
- src/lib/audio/AudioEngine.ts[687-698]
- src/lib/audio/AudioEngine.ts[729-762]
- src/lib/audio/AudioEngine.ts[764-778]
- src/lib/audio/AudioEngine.ts[946-967]

### What to change
- Iterate over a snapshot for each dispatch loop, e.g. `for (const cb of Array.from(this.audioChunkCallbacks)) { ... }` and similarly for `windowCallbacks`, `segmentCallbacks`, and `visualizationCallbacks`.
- Keep the underlying storage as `Set` so removals stay O(1); only snapshot at dispatch time.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

Expand Down Expand Up @@ -933,9 +933,9 @@ export class AudioEngine implements IAudioEngine {
* - Callbacks must not mutate `data` in place.
*/
onVisualizationUpdate(callback: (data: Float32Array, metrics: AudioMetrics, bufferEndTime: number) => void): () => void {
this.visualizationCallbacks.push(callback);
this.visualizationCallbacks.add(callback);
return () => {
this.visualizationCallbacks = this.visualizationCallbacks.filter((cb) => cb !== callback);
this.visualizationCallbacks.delete(callback);
};
}

Expand Down
Loading