-
Notifications
You must be signed in to change notification settings - Fork 0
Perf : LOH 최적화 #21
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: develop
Are you sure you want to change the base?
Perf : LOH 최적화 #21
Conversation
- AudioMemoryOwner.Memory.Span.Slice에서 AudioDataSize가 실제 메모리 크기 초과 시 예외 방지 - Math.Min을 활용한 안전한 크기 검증으로 IndexOutOfRangeException 방지 - 메모리 풀링 안전성 테스트 케이스 추가 및 검증 완료 - MemoryPool.Rent()의 실제 할당 크기 고려한 테스트 로직 개선 CodeRabbit 보안 검토 결과 반영으로 메모리 접근 안전성 확보
- IDisposable 인터페이스 구현으로 메모리 해제 시맨틱 강제 - WithAudioMemory 메서드에 소유권 이전 시 크기/수명 검증 추가 - ArgumentNullException: audioMemoryOwner null 체크 - ArgumentOutOfRangeException: audioDataSize 범위 검증 (0 ≤ size ≤ memory.Length) - 완전한 Dispose 패턴 구현 (Dispose(bool), Finalizer, GC.SuppressFinalize) - 소유권 이전 문서화: XML 주석으로 메모리 소유권 이전 명시 - 검증 로직 테스트 케이스 추가 및 기존 테스트 호환성 개선 메모리 누수 방지와 안전한 리소스 관리를 통한 시스템 안정성 강화
WalkthroughChatSegment이 레코드에서 IDisposable을 구현하는 봉인 클래스(sealed class)로 변경되고, 오디오 관리를 바이트 배열에서 IMemoryOwner 기반 메모리 소유권 모델로 전환했습니다. TextToSpeech 클라이언트와 TTS 응답 모델, ChatTTSProcessor 및 관련 테스트들이 메모리 기반 오디오 경로로 업데이트되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor TTS_Service
participant TextToSpeechClient
participant MemoryPool as IMemoryOwner<byte>
participant ChatProcessor
participant ChatSegment
TTS_Service->>TextToSpeechClient: TextToSpeechAsync(request)
TextToSpeechClient->>MemoryPool: ArrayPool/MemoryPool로 청크 읽기 → IMemoryOwner<byte> 생성
TextToSpeechClient-->>TTS_Service: TextToSpeechResponse(AudioMemoryOwner, AudioDataSize, AudioLength, ContentType)
TTS_Service->>ChatProcessor: TTS 응답 전달
ChatProcessor->>ChatSegment: segment = segment.WithAudioMemory(owner, size, contentType, length)
ChatSegment->>ChatSegment: 검증(owner != null, size), 기존 소유권 Dispose, 소유권 설정
ChatProcessor->>ChatSegment: span = segment.GetAudioSpan()
ChatSegment-->>ChatProcessor: Span(0..min(AudioDataSize, owner.Memory.Length))
sequenceDiagram
autonumber
actor Caller
participant ChatSegment
participant MemoryOwner as IMemoryOwner<byte>
Caller->>ChatSegment: Dispose()
ChatSegment->>ChatSegment: if not _disposed then dispose AudioMemoryOwner
ChatSegment->>MemoryOwner: Dispose()
ChatSegment->>Caller: _disposed = true
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 5
🧹 Nitpick comments (4)
ProjectVG.Application/Models/Chat/ChatSegment.cs (2)
40-42
: GetAudioSpan: Dispose 이후 접근 차단 가드 추가 권장해제된 후 접근을 명확히 실패시키기 위해 ObjectDisposedException을 던지면 디버깅이 쉬워집니다.
적용 예:
public ReadOnlySpan<byte> GetAudioSpan() { + if (_disposed) + throw new ObjectDisposedException(nameof(ChatSegment)); if (AudioMemoryOwner != null && AudioDataSize > 0) { var memory = AudioMemoryOwner.Memory; var safeSize = Math.Min(AudioDataSize, memory.Length); return memory.Span.Slice(0, safeSize); }아래 Dispose 코멘트의
_disposed
추가와 함께 사용하십시오.
85-96
: WithAudioMemory: 입력 검증 보강(컨텐츠 타입/길이) 및 메시지 개선현재 길이 범위 검증은 좋습니다. 추가로 다음을 권장합니다.
- audioContentType 공백/Null 금지
- audioLength 음수 금지
- 예외 메시지에 매개변수 값 포함
public ChatSegment WithAudioMemory(IMemoryOwner<byte> audioMemoryOwner, int audioDataSize, string audioContentType, float audioLength) { if (audioMemoryOwner is null) throw new ArgumentNullException(nameof(audioMemoryOwner)); + if (string.IsNullOrWhiteSpace(audioContentType)) + throw new ArgumentException("audioContentType must be non-empty.", nameof(audioContentType)); + + if (audioLength < 0) + throw new ArgumentOutOfRangeException(nameof(audioLength), audioLength, "audioLength must be >= 0."); if (audioDataSize < 0 || audioDataSize > audioMemoryOwner.Memory.Length) throw new ArgumentOutOfRangeException( nameof(audioDataSize), audioDataSize, $"audioDataSize는 0 이상 {audioMemoryOwner.Memory.Length} 이하여야 합니다.");또한 소유권 이전 규약을 강제하려면 외부에서 init 초기화를 못 하도록 접근 제한(earlier comment)도 함께 고려해주세요.
Also applies to: 98-106
ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs (2)
111-132
: WithAudioData(null!) 사용과 시그니처 불일치테스트는 null 오디오 배열을 허용하지만 WithAudioData 시그니처는
byte[]
(nullable 아님)입니다. 현재는 null 억제 연산자(null!)로 컴파일러 경고만 억제합니다.둘 중 하나로 일관화:
- API에서 null 허용:
- public ChatSegment WithAudioData(byte[] audioData, string audioContentType, float audioLength) + public ChatSegment WithAudioData(byte[]? audioData, string audioContentType, float audioLength)
- 또는 테스트에서 null 케이스 제거(권장 API 의도에 따라 선택).
278-317
: CI 실패 원인: 성능/GC 테스트의 비결정성 제거 필요파이프라인 실패 로그에 따르면 Base64 성능/GC 테스트가 환경에 따라 역전됩니다. GC.CollectionCount/시간 기반 비교는 CI에서 흔히 불안정합니다.
권장:
- Perf 테스트에 [Trait("Category","Perf")]를 달고 CI 기본 실행에서 제외하거나, 환경 변수로 opt-in.
- GC 압박 비교는
GC.GetAllocatedBytesForCurrentThread()
기반으로 바꾸고, 절대적 단언은 피하고 로그만 남기기.- 또는 BenchmarkDotNet 프로젝트로 분리.
필요 시 해당 테스트에 대한 구체 패치도 제안드릴 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ProjectVG.Application/Models/Chat/ChatSegment.cs
(4 hunks)ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs (1)
ProjectVG.Application/Models/Chat/ChatSegment.cs (8)
ChatSegment
(53-62)ChatSegment
(64-67)ChatSegment
(69-72)ChatSegment
(75-83)ChatSegment
(96-116)ChatSegment
(161-164)Dispose
(140-144)Dispose
(150-156)
🪛 GitHub Actions: Develop
ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs
[error] 316-316: Command: dotnet test --no-build --configuration Release --verbosity normal. Test failed: 1 failing test. MemoryPoolingPerformanceTests.Base64Encoding_ArrayPool_vs_Convert_PerformanceTest failed. ArrayPool Base64 encoding did not reduce GC pressure as expected (Gen2 optimized=38, standard=25).
[Fact] | ||
public void ChatSegment_GetAudioSpan_SafetyBoundaryTest() | ||
{ | ||
// 경계 조건 테스트: 유효한 범위 내에서의 메모리 접근 안전성 검증 | ||
var testData = GenerateTestAudioData(1000); | ||
using var memoryOwner = MemoryPool<byte>.Shared.Rent(500); // 더 작은 메모리 할당 | ||
var actualMemorySize = memoryOwner.Memory.Length; // 실제 할당된 메모리 크기 | ||
var copySize = Math.Min(500, actualMemorySize); | ||
testData.AsSpan(0, copySize).CopyTo(memoryOwner.Memory.Span); | ||
|
||
// 유효한 크기로 설정 (실제 메모리 크기 이하) | ||
var validSize = actualMemorySize - 10; // 안전한 크기 | ||
var segment = ChatSegment.CreateText("Test content") | ||
.WithAudioMemory(memoryOwner, validSize, "audio/wav", 5.0f); | ||
|
||
// GetAudioSpan이 정확한 크기를 반환해야 함 | ||
var span = segment.GetAudioSpan(); | ||
|
||
// 요청한 크기만큼 반환되어야 함 | ||
Assert.Equal(validSize, span.Length); | ||
_output.WriteLine($"요청 크기: {validSize}, 실제 메모리: {actualMemorySize}, 반환된 span 크기: {span.Length}"); | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
소유권 이전 규약 위반: WithAudioMemory 사용 시 memoryOwner를 using으로 해제하면 이중 Dispose 위험
WithAudioMemory는 소유권을 ChatSegment로 이전합니다. 현재 using var memoryOwner
는 테스트 종료 시 메모리 소유자를 다시 해제하므로 규약을 위반합니다.
적용 예(요지: using 제거, segment.Dispose()만 호출):
-using var memoryOwner = MemoryPool<byte>.Shared.Rent(500); // 더 작은 메모리 할당
+var memoryOwner = MemoryPool<byte>.Shared.Rent(500); // 소유권은 ChatSegment로 이전됨
...
-var segment = ChatSegment.CreateText("Test content")
- .WithAudioMemory(memoryOwner, validSize, "audio/wav", 5.0f);
+var segment = ChatSegment.CreateText("Test content")
+ .WithAudioMemory(memoryOwner, validSize, "audio/wav", 5.0f);
+try
+{
+ var span = segment.GetAudioSpan();
+ Assert.Equal(validSize, span.Length);
+}
+finally
+{
+ segment.Dispose();
+}
레포 전역에서 동일 패턴을 점검하려면 다음을 실행해주세요:
🏁 Script executed:
#!/bin/bash
# WithAudioMemory 사용 주변의 using var memoryOwner 패턴 탐지
rg -nP -C3 $'using\\s+var\\s+\\w+\\s*=\\s*MemoryPool<byte>\\.Shared\\.Rent\\(.+\\)|\\.WithAudioMemory\\(' --type cs
Length of output: 9773
소유권 이전 규약 위반 — WithAudioMemory에 IMemoryOwner 전달 시 memoryOwner를 using으로 해제하면 이중 Dispose(오류) 발생 위험
WithAudioMemory가 메모리 소유권을 ChatSegment로 이전한다고 가정하면, 현재 테스트들이 MemoryPool에서 Rent한 IMemoryOwner를 using var
로 감싼 뒤 WithAudioMemory(...)
에 넘기고 있어 범위 종료 시 이중 Dispose(또는 ObjectDisposed 예외)가 발생할 가능성이 큽니다. 동일 패턴이 ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs 여러 위치(예: l.74, l.93, l.120, l.137, l.167)에서 발견됨 — 테스트 전체에서 패턴 수정 필요.
수정 방안(예): memoryOwner를 using으로 해제하지 말고 ChatSegment가 소유권을 해제하도록 segment.Dispose()를 호출(또는 ChatSegment를 using으로 감싸 자동 Dispose). 예전 제안된 변경(diff)은 유효합니다:
-using var memoryOwner = MemoryPool<byte>.Shared.Rent(500); // 더 작은 메모리 할당
+var memoryOwner = MemoryPool<byte>.Shared.Rent(500); // 소유권은 ChatSegment로 이전됨
...
-var segment = ChatSegment.CreateText("Test content")
- .WithAudioMemory(memoryOwner, validSize, "audio/wav", 5.0f);
+var segment = ChatSegment.CreateText("Test content")
+ .WithAudioMemory(memoryOwner, validSize, "audio/wav", 5.0f);
+try
+{
+ var span = segment.GetAudioSpan();
+ Assert.Equal(validSize, span.Length);
+}
+finally
+{
+ segment.Dispose();
+}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
[Fact] | |
public void ChatSegment_GetAudioSpan_SafetyBoundaryTest() | |
{ | |
// 경계 조건 테스트: 유효한 범위 내에서의 메모리 접근 안전성 검증 | |
var testData = GenerateTestAudioData(1000); | |
using var memoryOwner = MemoryPool<byte>.Shared.Rent(500); // 더 작은 메모리 할당 | |
var actualMemorySize = memoryOwner.Memory.Length; // 실제 할당된 메모리 크기 | |
var copySize = Math.Min(500, actualMemorySize); | |
testData.AsSpan(0, copySize).CopyTo(memoryOwner.Memory.Span); | |
// 유효한 크기로 설정 (실제 메모리 크기 이하) | |
var validSize = actualMemorySize - 10; // 안전한 크기 | |
var segment = ChatSegment.CreateText("Test content") | |
.WithAudioMemory(memoryOwner, validSize, "audio/wav", 5.0f); | |
// GetAudioSpan이 정확한 크기를 반환해야 함 | |
var span = segment.GetAudioSpan(); | |
// 요청한 크기만큼 반환되어야 함 | |
Assert.Equal(validSize, span.Length); | |
_output.WriteLine($"요청 크기: {validSize}, 실제 메모리: {actualMemorySize}, 반환된 span 크기: {span.Length}"); | |
} | |
[Fact] | |
public void ChatSegment_GetAudioSpan_SafetyBoundaryTest() | |
{ | |
// 경계 조건 테스트: 유효한 범위 내에서의 메모리 접근 안전성 검증 | |
var testData = GenerateTestAudioData(1000); | |
var memoryOwner = MemoryPool<byte>.Shared.Rent(500); // 소유권은 ChatSegment로 이전됨 | |
var actualMemorySize = memoryOwner.Memory.Length; // 실제 할당된 메모리 크기 | |
var copySize = Math.Min(500, actualMemorySize); | |
testData.AsSpan(0, copySize).CopyTo(memoryOwner.Memory.Span); | |
// 유효한 크기로 설정 (실제 메모리 크기 이하) | |
var validSize = actualMemorySize - 10; // 안전한 크기 | |
var segment = ChatSegment.CreateText("Test content") | |
.WithAudioMemory(memoryOwner, validSize, "audio/wav", 5.0f); | |
try | |
{ | |
// GetAudioSpan이 정확한 크기를 반환해야 함 | |
var span = segment.GetAudioSpan(); | |
// 요청한 크기만큼 반환되어야 함 | |
Assert.Equal(validSize, span.Length); | |
_output.WriteLine($"요청 크기: {validSize}, 실제 메모리: {actualMemorySize}, 반환된 span 크기: {span.Length}"); | |
} | |
finally | |
{ | |
segment.Dispose(); | |
} | |
} |
🤖 Prompt for AI Agents
ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs
lines 88-109: the test currently wraps the IMemoryOwner returned by
MemoryPool<byte>.Shared.Rent(...) in a using, then passes that memoryOwner into
ChatSegment.WithAudioMemory, which transfers ownership and causes a
double-dispose risk; remove the using around memoryOwner (do not dispose it
directly) and instead ensure the ChatSegment created takes responsibility for
disposal by either calling segment.Dispose() at the end of the test or wrapping
the segment in a using so ChatSegment will release the IMemoryOwner; apply the
same pattern to the other occurrences noted (around lines 74, 93, 120, 137,
167).
[Fact] | ||
public void ChatSegment_WithAudioMemory_ValidationTest() | ||
{ | ||
var testData = GenerateTestAudioData(100); | ||
using var memoryOwner = MemoryPool<byte>.Shared.Rent(100); | ||
testData.CopyTo(memoryOwner.Memory.Span); | ||
|
||
// 정상 케이스 | ||
var validSegment = ChatSegment.CreateText("Test") | ||
.WithAudioMemory(memoryOwner, 50, "audio/wav", 1.0f); | ||
Assert.Equal(50, validSegment.AudioDataSize); | ||
|
||
// null audioMemoryOwner 테스트 | ||
var nullException = Assert.Throws<ArgumentNullException>(() => | ||
ChatSegment.CreateText("Test").WithAudioMemory(null!, 100, "audio/wav", 1.0f)); | ||
Assert.Equal("audioMemoryOwner", nullException.ParamName); | ||
|
||
// audioDataSize < 0 테스트 | ||
var negativeException = Assert.Throws<ArgumentOutOfRangeException>(() => | ||
ChatSegment.CreateText("Test").WithAudioMemory(memoryOwner, -1, "audio/wav", 1.0f)); | ||
Assert.Equal("audioDataSize", negativeException.ParamName); | ||
|
||
// audioDataSize > memory.Length 테스트 | ||
var oversizeException = Assert.Throws<ArgumentOutOfRangeException>(() => | ||
ChatSegment.CreateText("Test").WithAudioMemory(memoryOwner, memoryOwner.Memory.Length + 1, "audio/wav", 1.0f)); | ||
Assert.Equal("audioDataSize", oversizeException.ParamName); | ||
|
||
_output.WriteLine("모든 소유권 이전 검증 테스트 통과"); | ||
} | ||
|
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.
검증 테스트에서도 동일한 소유권 이전 규약 위반
정상 케이스 포함 모든 분기에서 using var memoryOwner
를 제거하고, 소유권을 이전한 segment만 Dispose하세요.
적용 예:
-using var memoryOwner = MemoryPool<byte>.Shared.Rent(100);
+var memoryOwner = MemoryPool<byte>.Shared.Rent(100);
...
-var validSegment = ChatSegment.CreateText("Test")
- .WithAudioMemory(memoryOwner, 50, "audio/wav", 1.0f);
+var validSegment = ChatSegment.CreateText("Test")
+ .WithAudioMemory(memoryOwner, 50, "audio/wav", 1.0f);
+try
+{
+ Assert.Equal(50, validSegment.AudioDataSize);
+}
+finally
+{
+ validSegment.Dispose();
+}
예외 경로에서는 WithAudioMemory 호출이 실패하므로 별도 Dispose 불필요합니다.
Committable suggestion skipped: line range outside the PR's diff.
[Fact] | ||
public void ChatSegment_Dispose_MemoryOwnerReleaseTest() | ||
{ | ||
var testData = GenerateTestAudioData(100); | ||
using var memoryOwner = MemoryPool<byte>.Shared.Rent(100); | ||
testData.CopyTo(memoryOwner.Memory.Span); | ||
|
||
var segment = ChatSegment.CreateText("Test") | ||
.WithAudioMemory(memoryOwner, 100, "audio/wav", 1.0f); | ||
|
||
// Dispose 호출 전에는 정상 접근 가능 | ||
Assert.True(segment.HasAudio); | ||
Assert.Equal(100, segment.GetAudioSpan().Length); | ||
|
||
// Dispose 호출 | ||
segment.Dispose(); | ||
|
||
// 메모리가 해제되었으므로 ObjectDisposedException 발생할 수 있음 | ||
// (실제 구현에 따라 다를 수 있음) | ||
_output.WriteLine("Dispose 호출 완료 - 메모리 소유자 해제됨"); | ||
} | ||
|
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.
Dispose 시나리오 테스트의 소유권/해제 순서 정정
이 테스트도 using var memoryOwner
로 이중 해제 위험이 있습니다. segment가 소유하므로 segment만 Dispose해야 합니다. 또한 Dispose 이후 접근 시 예외 던지도록 변경 시 해당 검증을 추가하세요.
-using var memoryOwner = MemoryPool<byte>.Shared.Rent(100);
+var memoryOwner = MemoryPool<byte>.Shared.Rent(100);
...
-var segment = ChatSegment.CreateText("Test")
- .WithAudioMemory(memoryOwner, 100, "audio/wav", 1.0f);
+var segment = ChatSegment.CreateText("Test")
+ .WithAudioMemory(memoryOwner, 100, "audio/wav", 1.0f);
// Dispose 호출 전에는 정상 접근 가능
Assert.True(segment.HasAudio);
Assert.Equal(100, segment.GetAudioSpan().Length);
// Dispose 호출
-segment.Dispose();
+segment.Dispose();
-// 메모리가 해제되었으므로 ObjectDisposedException 발생할 수 있음
-// (실제 구현에 따라 다를 수 있음)
-_output.WriteLine("Dispose 호출 완료 - 메모리 소유자 해제됨");
+// 구현을 Dispose 후 접근 금지로 바꿨다면 다음도 검증:
+Assert.Throws<ObjectDisposedException>(() => segment.GetAudioSpan());
+_output.WriteLine("Dispose 호출 완료 - 메모리 소유자 해제됨");
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
[Fact] | |
public void ChatSegment_Dispose_MemoryOwnerReleaseTest() | |
{ | |
var testData = GenerateTestAudioData(100); | |
using var memoryOwner = MemoryPool<byte>.Shared.Rent(100); | |
testData.CopyTo(memoryOwner.Memory.Span); | |
var segment = ChatSegment.CreateText("Test") | |
.WithAudioMemory(memoryOwner, 100, "audio/wav", 1.0f); | |
// Dispose 호출 전에는 정상 접근 가능 | |
Assert.True(segment.HasAudio); | |
Assert.Equal(100, segment.GetAudioSpan().Length); | |
// Dispose 호출 | |
segment.Dispose(); | |
// 메모리가 해제되었으므로 ObjectDisposedException 발생할 수 있음 | |
// (실제 구현에 따라 다를 수 있음) | |
_output.WriteLine("Dispose 호출 완료 - 메모리 소유자 해제됨"); | |
} | |
[Fact] | |
public void ChatSegment_Dispose_MemoryOwnerReleaseTest() | |
{ | |
var testData = GenerateTestAudioData(100); | |
var memoryOwner = MemoryPool<byte>.Shared.Rent(100); | |
testData.CopyTo(memoryOwner.Memory.Span); | |
var segment = ChatSegment.CreateText("Test") | |
.WithAudioMemory(memoryOwner, 100, "audio/wav", 1.0f); | |
// Dispose 호출 전에는 정상 접근 가능 | |
Assert.True(segment.HasAudio); | |
Assert.Equal(100, segment.GetAudioSpan().Length); | |
// Dispose 호출 | |
segment.Dispose(); | |
// 구현을 Dispose 후 접근 금지로 바꿨다면 다음도 검증: | |
Assert.Throws<ObjectDisposedException>(() => segment.GetAudioSpan()); | |
_output.WriteLine("Dispose 호출 완료 - 메모리 소유자 해제됨"); | |
} |
🤖 Prompt for AI Agents
In ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs
around lines 163-184, the test currently uses "using var memoryOwner" causing
double-disposal since ChatSegment is intended to take ownership; remove the
using so the MemoryOwner is not disposed by the test, create and copy test data
into the rented memory, call segment.Dispose() (not disposing memoryOwner
separately), and then add an assertion that accessing the segment's audio after
Dispose throws ObjectDisposedException (or the appropriate disposal exception
for your implementation) to validate ownership transfer and post-dispose
behavior.
record를 sealed class로 변경하여 'with' 복제 시 메모리 참조 공유 방지 AudioMemoryOwner 접근성을 internal로 제한하여 외부 직접 접근 차단 WithAudioMemory 메서드에서 기존 소유자 자동 해제로 메모리 누수 방지 중복 Dispose 방지를 위한 명시적 소유권 이전 구현
파이널라이저 제거로 종단 큐 압박 및 Gen2 승격 방지 _disposed 플래그 추가로 중복 Dispose 호출 시 멱등성 보장 Dispose 패턴 간소화로 불필요한 virtual 메서드 제거 관리형 리소스(IMemoryOwner)만 해제하는 경량 구현
자명한 코드의 불필요한 XML 문서화 주석 제거 복잡한 로직(소유권 이전, LOH 위험)만 핵심 주석 유지 코드 가독성 향상 및 유지보수 부담 감소
TextToSpeechResponse에 IMemoryOwner 필드 추가 TextToSpeechClient에서 MemoryPool.Shared.Rent 사용 ChatTTSProcessor에서 WithAudioMemory로 변경 이중/삼중 LOH 할당 제거로 GC 압박 대폭 감소
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs (2)
36-42
: HttpResponseMessage 미해제: 연결/소켓 자원 누수 위험응답 객체를 즉시 dispose 해야 커넥션이 풀로 반환됩니다.
- HttpResponseMessage response = await _httpClient.PostAsync($"/v1/text-to-speech/{voiceId}", content); + using var response = await _httpClient.PostAsync($"/v1/text-to-speech/{voiceId}", content);
22-23
: 취소 토큰 지원 권장I/O 경로에 CancellationToken을 통과시켜 상위 취소를 전파하세요(메서드/인터페이스 시그니처 포함).
원한다면 인터페이스까지 포함한 일괄 패치를 제안드리겠습니다.
ProjectVG.Application/Services/Chat/Processors/ChatTTSProcessor.cs (1)
35-43
: I/O 바운드에 Task.Run 불필요 — 순수 async로 팬아웃불필요한 스레드 전환 오버헤드가 있습니다. 순수 async 태스크 수집으로 대체하세요.
- var ttsTasks = new List<Task<(int idx, TextToSpeechResponse)>>(); + var ttsTasks = new List<Task<(int idx, TextToSpeechResponse)>>(); for (int i = 0; i < context.Segments?.Count; i++) { var segment = context.Segments[i]; if (!segment.HasContent) continue; var emotion = NormalizeEmotion(segment.Emotion, profile); int idx = i; - ttsTasks.Add(Task.Run(async () => (idx, await GenerateTTSAsync(profile, segment.Content!, emotion)))); + ttsTasks.Add(GenerateAsync(idx, segment.Content!, emotion)); } + + async Task<(int idx, TextToSpeechResponse)> GenerateAsync(int index, string text, string emo) + { + var resp = await GenerateTTSAsync(profile, text, emo); + return (index, resp); + }
♻️ Duplicate comments (1)
ProjectVG.Application/Models/Chat/ChatSegment.cs (1)
134-139
: Dispose 멱등성 강화: 필드 null/초기화로 후속 접근 안전화해제 후 참조를 null/0으로 초기화하여 HasAudio 등 상태 정합성을 보장하세요.
public void Dispose() { if (_disposed) return; - AudioMemoryOwner?.Dispose(); + AudioMemoryOwner?.Dispose(); + AudioMemoryOwner = null; + AudioDataSize = 0; + AudioData = null; _disposed = true; }
🧹 Nitpick comments (9)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs (3)
59-66
: 헤더 파싱 문화권 문제: InvariantCulture 사용 권장서버가 '.' 소수점을 사용한다면 문화권에 독립적으로 파싱해야 합니다.
- var audioLengthHeader = response.Headers.GetValues("X-Audio-Length").FirstOrDefault(); - if (float.TryParse(audioLengthHeader, out float audioLength)) + var audioLengthHeader = response.Headers.GetValues("X-Audio-Length").FirstOrDefault(); + if (float.TryParse(audioLengthHeader, System.Globalization.NumberStyles.Float, System.Globalization.CultureInfo.InvariantCulture, out float audioLength))추가: 파일 상단에
using System.Globalization;
를 포함하세요.
68-70
: 널 안전 로그 포매팅AudioLength가 null일 때 서식 지정자가 적용되지 않을 수 있습니다. 널 병합으로 안전하게 기록하세요.
- _logger.LogDebug("[TTS][Response] 오디오 길이: {AudioLength:F2}초, ContentType: {ContentType}, 바이트: {Length}, 소요시간: {Elapsed}ms", - voiceResponse.AudioLength, voiceResponse.ContentType, voiceResponse.AudioDataSize, elapsed); + _logger.LogDebug("[TTS][Response] 오디오 길이: {AudioLength:F2}초, ContentType: {ContentType}, 바이트: {Length}, 소요시간: {Elapsed}ms", + voiceResponse.AudioLength ?? 0f, voiceResponse.ContentType, voiceResponse.AudioDataSize, elapsed);
13-15
: 미사용 상수 제거 또는 실제 사용MaxPoolSize는 사용되지 않습니다. 제거하거나 읽기 최대치 등 실제 제약에 활용하세요.
- private const int MaxPoolSize = 1024 * 1024; // 1MB max pooled size
ProjectVG.Application/Services/Chat/Processors/ChatTTSProcessor.cs (3)
49-56
: ContentType 널 방어 및 안전 기본값 지정널 억제 연산자 사용 대신 안전 기본값을 적용하세요.
- context.Segments[idx] = segment.WithAudioMemory( + var contentType = ttsResult.ContentType ?? "application/octet-stream"; + context.Segments[idx] = segment.WithAudioMemory( ttsResult.AudioMemoryOwner, ttsResult.AudioDataSize, - ttsResult.ContentType!, + contentType, ttsResult.AudioLength ?? 0f);
48-66
: 미부착 오디오 메모리 누수 방지성공하지 못했거나 세그먼트가 부착되지 않은 경우 owner를 해제해야 합니다.
- foreach (var (idx, ttsResult) in ttsResults.OrderBy(x => x.idx)) { - if (ttsResult.Success == true && ttsResult.AudioMemoryOwner != null) { + foreach (var (idx, ttsResult) in ttsResults.OrderBy(x => x.idx)) { + if (ttsResult.Success == true && ttsResult.AudioMemoryOwner != null) { var segment = context.Segments?[idx]; if (segment != null && context.Segments != null) { context.Segments[idx] = segment.WithAudioMemory( ttsResult.AudioMemoryOwner, ttsResult.AudioDataSize, ttsResult.ContentType!, ttsResult.AudioLength ?? 0f); } if (ttsResult.AudioLength.HasValue) { var ttsCost = TTSCostInfo.CalculateTTSCost(ttsResult.AudioLength.Value); context.AddCost(ttsCost); - Console.WriteLine($"[TTS_DEBUG] 오디오 길이: {ttsResult.AudioLength.Value:F2}초, TTS 비용: {ttsCost:F0} Cost"); + _logger.LogDebug("[TTS_DEBUG] 오디오 길이: {AudioLength:F2}초, TTS 비용: {TTSCost:F0}", + ttsResult.AudioLength.Value, ttsCost); } processedCount++; - } + } else { + // 부착 실패/미부착 시 즉시 반환 + ttsResult.AudioMemoryOwner?.Dispose(); + } }
62-62
: Console.WriteLine → 로거 사용일관된 로깅 프레임워크 사용이 좋습니다(레벨/구조화 로그).
ProjectVG.Application/Models/Chat/ChatSegment.cs (3)
34-41
: Dispose 이후 접근 가드 및 안전 슬라이스해제된 인스턴스에서 GetAudioSpan 호출 시 빈 Span을 반환하도록 가드하세요. 또한 현재 구현은 안전하지만, 해제 상태 먼저 확인하면 오동작을 더 명확히 방지할 수 있습니다.
public ReadOnlySpan<byte> GetAudioSpan() { + if (_disposed) return ReadOnlySpan<byte>.Empty; if (AudioMemoryOwner != null && AudioDataSize > 0) { - var memory = AudioMemoryOwner.Memory; - var safeSize = Math.Min(AudioDataSize, memory.Length); - return memory.Span.Slice(0, safeSize); + var memory = AudioMemoryOwner.Memory; + var safeSize = Math.Min(AudioDataSize, memory.Length); + return memory.Span.Slice(0, safeSize); }
125-129
: GetAudioDataAsArray 경계 안전성Memory.Length가 AudioDataSize보다 작을 가능성에 대비해 안전 길이를 사용하세요(Span 쪽과 일관).
- var span = AudioMemoryOwner.Memory.Span.Slice(0, AudioDataSize); + var mem = AudioMemoryOwner.Memory; + var safeSize = Math.Min(AudioDataSize, mem.Length); + var span = mem.Span.Slice(0, safeSize); return span.ToArray();
28-33
: HasAudio 계산 일원화(선택)경계/해제 처리를 일관되게 하려면
HasAudio => GetAudioSpan().Length > 0
로 단순화 가능합니다.- public bool HasAudio => (AudioData != null && AudioData.Length > 0) || (AudioMemoryOwner != null && AudioDataSize > 0); + public bool HasAudio => GetAudioSpan().Length > 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ProjectVG.Application/Models/Chat/ChatSegment.cs
(5 hunks)ProjectVG.Application/Services/Chat/Processors/ChatTTSProcessor.cs
(1 hunks)ProjectVG.Infrastructure/Integrations/TextToSpeechClient/Models/TextToSpeechResponse.cs
(2 hunks)ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs
(3 hunks)ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs
🧰 Additional context used
🧬 Code graph analysis (2)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs (1)
ProjectVG.Application/Services/Chat/Processors/ChatTTSProcessor.cs (2)
Task
(20-70)Task
(83-118)
ProjectVG.Application/Models/Chat/ChatSegment.cs (1)
ProjectVG.Application/Services/Chat/Factories/ChatLLMFormat.cs (4)
ChatSegment
(167-244)List
(123-129)List
(131-165)List
(246-253)
🔇 Additional comments (2)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/Models/TextToSpeechResponse.cs (2)
2-2
: 참조 추가 적절IMemoryOwner 사용을 위한 System.Buffers 참조 추가는 타당합니다.
21-21
: 레거시 경로 유지 명확화 좋음AudioData를 “호환성용”으로 명시한 주석 업데이트 적절합니다.
/// <summary> | ||
/// ArrayPool 기반 오디오 메모리 소유자 (LOH 방지) | ||
/// </summary> | ||
[JsonIgnore] | ||
public IMemoryOwner<byte>? AudioMemoryOwner { get; set; } | ||
|
||
/// <summary> | ||
/// 실제 오디오 데이터 크기 | ||
/// </summary> | ||
[JsonIgnore] | ||
public int AudioDataSize { get; set; } |
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.
🛠️ Refactor suggestion
IMemoryOwner 소유권 명시 필요 (잠재적 누수/이중 해제 예방 가이드 추가 권장)
응답 DTO가 IMemoryOwner를 public으로 노출하므로 소유권/수명 규약을 문서화하거나 안전한 전달 API를 제공하는 편이 좋습니다. 예: 소비자가 세그먼트에 붙이지 않고 버릴 때 직접 해제해야 함을 명시하거나, 응답에서 “소유권 가져가기” 메서드를 제공해 중복 해제를 방지하세요.
아래 보조 메서드 추가를 제안드립니다(응답 → 세그먼트로 안전 이전):
// 클래스 내부에 추가
public bool TryTakeAudioOwner(out IMemoryOwner<byte>? owner, out int size)
{
owner = AudioMemoryOwner;
size = AudioDataSize;
AudioMemoryOwner = null;
AudioDataSize = 0;
return owner != null;
}
🤖 Prompt for AI Agents
In
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/Models/TextToSpeechResponse.cs
around lines 26 to 36, the DTO publicly exposes IMemoryOwner<byte> which risks
ownership confusion, leaks or double-dispose; add a clear ownership-transfer API
and docs: implement a TryTakeAudioOwner(out IMemoryOwner<byte>? owner, out int
size) method that returns the current AudioMemoryOwner and AudioDataSize, then
sets AudioMemoryOwner to null and AudioDataSize to 0 so ownership is transferred
safely, update the XML comments to state callers must dispose the returned
owner, and keep the original properties JsonIgnored and internal/private as
appropriate to prevent accidental external disposal.
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs
Outdated
Show resolved
Hide resolved
- MemoryStream 제거하고 IMemoryOwner 직접 증분 복사 방식으로 변경 - 메모리 할당 최적화: 동적 버퍼 크기 조정 및 메모리 재사용 - ArrayPool 활용으로 대규모 힙(LOH) 할당 방지 - 스트림 처리 성능 및 메모리 관리 개선
- ChatSegment의 WithAudioMemory 메서드 상태 정리 로직 추가 - AttachAudioMemory 메서드로 불변 인스턴스 생성 지원 - TextToSpeechResponse에 TryTakeAudioOwner 메서드 추가 - 메모리 누수 및 이중 해제 방지 메커니즘 강화 - 코드 가독성을 위한 주석 간소화
- GC 압박 테스트 제거 - 작은 크기 데이터에서 ArrayPool Base64 구현의 성능 문제 해결 - 속도 중심의 성능 테스트로 변경 - 성능 테스트 로직 간소화 및 명확한 주석 추가 성능 향상에 집중하여 테스트 신뢰성을 높였습니다.
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs (2)
36-43
: HttpResponseMessage 미처리로 소켓/핸들 누수 위험 — using 필요응답을 즉시 Dispose하지 않아 핸들 누수 가능성이 큽니다. using var로 감싸세요.
- HttpResponseMessage response = await _httpClient.PostAsync($"/v1/text-to-speech/{voiceId}", content); + using var response = await _httpClient.PostAsync($"/v1/text-to-speech/{voiceId}", content);
53-66
: 0바이트 오디오를 성공으로 처리함 — 호출자 혼동 유발데이터 없음은 실패로 간주하고 의미 있는 에러 메시지를 설정하세요. 조기 반환 권장.
// ArrayPool 기반으로 음성 데이터 읽기 (LOH 방지) var (memoryOwner, dataSize) = await ReadAudioDataWithPoolAsync(response.Content); - voiceResponse.AudioMemoryOwner = memoryOwner; - voiceResponse.AudioDataSize = dataSize; + if (memoryOwner is null || dataSize <= 0) + { + voiceResponse.Success = false; + voiceResponse.ErrorMessage = "TTS 응답에 오디오 데이터가 없습니다."; + _logger.LogDebug("[TTS] 빈 오디오 응답 수신"); + return voiceResponse; + } + voiceResponse.AudioMemoryOwner = memoryOwner; + voiceResponse.AudioDataSize = dataSize;ProjectVG.Application/Models/Chat/ChatSegment.cs (1)
1-171
: TryTakeAudioOwner로 IMemoryOwner 소유권을 명시적으로 이전하세요.현재 ChatTTSProcessor가 ttsResult.AudioMemoryOwner를 직접 전달해 TextToSpeechResponse에 참조가 남아 있습니다. ttsResult.TryTakeAudioOwner(out var owner, out var size)를 호출해 소유권을 가져온 뒤(success 시) segment.WithAudioMemory(owner, size, ttsResult.ContentType!, ttsResult.AudioLength ?? 0f)로 교체해 중복 Dispose/ArrayPool 반환을 방지하세요.
수정 위치: ProjectVG.Application/Services/Chat/Processors/ChatTTSProcessor.cs (context.Segments[idx] = segment.WithAudioMemory(...)).
♻️ Duplicate comments (2)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/Models/TextToSpeechResponse.cs (1)
60-73
: 소유권 이전 API(TryTakeAudioOwner) 추가 적절 — 이전 코멘트 해결됨응답에서 안전한 소유권 이전을 제공하고, 내부 상태를 null/0으로 정리하는 구현이 적절합니다.
ProjectVG.Application/Models/Chat/ChatSegment.cs (1)
88-90
: WithAudioMemory/AttachAudioMemory 중복 및 부작용 — API 일관화부작용 있는 With* 명명은 혼동을 줍니다. AttachAudioMemory만 유지 또는 WithAudioMemory를 폐기/Obsolete 처리 권장.
- // 주의: 원본 인스턴스의 AudioMemoryOwner 해제됨 + // 주의: 원본 인스턴스의 AudioMemoryOwner 해제됨 + [System.Obsolete("원본 불변인 AttachAudioMemory(...) 사용 권장")] public ChatSegment WithAudioMemory(IMemoryOwner<byte> audioMemoryOwner, int audioDataSize, string audioContentType, float audioLength)또한 호출부(예: ChatTTSProcessor)에서 AttachAudioMemory 사용으로 이관 권장.
🧹 Nitpick comments (9)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/Models/TextToSpeechResponse.cs (1)
29-33
: 문서에 소비자 책임(Dispose) 명시 보강 제안요약 주석에 “TryTakeAudioOwner로 가져간 경우 호출자가 Dispose해야 함”을 한 줄 추가해 오용을 줄이세요.
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs (3)
62-65
: 헤더 파싱 문화권 의존성 — InvariantCulture 사용HTTP 헤더 숫자 파싱은 문화권 독립적으로 처리해야 합니다.
- if (float.TryParse(audioLengthHeader, out float audioLength)) + if (float.TryParse(audioLengthHeader, System.Globalization.NumberStyles.Float, System.Globalization.CultureInfo.InvariantCulture, out float audioLength)) { voiceResponse.AudioLength = audioLength; }추가(파일 상단):
using System.Globalization;
68-70
: 널 길이 로그 포맷 안전화AudioLength가 null일 수 있으므로 안전 값으로 로깅하세요.
- _logger.LogDebug("[TTS][Response] 오디오 길이: {AudioLength:F2}초, ContentType: {ContentType}, 바이트: {Length}, 소요시간: {Elapsed}ms", - voiceResponse.AudioLength, voiceResponse.ContentType, voiceResponse.AudioDataSize, elapsed); + _logger.LogDebug("[TTS][Response] 오디오 길이: {AudioLength:F2}초, ContentType: {ContentType}, 바이트: {Length}, 소요시간: {Elapsed}ms", + voiceResponse.AudioLength ?? 0f, voiceResponse.ContentType, voiceResponse.AudioDataSize, elapsed);
14-14
: 미사용 상수 제거MaxPoolSize가 사용되지 않습니다. 제거하거나 실제 확장 상한으로 활용하세요(후자는 신중 권장).
- private const int MaxPoolSize = 1024 * 1024; // 1MB max pooled size
ProjectVG.Application/Models/Chat/ChatSegment.cs (5)
34-47
: Dispose 후 접근 보호 필요 — ObjectDisposedException 가드 추가해제된 인스턴스에서 GetAudioSpan 호출 시 사용자가 조기에 이슈를 발견하도록 가드를 두세요.
public ReadOnlySpan<byte> GetAudioSpan() { + if (_disposed) throw new ObjectDisposedException(nameof(ChatSegment)); if (AudioMemoryOwner != null && AudioDataSize > 0) { - var memory = AudioMemoryOwner.Memory; - var safeSize = Math.Min(AudioDataSize, memory.Length); - return memory.Span.Slice(0, safeSize); + var memory = AudioMemoryOwner.Memory; + var safeSize = Math.Min(AudioDataSize, memory.Length); + return memory.Span.Slice(0, safeSize); }
32-33
: Linq Any() 제거로 미세 최적화List.Count 사용이 더 경량입니다.
- public bool HasActions => Actions != null && Actions.Any(); + public bool HasActions => Actions != null && Actions.Count > 0;
100-105
: WithAudioMemory 원본 상태 정합성 보완소유자/크기만 초기화하면 메타데이터가 낡은 값으로 남습니다. 함께 정리하세요.
// 기존 소유자 해제 및 상태 정리 this.AudioMemoryOwner?.Dispose(); this.AudioMemoryOwner = null; this.AudioDataSize = 0; + this.AudioContentType = null; + this.AudioLength = null; + this.AudioData = null;
147-159
: 배열 변환 시 안전 길이 사용 및 Dispose 후 접근 보호메모리 길이와 AudioDataSize 불일치에 대비하고, 해제 후 호출을 방지하세요.
public byte[]? GetAudioDataAsArray() { + if (_disposed) throw new ObjectDisposedException(nameof(ChatSegment)); if (AudioData != null) { return AudioData; } if (AudioMemoryOwner != null && AudioDataSize > 0) { - var span = AudioMemoryOwner.Memory.Span.Slice(0, AudioDataSize); - return span.ToArray(); + var memory = AudioMemoryOwner.Memory; + var safeSize = Math.Min(AudioDataSize, memory.Length); + return memory.Span.Slice(0, safeSize).ToArray(); } return null; }
164-169
: Dispose 이후 참조 차단해제 후에도 필드가 남아 있으면 오용 가능성이 큽니다. null로 정리하세요.
public void Dispose() { if (_disposed) return; - AudioMemoryOwner?.Dispose(); + AudioMemoryOwner?.Dispose(); + AudioMemoryOwner = null; + AudioDataSize = 0; _disposed = true; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ProjectVG.Application/Models/Chat/ChatSegment.cs
(5 hunks)ProjectVG.Infrastructure/Integrations/TextToSpeechClient/Models/TextToSpeechResponse.cs
(3 hunks)ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs (3)
ProjectVG.Infrastructure/Realtime/WebSocketConnection/WebSocketClientConnection.cs (3)
Task
(30-53)Task
(58-61)Task
(66-83)ProjectVG.Application/Services/Chat/Processors/ChatTTSProcessor.cs (2)
Task
(20-70)Task
(83-118)ProjectVG.Application/Models/Chat/ChatSegment.cs (1)
Dispose
(164-169)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/Models/TextToSpeechResponse.cs (1)
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs (2)
TextToSpeechClient
(9-205)TextToSpeechClient
(16-20)
ProjectVG.Application/Models/Chat/ChatSegment.cs (1)
ProjectVG.Application/Services/Chat/Factories/ChatLLMFormat.cs (4)
ChatSegment
(167-244)List
(123-129)List
(131-165)List
(246-253)
🔇 Additional comments (2)
ProjectVG.Application/Models/Chat/ChatSegment.cs (1)
6-24
: record→sealed class 전환 및 내부 세터 적용 적절복제/수명 제어에 유리하며, 메모리 소유권 안정성이 개선됩니다.
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs (1)
84-145
: 취소 토큰 전파 필요 — 스트림 읽기에서 CancellationToken 전달하세요메서드 시그니처에 CancellationToken을 추가하고 content.ReadAsStreamAsync(token) 및 stream.ReadAsync(readBuffer, 0, toRead, token) 등 모든 비동기 I/O 호출에 전달해 상위 요청 취소 시 즉시 중단되게 하세요. 이 변경은 해당 메서드를 호출하는 상위 호출부(예: Predict/Generate)까지 토큰을 전파해야 합니다.
[JsonIgnore] | ||
public IMemoryOwner<byte>? AudioMemoryOwner { get; set; } | ||
|
||
/// <summary> | ||
/// 실제 오디오 데이터 크기 | ||
/// </summary> | ||
[JsonIgnore] | ||
public int AudioDataSize { get; set; } |
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.
🛠️ Refactor suggestion
IMemoryOwner/크기 세터 접근 축소로 오·남용 방지
소비자가 임의로 교체하면 이중 Dispose/누수 가능성이 있습니다. 동일 어셈블리에서만 설정되도록 setter를 internal로 축소하세요.
- public IMemoryOwner<byte>? AudioMemoryOwner { get; set; }
+ public IMemoryOwner<byte>? AudioMemoryOwner { get; internal set; }
- public int AudioDataSize { get; set; }
+ public int AudioDataSize { get; internal set; }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
[JsonIgnore] | |
public IMemoryOwner<byte>? AudioMemoryOwner { get; set; } | |
/// <summary> | |
/// 실제 오디오 데이터 크기 | |
/// </summary> | |
[JsonIgnore] | |
public int AudioDataSize { get; set; } | |
[JsonIgnore] | |
public IMemoryOwner<byte>? AudioMemoryOwner { get; internal set; } | |
/// <summary> | |
/// 실제 오디오 데이터 크기 | |
/// </summary> | |
[JsonIgnore] | |
public int AudioDataSize { get; internal set; } |
🤖 Prompt for AI Agents
In
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/Models/TextToSpeechResponse.cs
around lines 33 to 40, the public setters for AudioMemoryOwner and AudioDataSize
should be restricted to prevent external replacement and potential
double-dispose/leak; change both properties so only code within the same
assembly can set them (make their setters internal) while keeping their getters
public.
var newOwner = MemoryPool<byte>.Shared.Rent(Math.Min(owner.Memory.Length * 2, int.MaxValue)); | ||
owner.Memory.Span.Slice(0, total).CopyTo(newOwner.Memory.Span); | ||
owner.Dispose(); | ||
owner = newOwner; | ||
} |
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.
버퍼 확장 시 int 오버플로 가능성 — 안전한 배수 계산 필요
owner.Memory.Length*2에서 오버플로가 발생하면 음수 크기로 Rent 호출될 수 있습니다.
- var newOwner = MemoryPool<byte>.Shared.Rent(Math.Min(owner.Memory.Length * 2, int.MaxValue));
+ int cur = owner.Memory.Length;
+ int next = (cur > (int.MaxValue / 2)) ? int.MaxValue : cur * 2;
+ var newOwner = MemoryPool<byte>.Shared.Rent(next);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
var newOwner = MemoryPool<byte>.Shared.Rent(Math.Min(owner.Memory.Length * 2, int.MaxValue)); | |
owner.Memory.Span.Slice(0, total).CopyTo(newOwner.Memory.Span); | |
owner.Dispose(); | |
owner = newOwner; | |
} | |
int cur = owner.Memory.Length; | |
int next = (cur > (int.MaxValue / 2)) ? int.MaxValue : cur * 2; | |
var newOwner = MemoryPool<byte>.Shared.Rent(next); | |
owner.Memory.Span.Slice(0, total).CopyTo(newOwner.Memory.Span); | |
owner.Dispose(); | |
owner = newOwner; | |
} |
🤖 Prompt for AI Agents
In
ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs
around lines 106 to 110, the expression owner.Memory.Length * 2 can overflow
causing Rent to be called with a negative size; compute the grown buffer size
safely by using either a checked long multiplication or an overflow-safe branch:
determine newSize = owner.Memory.Length >= int.MaxValue/2 ? int.MaxValue :
owner.Memory.Length * 2, then ensure newSize is at least total (newSize =
Math.Max(newSize, total)) and finally cast to int before calling
MemoryPool<byte>.Shared.Rent(newSize); replace the existing multiplication with
this safe size calculation and keep the subsequent copy, dispose and
reassignment logic.
Summary by CodeRabbit
버그 수정
리팩터
기능 개선
문서
테스트