TINKERPOP 2498: Checks if Epoll is available and uses it instead of Nio #3
Conversation
| public final EventLoopGroup group; | ||
| // Checks and uses Epoll if it is available. ref: http://netty.io/wiki/native-transports.html | ||
| // Subclasses also depend on if Epoll is available or not, so making this protected | ||
| protected boolean isEpollAvailable; |
There was a problem hiding this comment.
I'm not quite convinced this is necessary as it's just a wrapper around Epoll.isAvailable().
Or, in other words, what is kind of concerning to me in this PR is that we are calling Epoll.isAvailable() in many places, but then again it looks like the logic of what to do after checking it is difference so having this consolidated in one place doesn't even seem really feasible.
There was a problem hiding this comment.
I guess there isn't any difference, so you mean to just use Epoll.isAvailable() directly?
There was a problem hiding this comment.
Yeah.
Like, it would make sense to wrap it if we had like a function that returns a EventLoopGroup pool and it abstracted this away, but it looks like we still end up needing to call Epoll.isAvailable() directly anyways so it's pointless.
There was a problem hiding this comment.
Makes sense, I'll change them then.
| final BasicThreadFactory threadFactory = new BasicThreadFactory.Builder().namingPattern("gremlin-driver-loop-%d").build(); | ||
| group = new NioEventLoopGroup(nioPoolSize, threadFactory); | ||
| // Checks and uses Epoll if it is available. ref: http://netty.io/wiki/native-transports.html | ||
| group = Epoll.isAvailable()? new EpollEventLoopGroup(nioPoolSize, threadFactory) : new NioEventLoopGroup(nioPoolSize, threadFactory); |
| final BasicThreadFactory threadFactory = new BasicThreadFactory.Builder().namingPattern(threadPattern).build(); | ||
| group = new NioEventLoopGroup(Runtime.getRuntime().availableProcessors(), threadFactory); | ||
| // Checks and uses Epoll if it is available. ref: http://netty.io/wiki/native-transports.html | ||
| group = Epoll.isAvailable()? new EpollEventLoopGroup(Runtime.getRuntime().availableProcessors(), threadFactory) |
| final MessageSerializer serializer = new GryoMessageSerializerV3d0(); | ||
| b.channel(NioSocketChannel.class) | ||
| // Checks and uses Epoll if it is available. ref: http://netty.io/wiki/native-transports.html | ||
| b.channel(Epoll.isAvailable()? EpollSocketChannel.class : NioSocketChannel.class) |
| public void shouldUseEpollIfAvailable() throws Exception { | ||
| final WebSocketClient client = new WebSocketClient(); | ||
| if (Epoll.isAvailable()) { | ||
| assertTrue(client.group instanceof EpollEventLoopGroup); |
There was a problem hiding this comment.
Nit: assertTrue(client.group instanceof (Epoll.isAvailable() ? EpollEventLoopGroup : NioEventLoopGroup));
There was a problem hiding this comment.
Using ternary operator gives an Expression expected error on the two values, which I guess it's because the ternary operator is looking for a variable to assign the values to based on the condition, but we are using this as a statement. So I will leave this with the if/else.
| assertTrue(client.group instanceof NioEventLoopGroup); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Nit - should add new line, even though they didn't have it
https://issues.apache.org/jira/browse/TINKERPOP-2498