Skip to content

Commit 5384bbc

Browse files
njhillnormanmaurer
authored andcommitted
Epoll: Don't wake event loop when splicing (netty#9354)
Motivation I noticed this while looking at something else. AbstractEpollStreamChannel::spliceQueue is an MPSC queue but only accessed from the event loop. So it could be just changed to e.g. an ArrayDeque. This PR instead reverts to using is as an MPSC queue to avoid dispatching a task to the EL, as appears was the original intention. Modification Change AbstractEpollStreamChannel::spliceQueue to be volatile and lazily initialized via double-checked locking. Add tasks directly to the queue from the public methods rather than possibly waking the EL just to enqueue. An alternative is just to change PlatformDependent.newMpscQueue() to new ArrayDeque() and be done with it :) Result Less disruptive channel/fd-splicing.
1 parent be26f4e commit 5384bbc

File tree

1 file changed

+16
-22
lines changed

1 file changed

+16
-22
lines changed

transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ public void run() {
7070
((AbstractEpollUnsafe) unsafe()).flush0();
7171
}
7272
};
73-
private Queue<SpliceInTask> spliceQueue;
7473

7574
// Lazy init these if we need to splice(...)
75+
private volatile Queue<SpliceInTask> spliceQueue;
7676
private FileDescriptor pipeIn;
7777
private FileDescriptor pipeOut;
7878

@@ -678,13 +678,14 @@ protected void doClose() throws Exception {
678678
}
679679

680680
private void clearSpliceQueue() {
681-
if (spliceQueue == null) {
681+
Queue<SpliceInTask> sQueue = spliceQueue;
682+
if (sQueue == null) {
682683
return;
683684
}
684685
ClosedChannelException exception = null;
685686

686687
for (;;) {
687-
SpliceInTask task = spliceQueue.poll();
688+
SpliceInTask task = sQueue.poll();
688689
if (task == null) {
689690
break;
690691
}
@@ -755,15 +756,16 @@ void epollInReady() {
755756
ByteBuf byteBuf = null;
756757
boolean close = false;
757758
try {
759+
Queue<SpliceInTask> sQueue = null;
758760
do {
759-
if (spliceQueue != null) {
760-
SpliceInTask spliceTask = spliceQueue.peek();
761+
if (sQueue != null || (sQueue = spliceQueue) != null) {
762+
SpliceInTask spliceTask = sQueue.peek();
761763
if (spliceTask != null) {
762764
if (spliceTask.spliceIn(allocHandle)) {
763765
// We need to check if it is still active as if not we removed all SpliceTasks in
764766
// doClose(...)
765767
if (isActive()) {
766-
spliceQueue.remove();
768+
sQueue.remove();
767769
}
768770
continue;
769771
} else {
@@ -823,24 +825,16 @@ void epollInReady() {
823825
}
824826

825827
private void addToSpliceQueue(final SpliceInTask task) {
826-
EventLoop eventLoop = eventLoop();
827-
if (eventLoop.inEventLoop()) {
828-
addToSpliceQueue0(task);
829-
} else {
830-
eventLoop.execute(new Runnable() {
831-
@Override
832-
public void run() {
833-
addToSpliceQueue0(task);
828+
Queue<SpliceInTask> sQueue = spliceQueue;
829+
if (sQueue == null) {
830+
synchronized (this) {
831+
sQueue = spliceQueue;
832+
if (sQueue == null) {
833+
spliceQueue = sQueue = PlatformDependent.newMpscQueue();
834834
}
835-
});
836-
}
837-
}
838-
839-
private void addToSpliceQueue0(SpliceInTask task) {
840-
if (spliceQueue == null) {
841-
spliceQueue = PlatformDependent.newMpscQueue();
835+
}
842836
}
843-
spliceQueue.add(task);
837+
sQueue.add(task);
844838
}
845839

846840
protected abstract class SpliceInTask {

0 commit comments

Comments
 (0)