-
-
Notifications
You must be signed in to change notification settings - Fork 15.8k
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
Allow domain sockets to configure SO_SNDBUF and SO_RCVBUF #9584
Allow domain sockets to configure SO_SNDBUF and SO_RCVBUF #9584
Conversation
Can one of the admins verify this patch? |
transport-native-unix-common/src/main/java/io/netty/channel/unix/DomainSocketChannelConfig.java
Outdated
Show resolved
Hide resolved
transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDomainSocketChannelConfig.java
Show resolved
Hide resolved
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.
Please also sign our icla: https://netty.io/s/icla
...ort-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueDomainSocketChannelConfig.java
Show resolved
Hide resolved
transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDomainSocketChannelConfig.java
Outdated
Show resolved
Hide resolved
Thanks for the review @normanmaurer. I signed the icla and addressed your comments. Let me know if there's anything else you'd like me to follow up with. |
I will just do it directly in master as a followup
… Am 20.09.2019 um 15:49 schrieb Joe Ellis ***@***.***>:
@ellisjoe commented on this pull request.
In transport-native-unix-common/src/main/java/io/netty/channel/unix/DomainSocketChannelConfig.java:
> @@ -77,4 +79,24 @@
* Return the ***@***.*** DomainSocketReadMode} for the channel.
*/
DomainSocketReadMode getReadMode();
removed. is there somewhere you'd like to track adding this in a future release?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDomainSocketChannelConfig.java
Outdated
Show resolved
Hide resolved
...ort-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueDomainSocketChannelConfig.java
Outdated
Show resolved
Hide resolved
@netty-bot test this please |
@ellisjoe thanks a lot! |
Motivation: Running tests with a `KQueueDomainSocketChannel` showed worse performance than an `NioSocketChannel`. It turns out that the default send buffer size for Nio sockets is 64k while for KQueue sockets it's 8k. I verified that manually setting the socket's send buffer size improved perf to expected levels. Modification: Plumb the `SO_SNDBUF` and `SO_RCVBUF` options into the `*DomainSocketChannelConfig`. Result: Can now configure send and receive buffer sizes for domain sockets.
Motivation:
Running tests with a
KQueueDomainSocketChannel
showed worse performance than anNioSocketChannel
. It turns out that the default send buffer size for Nio sockets is 64k while for KQueue sockets it's 8k. I verified that manually setting the socket's send buffer size improved perf to expected levels.Modification:
Plumb the
SO_SNDBUF
andSO_RCVBUF
options into theDomainSocketChannelConfig
.Result:
Can now configure send and receive buffer sizes for domain sockets.