-
-
Notifications
You must be signed in to change notification settings - Fork 67
Disable recorder warm-up to prevent coreaudiod CPU spike #139
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
base: main
Are you sure you want to change the base?
Disable recorder warm-up to prevent coreaudiod CPU spike #139
Conversation
Warm-up priming keeps a CoreAudio client alive and can drive coreaudiod CPU to ~10% at app start. Remove the launch-time warm-up and keep sound effect preloading lightweight so the app stays idle until the first recording.
📝 WalkthroughWalkthroughRecorder warm-up was disabled and recorder lifecycle handling was changed: RecordingClient now stops and destroys the AVAudioRecorder and delays before priming a new recorder; TranscriptionFeature startup no longer warms up the recorder; SoundEffect now tracks active plays and enables AVAudioEngine auto-shutdown. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant TranscriptionFeature
participant RecordingClient
participant CoreAudio as coreaudiod
User->>TranscriptionFeature: trigger transcription start (hotkey)
TranscriptionFeature->>RecordingClient: startMetering / startRecording
RecordingClient->>coreaudiod: open AVAudioRecorder (create)
RecordingClient-->>TranscriptionFeature: recorder ready
User->>RecordingClient: stopRecording
RecordingClient->>coreaudiod: recorder.stop() and deleteRecording()
RecordingClient->>RecordingClient: wait 200ms
RecordingClient->>coreaudiod: prime new recorder (create fresh instance)
RecordingClient-->>TranscriptionFeature: primed for next session
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Hex/Features/Transcription/TranscriptionFeature.swift (1)
79-87: Stale comment references removed effect.The comment on lines 81-83 still mentions "3) Priming the recorder for instant startup" but only two effects remain in the merge. Update the comment to reflect the current implementation.
📝 Suggested fix
case .task: - // Starts two concurrent effects: + // Starts concurrent effects: // 1) Observing audio meter // 2) Monitoring hot key events - // 3) Priming the recorder for instant startup return .merge( startMeteringEffect(), startHotKeyMonitoringEffect() )
🤖 Fix all issues with AI agents
In @Hex/Clients/SoundEffect.swift:
- Around line 95-99: The stop(_ soundEffect: SoundEffect) method currently
decrements activePlays immediately which can double-decrement when the play()
completion handler later calls handlePlaybackEnded(); remove the activePlays
decrement from stop(_:) and let handlePlaybackEnded() be solely responsible for
decrementing activePlays, or alternatively implement per-sound play state (e.g.,
a Set or Bool flag keyed by SoundEffect in playerNodes) and guard against
decrementing twice by checking that flag in both stop(_:) and
handlePlaybackEnded() before modifying activePlays; update stop(_:) and
handlePlaybackEnded() accordingly to use that per-sound state if choosing the
second option.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/silent-brisk-vivid.mdHex/Clients/RecordingClient.swiftHex/Clients/SoundEffect.swiftHex/Features/Transcription/TranscriptionFeature.swift
🧰 Additional context used
📓 Path-based instructions (5)
**/*Feature.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Use The Composable Architecture (TCA) for state management; implement features as TCA reducers
Files:
Hex/Features/Transcription/TranscriptionFeature.swift
**/{App,Transcription,Settings,History}Feature.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the core features as AppFeature, TranscriptionFeature, SettingsFeature, and HistoryFeature
Files:
Hex/Features/Transcription/TranscriptionFeature.swift
**/*.swift
📄 CodeRabbit inference engine (.cursor/rules/composable-architecture-tree-navigation.mdc)
**/*.swift: Model presented child features in parent state using the Presents() macro with optional child state (non-nil presents, nil dismisses)
Expose child feature actions in the parent as PresentationAction<Child.Action> (e.g., case child(PresentationAction<Child.Action>))
Integrate child reducers into parents using .ifLet(.$child, action: .child) { ChildFeature() } to compose optional-driven navigation
Drive navigation by populating optional presentation state to present and setting it to nil to dismiss
In SwiftUI views, use @bindable var store: StoreOf and pass a binding store to SwiftUI modifiers like .sheet(item:), .popover(item:), .navigationDestination(item:) via $store.scope(state:action:)
Prefer a single enum-based destination over multiple optional @presents properties when a feature can navigate to multiple screens
Define a nested @Reducer enum Destination with cases for each child feature, hold @presents var destination: Destination.State?, and route actions via case destination(PresentationAction<Destination.Action>)
Compose destination enum into the parent with .ifLet(.$destination, action: .destination)
Scope SwiftUI bindings to specific destination cases using $store.scope(state: .destination?.case, action: .destination.case) with .sheet/.popover/.navigationDestination
For platforms earlier than iOS 16/macOS 13/tvOS 16/watchOS 9, use a NavigationLink extension that drives navigation from Binding<D?> with appropriate @available annotations
Dismiss from reducers using @dependency(.dismiss) and invoke await dismiss() inside an effect (.run) rather than directly in the reducer
Do not send additional actions after invoking dismiss() within the same effect
**/*.swift: Use Swift Composable Architecture (TCA) for state management, effects, and reducers throughout the macOS app
Target Swift 6 language features and compatibility across the codebase
Use Swift async/await for asynchronous code instead of callback-based patterns
Files:
Hex/Features/Transcription/TranscriptionFeature.swiftHex/Clients/SoundEffect.swiftHex/Clients/RecordingClient.swift
**/SoundEffect.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Play audio feedback through SoundEffect utilities
Files:
Hex/Clients/SoundEffect.swift
**/RecordingClient.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Use RecordingClient (AVAudioRecorder wrapper) for audio capture instead of direct AVAudioRecorder usage
Files:
Hex/Clients/RecordingClient.swift
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: kitlangton/Hex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T12:14:22.927Z
Learning: Applies to **/RecordingClient.swift : Use RecordingClient (AVAudioRecorder wrapper) for audio capture instead of direct AVAudioRecorder usage
Learnt from: CR
Repo: kitlangton/Hex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T12:14:22.927Z
Learning: Applies to **/TranscriptionClient.swift : Use WhisperKit via TranscriptionClient for ML transcription (tracking WhisperKit main branch)
📚 Learning: 2025-09-10T12:14:22.927Z
Learnt from: CR
Repo: kitlangton/Hex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T12:14:22.927Z
Learning: Applies to **/{App,Transcription,Settings,History}Feature.swift : Maintain the core features as AppFeature, TranscriptionFeature, SettingsFeature, and HistoryFeature
Applied to files:
Hex/Features/Transcription/TranscriptionFeature.swift
📚 Learning: 2025-09-10T12:14:22.927Z
Learnt from: CR
Repo: kitlangton/Hex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T12:14:22.927Z
Learning: Applies to **/TranscriptionClient.swift : Use WhisperKit via TranscriptionClient for ML transcription (tracking WhisperKit main branch)
Applied to files:
Hex/Features/Transcription/TranscriptionFeature.swiftHex/Clients/RecordingClient.swift
📚 Learning: 2025-09-10T12:14:22.927Z
Learnt from: CR
Repo: kitlangton/Hex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T12:14:22.927Z
Learning: Applies to **/RecordingClient.swift : Use RecordingClient (AVAudioRecorder wrapper) for audio capture instead of direct AVAudioRecorder usage
Applied to files:
Hex/Features/Transcription/TranscriptionFeature.swiftHex/Clients/SoundEffect.swiftHex/Clients/RecordingClient.swift
📚 Learning: 2025-09-10T12:14:22.927Z
Learnt from: CR
Repo: kitlangton/Hex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T12:14:22.927Z
Learning: Applies to **/HotKeyProcessor.swift : Support both press-and-hold and double-tap hotkey recording modes in HotKeyProcessor
Applied to files:
Hex/Features/Transcription/TranscriptionFeature.swift
📚 Learning: 2025-09-10T12:14:22.927Z
Learnt from: CR
Repo: kitlangton/Hex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T12:14:22.927Z
Learning: Applies to **/KeyEventMonitorClient.swift : Use Sauce for global hotkey monitoring via KeyEventMonitorClient, not direct event taps
Applied to files:
Hex/Features/Transcription/TranscriptionFeature.swift
📚 Learning: 2025-09-10T12:14:22.927Z
Learnt from: CR
Repo: kitlangton/Hex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T12:14:22.927Z
Learning: Applies to **/SoundEffect.swift : Play audio feedback through SoundEffect utilities
Applied to files:
Hex/Features/Transcription/TranscriptionFeature.swiftHex/Clients/SoundEffect.swift
📚 Learning: 2025-09-10T12:14:58.888Z
Learnt from: CR
Repo: kitlangton/Hex PR: 0
File: .cursor/rules/composable-architecture-tree-navigation.mdc:0-0
Timestamp: 2025-09-10T12:14:58.888Z
Learning: Applies to **/*.swift : Do not send additional actions after invoking dismiss() within the same effect
Applied to files:
Hex/Features/Transcription/TranscriptionFeature.swift
🧬 Code graph analysis (1)
Hex/Clients/RecordingClient.swift (2)
Hex/Clients/SoundEffect.swift (1)
stop(95-99)Hex/Features/History/HistoryFeature.swift (1)
stop(74-77)
🔇 Additional comments (10)
.changeset/silent-brisk-vivid.md (1)
1-4: Changeset entry is well-formatted and accurately documents the fix.The entry correctly uses patch-level versioning for a performance improvement, has clear messaging that describes the user-facing benefit, and aligns with the PR objectives.
Hex/Clients/SoundEffect.swift (3)
101-105: LGTM!Resetting
activePlaysto 0 instopAll()is correct since all playbacks are terminated. Themax(activePlays - 1, 0)guard inhandlePlaybackEnded()protects against any pending completion handlers causing negative counts.
146-162: Good defensive approach combining auto-shutdown with explicit stop.Enabling
isAutoShutdownEnabledas a safety net while also explicitly stopping the engine instopEngineIfIdle()ensures CoreAudio resources are released promptly. The explicit stop provides immediate cleanup rather than waiting for the system's auto-shutdown timeout.
164-175: LGTM!The
handlePlaybackEnded()andstopEngineIfIdle()helpers are well-structured. The guard clauses ensure the engine only stops when truly idle, andmax(activePlays - 1, 0)prevents negative counts from stale completion handlers.Hex/Clients/RecordingClient.swift (6)
817-834: LGTM! Stop-and-destroy pattern correctly releases CoreAudio resources.Destroying the recorder by setting it to
nilafterstop()ensures the CoreAudio connection is fully released rather than kept alive for reuse. This addresses the core issue of CPU spikes from lingering audio IO threads.
846-858: Consider the robustness of the 200ms delay.The hardcoded 200ms delay is a pragmatic fix, but CoreAudio's cleanup timing may vary based on system load or hardware. If CPU spikes persist for some users, consider:
- Making the delay configurable via settings
- Increasing the delay slightly (e.g., 300-500ms) for more margin
For now, this is a reasonable starting point. Monitor user feedback to see if adjustments are needed.
984-1003: Good optimization deferring metering enablement.Enabling metering only when actually recording (line 985) rather than at recorder creation (line 1000) reduces idle CPU usage. This ensures no continuous audio monitoring overhead when the app is idle.
1025-1038: LGTM!The updated comments and logging clarify the expected state (recorder should be nil) and provide useful diagnostic information for troubleshooting.
1074-1089: Excellent fix for cleanup to release primed CoreAudio connections.The critical insight here is that
prepareToRecord()establishes an active CoreAudio connection even without recording. Callingstop()before nilling the recorder ensures this connection is properly released. The detailed comment explains the reasoning well.
1063-1071: LGTM!Log level appropriately changed to debug since warm-up is now an optional/on-demand operation rather than a startup requirement.
Track per-play tokens so stop() doesn't double-decrement activePlays when the completion handler fires. This prevents premature engine shutdown when multiple sounds overlap.
|
Addressed the review note: stop() no longer double-decrements activePlays. Added per-play tokens so completion handlers and stop() can't both decrement the same play. Pushed as commit ad3924c. |
Summary
Testing
Notes
The CPU spike appears immediately on app start due to recorder priming; disabling warm-up removes the idle CoreAudio IO thread.
Fixes #137