Author: markt
Date: Tue May 20 14:46:48 2014
New Revision: 1596275

URL: http://svn.apache.org/r1596275
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56521
Implement a TODO
Reuse the async write buffer between writes to reduce allocation and GC 
overhead. Based on a patch by leonzhx.
Also:
 - don't re-allocate the buffer / move the data in the buffer if the buffer is 
partially written on a subsequent write
 - make the buffer size configurable, defaulting to 8k

Modified:
    tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Protocol.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java
    
tomcat/trunk/java/org/apache/coyote/http11/upgrade/AbstractServletOutputStream.java
    tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprProcessor.java
    
tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java
    tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioProcessor.java
    
tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioServletOutputStream.java
    tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2Processor.java
    
tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletInputStream.java
    
tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java
    tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioProcessor.java
    
tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioServletOutputStream.java
    tomcat/trunk/webapps/docs/changelog.xml
    tomcat/trunk/webapps/docs/config/http.xml

Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java Tue 
May 20 14:46:48 2014
@@ -166,6 +166,17 @@ public abstract class AbstractHttp11Prot
     }
 
 
+    /**
+     * The size of the buffer used by the ServletOutputStream when performing
+     * delayed asynchronous writes using HTTP upgraded connections.
+     */
+    private int upgradeAsyncWriteBufferSize = 8192;
+    public int getUpgradeAsyncWriteBufferSize() { return 
upgradeAsyncWriteBufferSize; }
+    public void setUpgradeAsyncWriteBufferSize(int 
upgradeAsyncWriteBufferSize) {
+        this.upgradeAsyncWriteBufferSize = upgradeAsyncWriteBufferSize;
+    }
+
+
     // ------------------------------------------------ HTTP specific 
properties
     // ------------------------------------------ passed through to the 
EndPoint
 

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java Tue May 
20 14:46:48 2014
@@ -345,7 +345,8 @@ public class Http11AprProtocol extends A
                 HttpUpgradeHandler httpUpgradeProcessor)
                 throws IOException {
             return new AprProcessor(socket, httpUpgradeProcessor,
-                    (AprEndpoint) proto.endpoint);
+                    (AprEndpoint) proto.endpoint,
+                    proto.getUpgradeAsyncWriteBufferSize());
         }
     }
 }

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Protocol.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Protocol.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Protocol.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Protocol.java Tue May 
20 14:46:48 2014
@@ -265,7 +265,8 @@ public class Http11Nio2Protocol extends 
                 SocketWrapper<Nio2Channel> socket,
                 HttpUpgradeHandler httpUpgradeProcessor)
                 throws IOException {
-            return new Nio2Processor(proto.endpoint, socket, 
httpUpgradeProcessor);
+            return new Nio2Processor(proto.endpoint, socket, 
httpUpgradeProcessor,
+                    proto.getUpgradeAsyncWriteBufferSize());
         }
 
         @Override

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java Tue May 
20 14:46:48 2014
@@ -305,7 +305,8 @@ public class Http11NioProtocol extends A
                 HttpUpgradeHandler httpUpgradeProcessor)
                 throws IOException {
             return new NioProcessor(socket, httpUpgradeProcessor,
-                    ((Http11NioProtocol) 
getProtocol()).getEndpoint().getSelectorPool());
+                    proto.getEndpoint().getSelectorPool(),
+                    proto.getUpgradeAsyncWriteBufferSize());
         }
 
         @Override

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java Tue May 20 
14:46:48 2014
@@ -212,7 +212,8 @@ public class Http11Protocol extends Abst
                 SocketWrapper<Socket> socket,
                 HttpUpgradeHandler httpUpgradeProcessor)
                 throws IOException {
-            return new BioProcessor(socket, httpUpgradeProcessor);
+            return new BioProcessor(socket, httpUpgradeProcessor,
+                    proto.getUpgradeAsyncWriteBufferSize());
         }
 
         @Override

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/AbstractServletOutputStream.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/AbstractServletOutputStream.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/AbstractServletOutputStream.java
 (original)
+++ 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/AbstractServletOutputStream.java
 Tue May 20 14:46:48 2014
@@ -57,10 +57,16 @@ public abstract class AbstractServletOut
 
     // Writes guarded by writeLock
     private volatile byte[] buffer;
+    private volatile int bufferPos;
+    private volatile int bufferLimit;
+    private final int asyncWriteBufferSize;
 
 
-    public AbstractServletOutputStream(SocketWrapper<S> socketWrapper) {
+    public AbstractServletOutputStream(SocketWrapper<S> socketWrapper,
+            int asyncWriteBufferSize) {
         this.socketWrapper = socketWrapper;
+        this.asyncWriteBufferSize = asyncWriteBufferSize;
+        buffer = new byte[asyncWriteBufferSize];
     }
 
 
@@ -74,7 +80,7 @@ public abstract class AbstractServletOut
         // Make sure isReady() and onWritePossible() have a consistent view of
         // buffer and fireListener when determining if the listener should fire
         synchronized (fireListenerLock) {
-            boolean result = (buffer == null);
+            boolean result = (bufferLimit == 0);
             fireListener = !result;
             return result;
         }
@@ -134,9 +140,8 @@ public abstract class AbstractServletOut
 
 
     private void preWriteChecks() {
-        if (buffer != null) {
-            throw new IllegalStateException(
-                    sm.getString("upgrade.sis.write.ise"));
+        if (bufferLimit != 0) {
+            throw new 
IllegalStateException(sm.getString("upgrade.sis.write.ise"));
         }
     }
 
@@ -157,12 +162,25 @@ public abstract class AbstractServletOut
             // write executes.
             int written = doWrite(false, b, off, len);
             if (written < len) {
-                // TODO: - Reuse the buffer
-                //       - Only reallocate if it gets too big (>8k?)
-                buffer = new byte[len - written];
-                System.arraycopy(b, off + written, buffer, 0, len - written);
+                if (b == buffer) {
+                    // This is a partial write of the existing buffer. Just
+                    // increment the current position
+                    bufferPos += written;
+                } else {
+                    // This is a new partial write
+                    int bytesLeft = len - written;
+                    if (bytesLeft > buffer.length) {
+                        buffer = new byte[bytesLeft];
+                    } else if (bytesLeft < asyncWriteBufferSize &&
+                            buffer.length > asyncWriteBufferSize) {
+                        buffer = new byte[asyncWriteBufferSize];
+                    }
+                    bufferPos = 0;
+                    bufferLimit = bytesLeft;
+                    System.arraycopy(b, off + written, buffer, bufferPos, 
bufferLimit);
+                }
             } else {
-                buffer = null;
+                bufferLimit = 0;
             }
         }
     }
@@ -171,8 +189,8 @@ public abstract class AbstractServletOut
     protected final void onWritePossible() throws IOException {
         try {
             synchronized (writeLock) {
-                if (buffer != null) {
-                    writeInternal(buffer, 0, buffer.length);
+                if (bufferLimit > 0) {
+                    writeInternal(buffer, bufferPos, bufferLimit - bufferPos);
                 }
             }
         } catch (Throwable t) {
@@ -190,7 +208,7 @@ public abstract class AbstractServletOut
         // should fire
         boolean fire = false;
         synchronized (fireListenerLock) {
-            if (buffer == null && fireListener) {
+            if (bufferLimit == 0 && fireListener) {
                 fireListener = false;
                 fire = true;
             }

Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprProcessor.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprProcessor.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprProcessor.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprProcessor.java Tue 
May 20 14:46:48 2014
@@ -33,10 +33,11 @@ public class AprProcessor extends Abstra
     private static final int INFINITE_TIMEOUT = -1;
 
     public AprProcessor(SocketWrapper<Long> wrapper,
-            HttpUpgradeHandler httpUpgradeProcessor, AprEndpoint endpoint) {
+            HttpUpgradeHandler httpUpgradeProcessor, AprEndpoint endpoint,
+            int asyncWriteBufferSize) {
         super(httpUpgradeProcessor,
                 new AprServletInputStream(wrapper),
-                new AprServletOutputStream(wrapper, endpoint));
+                new AprServletOutputStream(wrapper, asyncWriteBufferSize, 
endpoint));
 
         Socket.timeoutSet(wrapper.getSocket().longValue(), INFINITE_TIMEOUT);
     }

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java 
(original)
+++ 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/AprServletOutputStream.java 
Tue May 20 14:46:48 2014
@@ -38,8 +38,8 @@ public class AprServletOutputStream exte
     private final ByteBuffer sslOutputBuffer;
 
     public AprServletOutputStream(SocketWrapper<Long> socketWrapper,
-            AprEndpoint endpoint) {
-        super(socketWrapper);
+            int asyncWriteBufferSize, AprEndpoint endpoint) {
+        super(socketWrapper, asyncWriteBufferSize);
         this.endpoint = endpoint;
         this.socket = socketWrapper.getSocket().longValue();
         if (endpoint.isSSLEnabled()) {

Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioProcessor.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioProcessor.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioProcessor.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioProcessor.java Tue 
May 20 14:46:48 2014
@@ -34,9 +34,10 @@ public class BioProcessor extends Abstra
     private static final int INFINITE_TIMEOUT = 0;
 
     public BioProcessor(SocketWrapper<Socket> wrapper,
-            HttpUpgradeHandler httpUpgradeProcessor) throws IOException {
+            HttpUpgradeHandler httpUpgradeProcessor,
+            int asyncWriteBufferSize) throws IOException {
         super(httpUpgradeProcessor, new BioServletInputStream(wrapper),
-                new BioServletOutputStream(wrapper));
+                new BioServletOutputStream(wrapper, asyncWriteBufferSize));
 
         wrapper.getSocket().setSoTimeout(INFINITE_TIMEOUT);
     }

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioServletOutputStream.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioServletOutputStream.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioServletOutputStream.java 
(original)
+++ 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/BioServletOutputStream.java 
Tue May 20 14:46:48 2014
@@ -26,9 +26,9 @@ public class BioServletOutputStream exte
 
     private final OutputStream os;
 
-    public BioServletOutputStream(SocketWrapper<Socket> socketWrapper)
-            throws IOException {
-        super(socketWrapper);
+    public BioServletOutputStream(SocketWrapper<Socket> socketWrapper,
+            int asyncWriteBufferSize) throws IOException {
+        super(socketWrapper, asyncWriteBufferSize);
         os = socketWrapper.getSocket().getOutputStream();
     }
 

Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2Processor.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2Processor.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2Processor.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2Processor.java Tue 
May 20 14:46:48 2014
@@ -34,10 +34,11 @@ public class Nio2Processor extends Abstr
 
     public Nio2Processor(AbstractEndpoint<Nio2Channel> endpoint,
             SocketWrapper<Nio2Channel> wrapper,
-            HttpUpgradeHandler httpUpgradeProcessor) {
+            HttpUpgradeHandler httpUpgradeProcessor,
+            int asyncWriteBufferSize) {
         super(httpUpgradeProcessor,
-                new Nio2ServletInputStream(endpoint, wrapper),
-                new Nio2ServletOutputStream(endpoint, wrapper));
+                new Nio2ServletInputStream(wrapper, endpoint),
+                new Nio2ServletOutputStream(wrapper, asyncWriteBufferSize, 
endpoint));
 
         wrapper.setTimeout(INFINITE_TIMEOUT);
     }

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletInputStream.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletInputStream.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletInputStream.java 
(original)
+++ 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletInputStream.java 
Tue May 20 14:46:48 2014
@@ -41,7 +41,7 @@ public class Nio2ServletInputStream exte
     private boolean flipped = false;
     private volatile boolean readPending = false;
 
-    public Nio2ServletInputStream(AbstractEndpoint<Nio2Channel> endpoint0, 
SocketWrapper<Nio2Channel> wrapper) {
+    public Nio2ServletInputStream(SocketWrapper<Nio2Channel> wrapper, 
AbstractEndpoint<Nio2Channel> endpoint0) {
         this.endpoint = endpoint0;
         this.wrapper = wrapper;
         this.channel = wrapper.getSocket();

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java 
(original)
+++ 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java 
Tue May 20 14:46:48 2014
@@ -41,8 +41,9 @@ public class Nio2ServletOutputStream ext
     private final CompletionHandler<Integer, ByteBuffer> completionHandler;
     private final Semaphore writePending = new Semaphore(1);
 
-    public Nio2ServletOutputStream(AbstractEndpoint<Nio2Channel> endpoint0, 
SocketWrapper<Nio2Channel> socketWrapper0) {
-        super(socketWrapper0);
+    public Nio2ServletOutputStream(SocketWrapper<Nio2Channel> socketWrapper0,
+            int asyncWriteBufferSize, AbstractEndpoint<Nio2Channel> endpoint0) 
{
+        super(socketWrapper0, asyncWriteBufferSize);
         this.endpoint = endpoint0;
         channel = socketWrapper0.getSocket();
         maxWrite = channel.getBufHandler().getWriteBuffer().capacity();

Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioProcessor.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioProcessor.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioProcessor.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioProcessor.java Tue 
May 20 14:46:48 2014
@@ -33,10 +33,11 @@ public class NioProcessor extends Abstra
     private static final int INFINITE_TIMEOUT = -1;
 
     public NioProcessor(SocketWrapper<NioChannel> wrapper,
-            HttpUpgradeHandler httpUpgradeProcessor, NioSelectorPool pool) {
+            HttpUpgradeHandler httpUpgradeProcessor, NioSelectorPool pool,
+            int asyncWriteBufferSize) {
         super(httpUpgradeProcessor,
                 new NioServletInputStream(wrapper, pool),
-                new NioServletOutputStream(wrapper, pool));
+                new NioServletOutputStream(wrapper, asyncWriteBufferSize, 
pool));
 
         wrapper.setTimeout(INFINITE_TIMEOUT);
     }

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioServletOutputStream.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioServletOutputStream.java?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioServletOutputStream.java 
(original)
+++ 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/NioServletOutputStream.java 
Tue May 20 14:46:48 2014
@@ -32,9 +32,9 @@ public class NioServletOutputStream exte
     private final int maxWrite;
 
 
-    public NioServletOutputStream(
-            SocketWrapper<NioChannel> socketWrapper, NioSelectorPool pool) {
-        super(socketWrapper);
+    public NioServletOutputStream(SocketWrapper<NioChannel> socketWrapper,
+            int asyncWriteBufferSize, NioSelectorPool pool) {
+        super(socketWrapper, asyncWriteBufferSize);
         channel = socketWrapper.getSocket();
         this.pool = pool;
         maxWrite = channel.getBufHandler().getWriteBuffer().capacity();

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue May 20 14:46:48 2014
@@ -85,6 +85,17 @@
       </scode>
     </changelog>
   </subsection>
+  <subsection name="Coyote">
+    <changelog>
+      <fix>
+        <bug>56521</bug>: Re-use the asynchronous write buffer between writes 
to
+        reduce allocation and GC overhead. Based on a patch by leonzhx. Also
+        make the buffer size configurable and remove copying of data within
+        buffer when the buffer is only partially written on a subsequent write.
+        (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Jasper">
     <changelog>
       <update>

Modified: tomcat/trunk/webapps/docs/config/http.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/http.xml?rev=1596275&r1=1596274&r2=1596275&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/http.xml (original)
+++ tomcat/trunk/webapps/docs/config/http.xml Tue May 20 14:46:48 2014
@@ -530,6 +530,14 @@
       </p>
     </attribute>
 
+    <attribute name="upgradeAsyncWriteBufferSize" required="false">
+      <p>The default size of the buffer to allocate to for asynchronous writes
+      that can not be completed in a single operation. Data that can't be
+      written immediately will be stored in this buffer until it can be 
written.
+      If more data needs to be stored than space is available in the buffer 
than
+      the size of the buffer will be increased for the duration of the write. 
If
+      not specified the default value of 8192 will be used.</p>
+    </attribute>
   </attributes>
 
   </subsection>



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

Reply via email to