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
Remove edge-triggered support for epoll and just always use level-tri… #14031
Conversation
…ggered Motivation: We supported edge-triggered and level-triggered modes for our epoll transport. This made things more complex while not really providing much value. Modifications: - Remove edge-triggered support and just use level-triggered all the time Result: Less complexity. Fixes #14007
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.
That is a nice cleanup but I wonder why was ET ever added if there is no benefit? In my minds eye there are less system calls under some scenarios but it's also possible that we just never hit those in practice.
transport-classes-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollChannel.java
Outdated
Show resolved
Hide resolved
if (shouldStopReading(config)) { | ||
clearEpollIn(); | ||
} |
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.
fwiw, this whole if block is quite repetitive so maybe it would be a good candidate for a new definition of epollInFinally(Config)
.
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.
I was thinking about it but I found it more readable this way :)
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.
Fair enough reason. It's not a lot of code in any case.
I added it because in theory it should be less expensive. That said it not really gains us much because of how netty works. We still use ET for eventfd and timerfd tho. |
…ggered
Motivation:
We supported edge-triggered and level-triggered modes for our epoll transport. This made things more complex while not really providing much value.
Modifications:
Result:
Less complexity. Fixes #14007