Skip to content

Performance: Optimize AudioEngine subscriber unsubscriptions using Sets#250

Open
ysdede wants to merge 1 commit intomasterfrom
performance/optimize-audio-engine-callbacks-10374202247365408348
Open

Performance: Optimize AudioEngine subscriber unsubscriptions using Sets#250
ysdede wants to merge 1 commit intomasterfrom
performance/optimize-audio-engine-callbacks-10374202247365408348

Conversation

@ysdede
Copy link
Copy Markdown
Owner

@ysdede ysdede commented Apr 14, 2026

What changed

Refactored subscriber lists in src/lib/audio/AudioEngine.ts (segmentCallbacks, windowCallbacks, audioChunkCallbacks, and visualizationCallbacks) from Array to Set.
Modified subscription add and delete methods accordingly, taking advantage of Set's built-in .add() and .delete() APIs instead of Array.filter().

Why it was needed

Using Array.filter() to remove subscribers created a new Array object each time, causing O(N) array reallocation and Garbage Collection (GC) churn. Given these events can be bound or unbound during specific lifecycle methods in a performance-critical audio recording path, reducing unnecessary allocations aids frame rate stability and lessens main thread pauses due to GC. Using Set ensures O(1) unsubscription and mutation in-place.

Impact

Significantly lowers memory allocation pressure and prevents Array reallocations. O(N) time complexity to remove elements was reduced to O(1). This change eliminates Array.filter() allocations within hot execution paths. Set.prototype.forEach still preserves insertion order so execution sequences remain entirely unaltered.

How to verify

  1. Run npm run test and verify that all 219 tests pass.
  2. The core audio streaming logic inside AudioEngine functions as before.

PR created automatically by Jules for task 10374202247365408348 started by @ysdede

Summary by Sourcery

Enhancements:

  • Replace array-backed callback lists with Set-based collections for segment, window, audio chunk, and visualization subscribers to enable O(1) add/remove operations without array reallocations.

@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 14, 2026

Warning

Rate limit exceeded

@ysdede has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 58 minutes and 57 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 58 minutes and 57 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ba274187-cc66-4e07-92d5-b88a08e84986

📥 Commits

Reviewing files that changed from the base of the PR and between 474dbe6 and b054a87.

📒 Files selected for processing (1)
  • src/lib/audio/AudioEngine.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch performance/optimize-audio-engine-callbacks-10374202247365408348

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Optimize AudioEngine callbacks using Sets for O(1) unsubscription

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Refactored subscriber callback lists from Arrays to Sets
• Reduced unsubscription complexity from O(N) to O(1)
• Eliminates Array.filter() allocations in hot audio paths
• Preserves callback execution order via Set.forEach semantics
Diagram
flowchart LR
  A["Array-based callbacks<br/>with filter()"] -- "Refactor to Set" --> B["Set-based callbacks<br/>with delete()"]
  A -- "O(N) complexity<br/>GC churn" --> C["Performance issue"]
  B -- "O(1) complexity<br/>In-place mutation" --> D["Optimized performance"]
Loading

Grey Divider

File Changes

1. src/lib/audio/AudioEngine.ts ✨ Enhancement +13/-13

Convert callback subscriber lists to Sets

• Changed segmentCallbacks, windowCallbacks, audioChunkCallbacks, and visualizationCallbacks
 from Array to Set type declarations
• Updated subscription methods to use Set.add() instead of Array.push()
• Replaced Array.filter() unsubscription logic with Set.delete() for O(1) removal
• Maintained callback execution order through Set.forEach iteration semantics

src/lib/audio/AudioEngine.ts


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Apr 14, 2026

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)
🐞\ ≡ Correctness (1)

Grey Divider


Remediation recommended

1. Set mutation changes dispatch 🐞
Description
Callback lists now use Set with in-place delete, so modifications during notification can change
which callbacks run within the same notification cycle (e.g., unsubscribing another listener can
cause it to be skipped, and newly-added listeners may run immediately). This is a behavioral change
from the previous Array+filter approach, which effectively provided a snapshot for in-flight
iterations by rebinding to a new Array.
Code

src/lib/audio/AudioEngine.ts[R478-482]

    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);
        };
Evidence
Subscriptions/unsubscriptions now mutate Sets in place (add/delete), while notifications iterate
directly over those same Sets. With the prior Array implementation, unsubscription used filter() and
reassigned a new array, so an in-flight iterator would continue over the old array object,
preserving a stable listener list for that dispatch; the new Set approach makes the iteration
observe live mutations.

src/lib/audio/AudioEngine.ts[39-52]
src/lib/audio/AudioEngine.ts[78-78]
src/lib/audio/AudioEngine.ts[442-447]
src/lib/audio/AudioEngine.ts[478-483]
src/lib/audio/AudioEngine.ts[935-940]
src/lib/audio/AudioEngine.ts[687-695]
src/lib/audio/AudioEngine.ts[764-778]
src/lib/audio/AudioEngine.ts[729-762]
src/lib/audio/AudioEngine.ts[946-967]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The subscriber collections were changed to `Set` and are mutated in-place via `.delete()`. Notification loops iterate directly over those Sets, so if any callback subscribes/unsubscribes during dispatch, the set's iteration can skip not-yet-called listeners or include newly added ones in the same cycle, which differs from the prior Array+filter rebinding behavior.

## Issue Context
This affects all four callback collections (`segmentCallbacks`, `windowCallbacks`, `audioChunkCallbacks`, `visualizationCallbacks`). The previous `Array.filter()` pattern reassigned a new array, effectively making in-flight dispatch iterate over a stable snapshot.

## Fix Focus Areas
Consider snapshotting the current subscribers before iterating (or maintaining a stable iteration snapshot updated only on subscribe/unsubscribe) to preserve dispatch semantics:
- src/lib/audio/AudioEngine.ts[687-695]
- src/lib/audio/AudioEngine.ts[729-762]
- src/lib/audio/AudioEngine.ts[764-778]
- src/lib/audio/AudioEngine.ts[946-967]
- src/lib/audio/AudioEngine.ts[442-483]
- src/lib/audio/AudioEngine.ts[935-940]ेय

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Now that the callback collections are Sets instead of Arrays, scan remaining usages of segmentCallbacks, windowCallbacks, audioChunkCallbacks, and visualizationCallbacks in this file for any array-specific operations (e.g. .length, indexing, .map, .filter, .slice) and update them to Set-appropriate patterns (e.g. .size, for..of, forEach).
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Now that the callback collections are Sets instead of Arrays, scan remaining usages of `segmentCallbacks`, `windowCallbacks`, `audioChunkCallbacks`, and `visualizationCallbacks` in this file for any array-specific operations (e.g. `.length`, indexing, `.map`, `.filter`, `.slice`) and update them to Set-appropriate patterns (e.g. `.size`, `for..of`, `forEach`).

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the AudioEngine class to use Set instead of Array for managing various callback collections, including speech segments, windowed audio, audio chunks, and visualization updates. This change simplifies the subscription and unsubscription logic by utilizing the native add and delete methods of the Set object. I have no feedback to provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant