tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r...@apache.org
Subject [tomcat] branch 8.5.x updated: Simplify regular endpoint writes by removing write(Non)BlockingDirect
Date Wed, 27 Nov 2019 09:30:30 GMT
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
     new 7d2365d  Simplify regular endpoint writes by removing write(Non)BlockingDirect
7d2365d is described below

commit 7d2365d4e302717140946e019b63f05598b4a080
Author: remm <remm@apache.org>
AuthorDate: Wed Nov 27 10:30:15 2019 +0100

    Simplify regular endpoint writes by removing write(Non)BlockingDirect
    
    Port patch from trunk/9.0.
---
 java/org/apache/tomcat/util/net/AprEndpoint.java   |  51 -----------
 .../apache/tomcat/util/net/SocketWrapperBase.java  | 100 ++++-----------------
 webapps/docs/changelog.xml                         |   5 ++
 3 files changed, 22 insertions(+), 134 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 927dd01..64f0257 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -2330,57 +2330,6 @@ public class AprEndpoint extends AbstractEndpoint<Long> implements
SNICallBack {
 
 
         @Override
-        protected void writeBlockingDirect(ByteBuffer from) throws IOException {
-            if (from.isDirect()) {
-                super.writeBlockingDirect(from);
-            } else {
-                // The socket write buffer capacity is socket.appWriteBufSize
-                ByteBuffer writeBuffer = socketBufferHandler.getWriteBuffer();
-                int limit = writeBuffer.capacity();
-                while (from.remaining() >= limit) {
-                    socketBufferHandler.configureWriteBufferForWrite();
-                    transfer(from, writeBuffer);
-                    doWrite(true);
-                }
-
-                if (from.remaining() > 0) {
-                    socketBufferHandler.configureWriteBufferForWrite();
-                    transfer(from, writeBuffer);
-                }
-            }
-        }
-
-
-        @Override
-        protected void writeNonBlockingDirect(ByteBuffer from) throws IOException {
-            if (from.isDirect()) {
-                super.writeNonBlockingDirect(from);
-            } else {
-                // The socket write buffer capacity is socket.appWriteBufSize
-                ByteBuffer writeBuffer = socketBufferHandler.getWriteBuffer();
-                int limit = writeBuffer.capacity();
-                while (from.remaining() >= limit) {
-                    socketBufferHandler.configureWriteBufferForWrite();
-                    transfer(from, writeBuffer);
-                    int newPosition = writeBuffer.position() + limit;
-                    doWrite(false);
-                    if (writeBuffer.position() != newPosition) {
-                        // Didn't write the whole amount of data in the last
-                        // non-blocking write.
-                        // Exit the loop.
-                        return;
-                    }
-                }
-
-                if (from.remaining() > 0) {
-                    socketBufferHandler.configureWriteBufferForWrite();
-                    transfer(from, writeBuffer);
-                }
-            }
-        }
-
-
-        @Override
         protected void doWrite(boolean block, ByteBuffer from) throws IOException {
             if (closed) {
                 throw new IOException(sm.getString("socket.apr.closed", getSocket()));
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index dd5e741..545d333 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -403,8 +403,6 @@ public abstract class SocketWrapperBase<E> {
          * - To enable a marginally more efficient implemented for blocking
          *   writes which do not require the additional checks related to the
          *   use of the non-blocking write buffer
-         *   TODO: Explore re-factoring options to remove the split into
-         *         separate methods
          */
         if (block) {
             writeBlocking(buf, off, len);
@@ -452,8 +450,6 @@ public abstract class SocketWrapperBase<E> {
          * - To enable a marginally more efficient implemented for blocking
          *   writes which do not require the additional checks related to the
          *   use of the non-blocking write buffer
-         *   TODO: Explore re-factoring options to remove the split into
-         *         separate methods
          */
         if (block) {
             writeBlocking(from);
@@ -481,7 +477,7 @@ public abstract class SocketWrapperBase<E> {
     protected void writeBlocking(byte[] buf, int off, int len) throws IOException {
         socketBufferHandler.configureWriteBufferForWrite();
         int thisTime = transfer(buf, off, len, socketBufferHandler.getWriteBuffer());
-        while (socketBufferHandler.getWriteBuffer().remaining() == 0) {
+        while (!socketBufferHandler.getWriteBuffer().hasRemaining()) {
             len = len - thisTime;
             off = off + thisTime;
             doWrite(true);
@@ -505,46 +501,10 @@ public abstract class SocketWrapperBase<E> {
      * @throws IOException If an IO error occurs during the write
      */
     protected void writeBlocking(ByteBuffer from) throws IOException {
-        if (socketBufferHandler.isWriteBufferEmpty()) {
-            // Socket write buffer is empty. Write the provided buffer directly
-            // to the network.
-            // TODO Shouldn't smaller writes be buffered anyway?
-            writeBlockingDirect(from);
-        } else {
-            // Socket write buffer has some data.
-            socketBufferHandler.configureWriteBufferForWrite();
-            // Put as much data as possible into the write buffer
-            transfer(from, socketBufferHandler.getWriteBuffer());
-            // If the buffer is now full, write it to the network and then write
-            // the remaining data directly to the network.
-            if (!socketBufferHandler.isWriteBufferWritable()) {
-                doWrite(true);
-                writeBlockingDirect(from);
-            }
-        }
-    }
-
-
-    /**
-     * Writes directly to the network, bypassing the socket write buffer.
-     *
-     * @param from The ByteBuffer containing the data to be written
-     *
-     * @throws IOException If an IO error occurs during the write
-     */
-    protected void writeBlockingDirect(ByteBuffer from) throws IOException {
-        // The socket write buffer capacity is socket.appWriteBufSize
-        // TODO This only matters when using TLS. For non-TLS connections it
-        //      should be possible to write the ByteBuffer in a single write
-        int limit = socketBufferHandler.getWriteBuffer().capacity();
-        int fromLimit = from.limit();
-        while (from.remaining() >= limit) {
-            from.limit(from.position() + limit);
-            doWrite(true, from);
-            from.limit(fromLimit);
-        }
-
-        if (from.remaining() > 0) {
+        socketBufferHandler.configureWriteBufferForWrite();
+        transfer(from, socketBufferHandler.getWriteBuffer());
+        while (from.hasRemaining()) {
+            doWrite(true);
             socketBufferHandler.configureWriteBufferForWrite();
             transfer(from, socketBufferHandler.getWriteBuffer());
         }
@@ -614,11 +574,12 @@ public abstract class SocketWrapperBase<E> {
     protected void writeNonBlocking(ByteBuffer from)
             throws IOException {
 
-        if (nonBlockingWriteBuffer.isEmpty() && socketBufferHandler.isWriteBufferWritable())
{
+        if (from.hasRemaining() && nonBlockingWriteBuffer.isEmpty()
+                && socketBufferHandler.isWriteBufferWritable()) {
             writeNonBlockingInternal(from);
         }
 
-        if (from.remaining() > 0) {
+        if (from.hasRemaining()) {
             // Remaining data must be buffered
             nonBlockingWriteBuffer.add(from);
         }
@@ -634,44 +595,17 @@ public abstract class SocketWrapperBase<E> {
      * @throws IOException If an IO error occurs during the write
      */
     protected void writeNonBlockingInternal(ByteBuffer from) throws IOException {
-        if (socketBufferHandler.isWriteBufferEmpty()) {
-            writeNonBlockingDirect(from);
-        } else {
-            socketBufferHandler.configureWriteBufferForWrite();
-            transfer(from, socketBufferHandler.getWriteBuffer());
-            if (!socketBufferHandler.isWriteBufferWritable()) {
-                doWrite(false);
-                if (socketBufferHandler.isWriteBufferWritable()) {
-                    writeNonBlockingDirect(from);
-                }
-            }
-        }
-    }
-
-
-    protected void writeNonBlockingDirect(ByteBuffer from) throws IOException {
-        // The socket write buffer capacity is socket.appWriteBufSize
-        // TODO This only matters when using TLS. For non-TLS connections it
-        //      should be possible to write the ByteBuffer in a single write
-        int limit = socketBufferHandler.getWriteBuffer().capacity();
-        int fromLimit = from.limit();
-        while (from.remaining() >= limit) {
-            int newLimit = from.position() + limit;
-            from.limit(newLimit);
-            doWrite(false, from);
-            from.limit(fromLimit);
-            if (from.position() != newLimit) {
-                // Didn't write the whole amount of data in the last
-                // non-blocking write.
-                // Exit the loop.
-                return;
+        socketBufferHandler.configureWriteBufferForWrite();
+        transfer(from, socketBufferHandler.getWriteBuffer());
+        while (from.hasRemaining() && !socketBufferHandler.isWriteBufferWritable())
{
+            doWrite(false);
+            if (socketBufferHandler.isWriteBufferWritable()) {
+                socketBufferHandler.configureWriteBufferForWrite();
+                transfer(from, socketBufferHandler.getWriteBuffer());
+            } else {
+                break;
             }
         }
-
-        if (from.remaining() > 0) {
-            socketBufferHandler.configureWriteBufferForWrite();
-            transfer(from, socketBufferHandler.getWriteBuffer());
-        }
     }
 
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8452d2e..efc034a 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -72,6 +72,11 @@
       <add>
         Add async API to the NIO and APR connector. (remm)
       </add>
+      <fix>
+        Simplify regular endpoint writes by removing write(Non)BlockingDirect.
+        All regular writes will now be buffered for a more predictable
+        behavior. (remm)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Web applications">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message