-
-
Notifications
You must be signed in to change notification settings - Fork 15.9k
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
Introduce a native transport for linux using epoll ET #2229
Conversation
@@ -0,0 +1,116 @@ | |||
<?xml version="1.0" encoding="ISO-8859-15"?> | |||
<!-- | |||
Licensed to the Apache Software Foundation (ASF) under one |
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.
licensed to the ASF ? :-)
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.
Ups... :)
Am 13.02.2014 um 15:44 schrieb Matthias Wessendorf notifications@github.com:
In transport-native-epoll/pom.xml:
@@ -0,0 +1,116 @@
+
+<!--
- Licensed to the Apache Software Foundation (ASF) under one
licensed to the ASF ? :-)—
Reply to this email directly or view it on GitHub.
@trustin you need to pimp up our CI so it can built it :)
|
Interesting...my tries at integrating native code compilation & maven has failed because all maven jni plugins were too buggy(namely: nar-maven-plugin). Do you have any benchmarks yet @normanmaurer ? cc @md-5 |
@ninja- no "official" benchmarks yet, I will write an blog post with benchmarks soon. To make it short it is faster then "plain nio" and produce less GC. |
I installed necessary tools to the CI machine and the build still fails. It seems like it's related with working directory? |
Nevermind - CI machine did not have |
From the error it looks like you missed to install autoconf..
|
Ok .. All good now? Be sure to install libs for 32 and 64 bit :)
|
@trustin I think you are still missing: |
@trustin ok now all bugs are fixed... :) |
* Helper class to load JNI resources. | ||
* | ||
*/ | ||
public final class JNILoader { |
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.
JniLoader
setOption(env, fd, SOL_SOCKET, SO_KEEPALIVE, &optval, sizeof(optval)); | ||
} | ||
|
||
JNIEXPORT void JNICALL Java_io_netty_channel_epoll_Native_setTCPCork(JNIEnv *env, jclass clazz, jint fd, jint optval) { |
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.
Shouldn't this be setTcpCork
? I guess you renamed some methods after generating this header file.
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.
Yes let me fix it
Could we also add and support an arbitrary option? We could expose the following operations to a user:
For example:
|
@trustin I guess we could but I would like to be careful and test this in more detail. Just it is not too easy to kill the JVM. |
@normanmaurer I see. Maybe we can add it later when it's on demand. |
# sudo yum install autoconf automake libtool glibc-devel.i686 glibc-devel libgcc.i686 make | ||
|
||
__Debian(64Bit)__ | ||
# sudo apt-get install autoconf automake libtool make gcc-multilib |
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.
Could you move this to the dedicated wiki page and add a link to it - just like we did for microbench: https://github.com/netty/netty/blob/master/microbench/README.md
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.
Sure..
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.
Additionally, you could add some stuff like how to switch to the Epoll transport (and how easy it is) by a simple example.
@trustin so ready to merge ? If so I will squash and merge :) |
|
||
@Override | ||
public EpollSocketChannelConfig setTrafficClass(int trafficClass) { | ||
return this; |
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.
Could this be implemented? Read:
super(channel); | ||
|
||
this.channel = channel; | ||
setTcpNoDelay(true); |
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.
Could you wrap this with:
if (PlatformDependent.canEnableTcpNoDelayByDefault()) { .. }
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.
Will do
Am 14.02.2014 um 21:12 schrieb Trustin Lee notifications@github.com:
In transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannelConfig.java:
+import static io.netty.channel.ChannelOption.*;
+
+public final class EpollSocketChannelConfig extends DefaultChannelConfig
implements SocketChannelConfig {
- protected final EpollSocketChannel channel;
- private volatile boolean allowHalfClosure;
- /**
\* Creates a new instance.
*/
- EpollSocketChannelConfig(EpollSocketChannel channel) {
super(channel);
this.channel = channel;
Could you wrap this with:setTcpNoDelay(true);
if (PlatformDependent.canEnableTcpNoDelayByDefault()) { .. }
—
Reply to this email directly or view it on GitHub.
This transport use JNI (C) to directly make use of epoll in Edge-Triggered mode for maximal performance on Linux. Beside this it also support using TCP_CORK and produce less GC then the NIO transport using JDK NIO. It only builds on linux and skip the build if linux is not used. The transport produce a jar which contains all needed .so files for 32bit and 64 bit. The user only need to include the jar as dependency as usually to make use of it and use the correct classes. This includes also some cleanup of @trustin
Addressed all comments, squashed and merged into 4.0, 4.1 and master.. Thanks for the review guys! |
@normanmaurer, Mr. @bnoordhuis said in libuv#485#issuecomment-9198365 that he was reconsidering edge-triggered mode in libuv. It reduces the number of system calls substantially but it slows down some benchmarks because a lot more time is spent inside kernel-side spinlocks. Had you been aware of this issue? |
@dawud-tan we not saw any of his observations. That said you can use LT and ET in Netty. You can configure it via a |
The scalability issues have, by and large, been addressed in newer kernels. I profiled on (IIRC) a stock 2.6.32 kernel. |
@bnoordhuis thanks! This explains why I not saw these yet I guess :) |
This transport use JNI (C) to directly make use of epoll in Edge-Triggered mode for maximal performance on Linux. Beside this it also support using TCP_CORK and produce less GC then the NIO transport using JDK NIO.
It only builds on linux and skip the build if linux is not used. The transport produce a jar which contains all needed .so files for 32bit and 64 bit. The user only need to include the jar as dependency as usually
to make use of it and use the correct classes.