This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new 5bdd7d4 Simplify blocking read and write for NIO 5bdd7d4 is described below commit 5bdd7d4712fac4e1af47421c3600b18fabc22ed6 Author: remm <r...@apache.org> AuthorDate: Mon Dec 9 15:15:00 2019 +0100 Simplify blocking read and write for NIO This does not remove or cleanup any of the code that is now unused (NioSelectorPool, NioBlockingSlector, channel flush method, fields, etc), it will be done after actual review. I do not see any negative performance impact. Note: for performance testing, use HTTP/1.1 (avoiding sendfile). --- java/org/apache/tomcat/util/net/NioEndpoint.java | 106 ++++++++++++++++------- webapps/docs/changelog.xml | 7 ++ 2 files changed, 84 insertions(+), 29 deletions(-) diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index 6a0bfdc..7d4104a 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -40,6 +40,7 @@ import java.util.Iterator; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import javax.net.ssl.SSLEngine; @@ -773,6 +774,12 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> if (!socketWrapper.readOperation.process()) { closeSocket = true; } + } else if (socketWrapper.blockReadDone != null) { + if (socketWrapper.blockReadDone.compareAndSet(false, true)) { + synchronized (socketWrapper.blockReadDone) { + socketWrapper.blockReadDone.notify(); + } + } } else if (!processSocket(socketWrapper, SocketEvent.OPEN_READ, true)) { closeSocket = true; } @@ -782,6 +789,12 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> if (!socketWrapper.writeOperation.process()) { closeSocket = true; } + } else if (socketWrapper.blockWriteDone != null) { + if (socketWrapper.blockWriteDone.compareAndSet(false, true)) { + synchronized (socketWrapper.blockWriteDone) { + socketWrapper.blockWriteDone.notify(); + } + } } else if (!processSocket(socketWrapper, SocketEvent.OPEN_WRITE, true)) { closeSocket = true; } @@ -1025,6 +1038,9 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> private volatile long lastRead = System.currentTimeMillis(); private volatile long lastWrite = lastRead; + private AtomicBoolean blockReadDone = null; + private AtomicBoolean blockWriteDone = null; + public NioSocketWrapper(NioChannel channel, NioEndpoint endpoint) { super(channel, endpoint); pool = endpoint.getSelectorPool(); @@ -1215,24 +1231,37 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> if (socket instanceof ClosedNioChannel) { throw new ClosedChannelException(); } - if (block) { - Selector selector = null; - try { - selector = pool.get(); - } catch (IOException x) { - // Ignore - } + nRead = socket.read(to); + if (nRead == -1) { + throw new EOFException(); + } + if (block && nRead == 0) { + long timeout = getReadTimeout(); try { - nRead = pool.read(to, socket, selector, getReadTimeout()); - } finally { - if (selector != null) { - pool.put(selector); + blockReadDone = new AtomicBoolean(false); + registerReadInterest(); + synchronized (blockReadDone) { + if (!blockReadDone.get()) { + try { + if (timeout > 0) { + blockReadDone.wait(timeout); + } else { + blockReadDone.wait(); + } + } catch (InterruptedException e) { + // Continue ... + } + if (!blockReadDone.get()) { + throw new SocketTimeoutException(); + } + } } - } - } else { - nRead = socket.read(to); - if (nRead == -1) { - throw new EOFException(); + nRead = socket.read(to); + if (nRead == -1) { + throw new EOFException(); + } + } finally { + blockReadDone = null; } } return nRead; @@ -1246,22 +1275,41 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> throw new ClosedChannelException(); } if (block) { - long writeTimeout = getWriteTimeout(); - Selector selector = null; + long timeout = getWriteTimeout(); try { - selector = pool.get(); - } catch (IOException x) { - // Ignore - } - try { - pool.write(from, socket, selector, writeTimeout); - // Make sure we are flushed + int n = 0; do { - } while (!socket.flush(true, selector, writeTimeout)); + n = socket.write(from); + if (n == -1) { + throw new EOFException(); + } + if (n == 0) { + if (blockWriteDone == null) { + blockWriteDone = new AtomicBoolean(false); + } else { + blockWriteDone.set(false); + } + registerWriteInterest(); + synchronized (blockWriteDone) { + if (!blockWriteDone.get()) { + try { + if (timeout > 0) { + blockWriteDone.wait(timeout); + } else { + blockWriteDone.wait(); + } + } catch (InterruptedException e) { + // Continue ... + } + if (!blockWriteDone.get()) { + throw new SocketTimeoutException(); + } + } + } + } + } while (from.hasRemaining()); } finally { - if (selector != null) { - pool.put(selector); - } + blockWriteDone = null; } // If there is data left in the buffer the socket will be registered for // write further up the stack. This is to ensure the socket is only diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index f9eaaeb..5e48ebf 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -45,6 +45,13 @@ issues do not "pop up" wrt. others). --> <section name="Tomcat 9.0.31 (markt)" rtext="in development"> + <subsection name="Coyote"> + <changelog> + <update> + Simplify NIO blocking read and write. (remm) + </update> + </changelog> + </subsection> </section> <section name="Tomcat 9.0.30 (markt)" rtext="release in progress"> <subsection name="Catalina"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org