Add support for Unix Domain Sockets when using native epoll transport #3344
Conversation
|
@trustin please review |
| if((ctrl_msg->cmsg_level == SOL_SOCKET) && (ctrl_msg->cmsg_type == SCM_RIGHTS)) { | ||
| socketFd = *((int *) CMSG_DATA(ctrl_msg)); | ||
| // set as non blocking as we want to use it with epoll | ||
| if (fcntl(socketFd, F_SETFL, O_NONBLOCK) == -1) { |
There was a problem hiding this comment.
Do we have a consensus on ioctl vs fcntl (i.e. always use fcntl for portability/standardization reasons)? Word on the street is ioctl can sometimes be less expensive for this case.
There was a problem hiding this comment.
Nope... can you tell me more about these "word on the street" ? Maybe links etc...
There was a problem hiding this comment.
@normanmaurer - Sure. For example see nginx comment at the top.
The "word on the street" expression is because I didn't have anything concrete but was also interested if you had any feed back (been outstanding question of mine, and haven't dug into the kernel to investigate).
There was a problem hiding this comment.
That's interesting... did not know this. I guess we should use ioctl then :)
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes I also looked at that stackoverflow post. The comments in nginx were more convincing to me (listing the calls that fcntl does). I think for setting non-blocking ioctl is the way to go, but unfortunately I'm not sure if ioctl is always better.
There was a problem hiding this comment.
Let me open another issue for that. I would love to investigate a bit more here before doing any changes that may affect things in a bad way.
|
@normanmaurer Nice! This is required in netflix, so its great that you got it going! |
|
@NiteshKant BOOM ! ;) |
| break; | ||
| } | ||
| expectedWrittenBytes -= localWrittenBytes; | ||
| writtenBytes += localWrittenBytes; |
There was a problem hiding this comment.
To avoid redundant computation would it make sense to compute this once at the end? For example:
final long initialExpectedWrittenBytes = expectedWrittenBytes;
for (;;) { /*do writes and decrement expectedWrittenBytes*/ }
in.removeBytes(initialExpectedWrittenBytes - expectedWrittenBytes);There was a problem hiding this comment.
I guess yeah we could do this. Not sure it makes any difference though
There was a problem hiding this comment.
Just less computation and memory access required. A reduction from O(n) load/add/store operations (where n is the number of iterations required for this loop) to O(1).
There was a problem hiding this comment.
@Scottmitch done... I guess we should do the same in nio from which this code derived :)
| return fd; | ||
| } | ||
|
|
||
| JNIEXPORT jint JNICALL Java_io_netty_channel_epoll_Native_bindDomainSocket(JNIEnv * env, jclass clazz, jint fd, jstring socketPath) { |
There was a problem hiding this comment.
I see:
JNIEnv * env,JNIEnv *env, andJNIEnv* env
Could you use JNIEnv* env everywhere?
There was a problem hiding this comment.
Same for other pointer declarations, such as const char*
|
First pass done. Please ping me once ready for another pass. |
|
@trustin ping :) |
Motivation: Using Unix Domain Sockets can be very useful when communication should take place on the same host and has less overhead then using loopback. We should support this with the native epoll transport. Modifications: - Add support for Unix Domain Sockets. - Adjust testsuite to be able to reuse tests. Result: Unix Domain Sockets are now support when using native epoll transport.
normanmaurer
force-pushed
the
domain_socket_1
branch
from
217e176
to
5f2be6a
Compare
|
i want to got a link for Unix domain Socket guide demo ,use netty! |
Motivation:
Using Unix Domain Sockets can be very useful when communication should take place on the same host and has less overhead then using loopback. We should support this with the native epoll transport.
Modifications:
Result:
Unix Domain Sockets are now support when using native epoll transport.