You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2019/12/09 14:15:19 UTC

[tomcat] branch master updated: Simplify blocking read and write for NIO

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 <re...@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org