Support netty io_uring #1405
Support netty io_uring #1405
Conversation
|
Important Auto Review SkippedReview was skipped due to path filters Files ignored due to path filters (1)
WalkthroughThis update introduces the experimental support for Changes
Related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
bom/pom.xmlis excluded by:!**/*.xmlremoting/remoting-http/pom.xmlis excluded by:!**/*.xml
Files selected for processing (4)
- core/api/src/main/java/com/alipay/sofa/rpc/common/RpcOptions.java (1 hunks)
- remoting/remoting-http/src/main/java/com/alipay/sofa/rpc/transport/http/AbstractHttp2ClientTransport.java (2 hunks)
- remoting/remoting-http/src/main/java/com/alipay/sofa/rpc/transport/netty/NettyHelper.java (3 hunks)
- remoting/remoting-http/src/test/java/com/alipay/sofa/rpc/transport/netty/NettyHelperTest.java (1 hunks)
Additional comments: 3
remoting/remoting-http/src/main/java/com/alipay/sofa/rpc/transport/netty/NettyHelper.java (1)
- 218-227: The
socketChannelmethod dynamically selects the socket channel class based on configuration flags and system capabilities. Adding documentation to clarify the selection process for the socket channel class would enhance code readability and maintainability.remoting/remoting-http/src/main/java/com/alipay/sofa/rpc/transport/http/AbstractHttp2ClientTransport.java (1)
- 146-146: The change to dynamically select the channel class using
NettyHelper.socketChannel()aligns with the PR's objectives and enhances the flexibility of the transport layer. This is a good practice that allows for better adaptability to different environments and configurations.core/api/src/main/java/com/alipay/sofa/rpc/common/RpcOptions.java (1)
- 399-404: The addition of the
TRANSPORT_USE_IO_URINGconstant is well-aligned with the existing codebase conventions and the PR's objectives to introduceio_uringsupport. The naming, documentation, and placement within the file are appropriate.
| @Test | ||
| public void testEventLoopGroup() { | ||
| System.setProperty("os.name", "linux111"); | ||
| System.setProperty(TRANSPORT_USE_IO_URING, "true"); | ||
|
|
||
| EventLoopGroup eventLoopGroup = NettyHelper.eventLoopGroup(1, new NamedThreadFactory("test", true)); | ||
| Class<? extends SocketChannel> socketChannel = NettyHelper.socketChannel(); | ||
| if (IOUring.isAvailable()) { | ||
| Assert.assertTrue(eventLoopGroup instanceof IOUringEventLoopGroup); | ||
| Assert.assertEquals(IOUringSocketChannel.class, socketChannel); | ||
| } else { | ||
| Assert.assertTrue(eventLoopGroup instanceof NioEventLoopGroup); | ||
| Assert.assertEquals(NioSocketChannel.class, socketChannel); | ||
| } | ||
| } |
There was a problem hiding this comment.
Manipulating system properties directly within a test can lead to unintended side effects, especially when tests are run in parallel. Consider using a setup and teardown approach to ensure system properties are reset after the test. Additionally, it would be beneficial to include test scenarios that cover cases when io_uring is not available or when running on a non-Linux system to ensure comprehensive coverage.
| public synchronized static EventLoopGroup eventLoopGroup(int threads, NamedThreadFactory threadName) { | ||
| boolean useEpoll = getBooleanValue(TRANSPORT_USE_EPOLL); | ||
| boolean useIoUring = getBooleanValue(TRANSPORT_USE_IO_URING); | ||
| if (useEpoll) { | ||
| return new EpollEventLoopGroup(threads, threadName); | ||
| } else if (useIoUring && SystemInfo.isLinux() && IOUring.isAvailable()) { | ||
| return new IOUringEventLoopGroup(threads, threadName); | ||
| } else { | ||
| return new NioEventLoopGroup(threads, threadName); | ||
| } |
There was a problem hiding this comment.
The eventLoopGroup method dynamically selects the event loop group based on configuration flags and system capabilities. It would be beneficial to add documentation explaining the precedence of useEpoll over useIoUring and under what conditions each is used. Additionally, consider handling the case where both useEpoll and useIoUring are false more explicitly to improve code clarity.
|
看一下dubbo的issue Introduce I/O URing to netty4 remoting |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
related #1349
Summary by CodeRabbit