[AI Task] [Tizen.Multimedia.AudioIO] Remove params array allocation in ValidateState hot path#7645
[AI Task] [Tizen.Multimedia.AudioIO] Remove params array allocation in ValidateState hot path#7645JoonghyunCho wants to merge 2 commits into
Conversation
…teState hot paths AudioCapture.Read / AudioPlayback.Write are streaming hot loops that each called ValidateState(...) with 1 or 2 explicit states. The `params AudioIOState[]` overload allocated a fresh array per call and went through LINQ Contains. Split into explicit 1-arg and 2-arg overloads on AudioIOUtil and on the AudioCaptureBase / AudioPlayback wrappers. The success path is now inline-friendly with no allocation; the throw branch is factored out behind [MethodImpl(NoInlining)] helpers. All 13 callers in AudioCapture / AudioPlayback already pass 1 or 2 states, so the params overload is removed entirely. No public API change (all types involved are internal/private). Exception type and message format preserved. Fixes #7609
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors state validation in AudioCapture, AudioPlayback, and AudioIOUtil by replacing the params array-based ValidateState method with specific overloads for one or two states, which avoids array allocations and LINQ overhead. The reviewer suggests marking these new validation methods with [MethodImpl(MethodImplOptions.AggressiveInlining)] to further optimize performance in critical hot paths.
| internal static void ValidateState(AudioIOState curState, AudioIOState desired) | ||
| { | ||
| Debug.Assert(desiredStates.Length > 0); | ||
| if (curState == desired) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (desiredStates.Contains(curState)) | ||
| ThrowInvalidState(curState, desired); | ||
| } |
There was a problem hiding this comment.
Since these methods are part of a performance-critical hot path (as mentioned in the PR description), it is recommended to mark them with [MethodImpl(MethodImplOptions.AggressiveInlining)]. This encourages the JIT compiler to inline these small validation checks directly into the calling methods (like Read or Write), reducing method call overhead and potentially enabling further optimizations.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static void ValidateState(AudioIOState curState, AudioIOState desired)
{
if (curState == desired)
{
return;
}
ThrowInvalidState(curState, desired);
}| internal static void ValidateState(AudioIOState curState, AudioIOState desired1, AudioIOState desired2) | ||
| { | ||
| if (curState == desired1 || curState == desired2) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| throw new InvalidOperationException($"The audio is not in a valid state. " + | ||
| $"Current State : { curState }, Valid State : { string.Join(", ", desiredStates) }."); | ||
| ThrowInvalidState(curState, desired1, desired2); | ||
| } |
There was a problem hiding this comment.
Similar to the 1-argument overload, this method should also be marked with [MethodImpl(MethodImplOptions.AggressiveInlining)] to ensure optimal performance in the streaming hot path.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static void ValidateState(AudioIOState curState, AudioIOState desired1, AudioIOState desired2)
{
if (curState == desired1 || curState == desired2)
{
return;
}
ThrowInvalidState(curState, desired1, desired2);
}
|
🤖 [AI Review] Reviewed — no findings. Scope checked:
No 🔴 critical issues, no 🟡 suggestions to flag. Automated review — final merge decision rests with human reviewers. |
Add [MethodImpl(MethodImplOptions.AggressiveInlining)] to both AudioIOUtil.ValidateState overloads so the JIT inlines these tiny state checks into the streaming hot path (Read/Write). Applied-Human-Comments: 3254517301,3254517303
|
🤖 [AI Review] |
1 participant
Summary
AudioCapture.Read(int)andAudioPlayback.Write(byte[])are streaming hot paths that callValidateState(...)with 1 or 2 explicitAudioIOStatevalues. The previousparams AudioIOState[] desiredStatessignature allocated a fresh single-element (or two-element) array per call and then went through LINQContains. This PR replaces theparamsoverload with explicit 1-arg and 2-arg overloads, eliminating the per-call allocation and the LINQ dispatch on the success path.Changes
src/Tizen.Multimedia.AudioIO/AudioIO/AudioIOUtil.csValidateState(AudioIOState, params AudioIOState[])with two overloads:(curState, desired)and(curState, desired1, desired2).[MethodImpl(MethodImplOptions.NoInlining)]helpers so the success path stays small and inline-friendly.System.Linqusing (no longer needed).src/Tizen.Multimedia.AudioIO/AudioIO/AudioCapture.csAudioCaptureBase.ValidateStatesplit into 1-arg and 2-arg overloads delegating toAudioIOUtil.src/Tizen.Multimedia.AudioIO/AudioIO/AudioPlayback.csAudioPlayback.ValidateStatesplit into 1-arg and 2-arg overloads delegating toAudioIOUtil.All 13 call sites in
AudioCapture/AudioPlaybackalready pass 1 or 2 states, so they bind to the new overloads with no source changes.Mode
Refactoring
Performance impact (per call)
paramsarray allocationAudioIOState[1]/AudioIOState[2])ContainsdispatchContains==(and `Significant in
AudioCapture.Read/AudioPlayback.Writewhich can be called many times per second per stream.API compatibility
internal(AudioIOUtil) orinternal/private(the wrappers onAudioCaptureBase/AudioPlayback). No public API change.InvalidOperationException)."The audio is not in a valid state. Current State : X, Valid State : Y."shape).Verification
dotnet build src/Tizen.Multimedia.AudioIO/Tizen.Multimedia.AudioIO.csproj— 0 errors; pre-existing warnings unchanged).sdbdevice available in this environment — manual on-device benchmark recommended).Fixes #7609