feat(ITcpSocketClient): use ReadOnlyMemory improve performance #6264
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideRefactors TCP socket client and data package handler APIs and implementations from Task/Memory to ValueTask/ReadOnlyMemory for improved performance, updating interfaces, default client, handlers, and unit tests accordingly. Class diagram for updated ITcpSocketClient and DefaultTcpSocketClient APIsclassDiagram
class ITcpSocketClient {
+ValueTask<bool> ConnectAsync(string host, int port, CancellationToken token = default)
+ValueTask<bool> ConnectAsync(IPEndPoint endPoint, CancellationToken token = default)
+ValueTask<bool> SendAsync(ReadOnlyMemory<byte> data, CancellationToken token = default)
+void Dispose()
}
class DefaultTcpSocketClient {
+ValueTask<bool> ConnectAsync(string host, int port, CancellationToken token = default)
+ValueTask<bool> ConnectAsync(IPEndPoint endPoint, CancellationToken token = default)
+ValueTask<bool> SendAsync(ReadOnlyMemory<byte> data, CancellationToken token = default)
-ValueTask ReceiveAsync()
+void SetDataHandler(IDataPackageHandler handler)
}
ITcpSocketClient <|.. DefaultTcpSocketClient
Class diagram for updated IDataPackageHandler and DataPackageHandlerBaseclassDiagram
class IDataPackageHandler {
+Func<ReadOnlyMemory<byte>, ValueTask>? ReceivedCallBack
+ValueTask<ReadOnlyMemory<byte>> SendAsync(ReadOnlyMemory<byte> data)
+ValueTask ReceiveAsync(ReadOnlyMemory<byte> data)
}
class DataPackageHandlerBase {
+Func<ReadOnlyMemory<byte>, ValueTask>? ReceivedCallBack
+virtual ValueTask<ReadOnlyMemory<byte>> SendAsync(ReadOnlyMemory<byte> data)
+virtual ValueTask ReceiveAsync(ReadOnlyMemory<byte> data)
-void SlicePackage(ReadOnlyMemory<byte> buffer, int length)
-ReadOnlyMemory<byte> ConcatBuffer(ReadOnlyMemory<byte> buffer)
}
IDataPackageHandler <|.. DataPackageHandlerBase
Class diagram for updated DelimiterDataPackageHandler and FixLengthDataPackageHandlerclassDiagram
class DelimiterDataPackageHandler {
+override ValueTask ReceiveAsync(ReadOnlyMemory<byte> data)
}
class FixLengthDataPackageHandler {
+override ValueTask ReceiveAsync(ReadOnlyMemory<byte> data)
}
DataPackageHandlerBase <|-- DelimiterDataPackageHandler
DataPackageHandlerBase <|-- FixLengthDataPackageHandler
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `test/UnitTest/Services/TcpSocketFactoryTest.cs:75` </location>
<code_context>
// 测试未建立连接前调用 SendAsync 方法报异常逻辑
- var data = new Memory<byte>([1, 2, 3, 4, 5]);
- var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => client.SendAsync(data));
+ var data = new ReadOnlyMemory<byte>([1, 2, 3, 4, 5]);
+ var ex = await Assert.ThrowsAsync<InvalidOperationException>(async () => await client.SendAsync(data));
Assert.Equal("TCP Socket is not connected 127.0.0.1:0", ex.Message);
</code_context>
<issue_to_address>
Consider adding tests for empty ReadOnlyMemory<byte> input.
Testing with empty ReadOnlyMemory<byte> will verify that SendAsync and ReceiveAsync handle empty buffers correctly and do not throw unexpected exceptions.
Suggested implementation:
```csharp
var data = new ReadOnlyMemory<byte>([1, 2, 3, 4, 5]);
var ex = await Assert.ThrowsAsync<InvalidOperationException>(async () => await client.SendAsync(data));
Assert.Equal("TCP Socket is not connected 127.0.0.1:0", ex.Message);
// 测试未建立连接前调用 SendAsync 方法，传入空的 ReadOnlyMemory<byte>
var emptyData = ReadOnlyMemory<byte>.Empty;
var exEmpty = await Assert.ThrowsAsync<InvalidOperationException>(async () => await client.SendAsync(emptyData));
Assert.Equal("TCP Socket is not connected 127.0.0.1:0", exEmpty.Message);
// 测试未建立连接前调用 ReceiveAsync 方法，传入空的 Memory<byte>
var emptyBuffer = Memory<byte>.Empty;
var exReceive = await Assert.ThrowsAsync<InvalidOperationException>(async () => await client.ReceiveAsync(emptyBuffer));
Assert.Equal("TCP Socket is not connected 127.0.0.1:0", exReceive.Message);
}
```
```csharp
var data = new ReadOnlyMemory<byte>([1, 2, 3, 4, 5]);
var result = await client.SendAsync(data, cst.Token);
Assert.False(result);
// 测试取消时传入空的 ReadOnlyMemory<byte>
var emptyData = ReadOnlyMemory<byte>.Empty;
var resultEmpty = await client.SendAsync(emptyData, cst.Token);
Assert.False(resultEmpty);
// 测试取消时传入空的 Memory<byte> 给 ReceiveAsync
var emptyBuffer = Memory<byte>.Empty;
var receiveResult = await client.ReceiveAsync(emptyBuffer, cst.Token);
Assert.Equal(0, receiveResult);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6264 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 711 711
Lines 31374 31374
Branches 4431 4431
=========================================
Hits 31374 31374
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #6263
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Refactor TCP socket client and data package handler APIs to leverage ReadOnlyMemory and ValueTask, aiming to enhance performance and resource efficiency across the implementation and tests.
Enhancements:
Tests: