This is an automated email from the ASF dual-hosted git repository.

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

commit 3c5b14b713c67c81bfd95aa04c2e3f3ddd6a669f
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Jul 24 08:01:26 2024 +0100

    Add discardRequestsAndResponses to HTTP/2 with a default of false
    
    This aligns HTTP/2 with HTTP/1.1 and recycles and re-uses the coyote
    request and response objects rather than recreating them for every
    request.
---
 .../apache/catalina/connector/CoyoteAdapter.java   | 19 +++----
 java/org/apache/coyote/http2/Http2Protocol.java    | 64 ++++++++++++++++++++++
 java/org/apache/coyote/http2/Stream.java           | 19 ++++++-
 webapps/docs/changelog.xml                         |  6 ++
 webapps/docs/config/http2.xml                      |  8 +++
 5 files changed, 103 insertions(+), 13 deletions(-)

diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java 
b/java/org/apache/catalina/connector/CoyoteAdapter.java
index 2ca2280c1f..7231d93fe2 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -311,21 +311,20 @@ public class CoyoteAdapter implements Adapter {
             // Create objects
             request = connector.createRequest();
             request.setCoyoteRequest(req);
+            req.setNote(ADAPTER_NOTES, request);
+            
req.getParameters().setQueryStringCharset(connector.getURICharset());
+        }
+
+        if (response == null) {
             response = connector.createResponse();
             response.setCoyoteResponse(res);
-
-            // Link objects
-            request.setResponse(response);
-            response.setRequest(request);
-
-            // Set as notes
-            req.setNote(ADAPTER_NOTES, request);
             res.setNote(ADAPTER_NOTES, response);
-
-            // Set query string encoding
-            
req.getParameters().setQueryStringCharset(connector.getURICharset());
         }
 
+        // Link objects
+        request.setResponse(response);
+        response.setRequest(request);
+
         if (connector.getXpoweredBy()) {
             response.addHeader("X-Powered-By", POWERED_BY);
         }
diff --git a/java/org/apache/coyote/http2/Http2Protocol.java 
b/java/org/apache/coyote/http2/Http2Protocol.java
index e9095489a9..21c3a52795 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -44,6 +44,7 @@ import 
org.apache.coyote.http11.upgrade.UpgradeProcessorInternal;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.buf.StringUtils;
+import org.apache.tomcat.util.collections.SynchronizedStack;
 import org.apache.tomcat.util.modeler.Registry;
 import org.apache.tomcat.util.net.SocketWrapperBase;
 import org.apache.tomcat.util.res.StringManager;
@@ -114,6 +115,19 @@ public class Http2Protocol implements UpgradeProtocol {
 
     private RequestGroupInfo global = new RequestGroupInfo();
 
+    /*
+     * Setting discardRequestsAndResponses can have a significant performance 
impact. The magnitude of the impact is
+     * very application dependent but with a simple Spring Boot application[1] 
returning a short JSON response running
+     * on markt's desktop in 2024 the difference was 108k req/s with this set 
to true compared to 124k req/s with this
+     * set to false. The larger the response and/or the larger the request 
processing time, the smaller the performance
+     * impact of this setting.
+     *
+     * [1] https://github.com/markt-asf/spring-boot-http2
+     */
+    private boolean discardRequestsAndResponses = false;
+    private final SynchronizedStack<Request> recycledRequests = new 
SynchronizedStack<>();
+    private final SynchronizedStack<Response> recycledResponses = new 
SynchronizedStack<>();
+
     @Override
     public String getHttpUpgradeName(boolean isSSLEnabled) {
         if (isSSLEnabled) {
@@ -503,4 +517,54 @@ public class Http2Protocol implements UpgradeProtocol {
     public RequestGroupInfo getGlobal() {
         return global;
     }
+
+
+    public boolean getDiscardRequestsAndResponses() {
+        return discardRequestsAndResponses;
+    }
+
+
+    public void setDiscardRequestsAndResponses(boolean 
discardRequestsAndResponses) {
+        this.discardRequestsAndResponses = discardRequestsAndResponses;
+    }
+
+
+    Request popRequest() {
+        Request request = null;
+        if (!discardRequestsAndResponses) {
+            request = recycledRequests.pop();
+        }
+        if (request == null) {
+            request = new Request();
+        }
+        return request;
+    }
+
+
+    void pushRequest(Request request) {
+        request.recycle();
+        if (!discardRequestsAndResponses) {
+            recycledRequests.push(request);
+        }
+    }
+
+
+    Response popResponse() {
+        Response response = null;
+        if (!discardRequestsAndResponses) {
+            response = recycledResponses.pop();
+        }
+        if (response == null) {
+            response = new Response();
+        }
+        return response;
+    }
+
+
+    void pushResponse(Response response) {
+        response.recycle();
+        if (!discardRequestsAndResponses) {
+            recycledResponses.push(response);
+        }
+    }
 }
diff --git a/java/org/apache/coyote/http2/Stream.java 
b/java/org/apache/coyote/http2/Stream.java
index 5cc812f3f2..19fac98209 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -89,10 +89,10 @@ class Stream extends AbstractNonZeroStream implements 
HeaderEmitter {
     private final Http2UpgradeHandler handler;
     private final WindowAllocationManager allocationManager = new 
WindowAllocationManager(this);
     private final Request coyoteRequest;
-    private final Response coyoteResponse = new Response();
+    private final Response coyoteResponse;
     private final StreamInputBuffer inputBuffer;
     private final StreamOutputBuffer streamOutputBuffer = new 
StreamOutputBuffer();
-    private final Http2OutputBuffer http2OutputBuffer = new 
Http2OutputBuffer(coyoteResponse, streamOutputBuffer);
+    private final Http2OutputBuffer http2OutputBuffer;
     private final AtomicBoolean removedFromActiveCount = new 
AtomicBoolean(false);
 
     // State machine would be too much overhead
@@ -119,13 +119,22 @@ class Stream extends AbstractNonZeroStream implements 
HeaderEmitter {
         super(handler.getConnectionId(), identifier);
         this.handler = handler;
         setWindowSize(handler.getRemoteSettings().getInitialWindowSize());
+        Response coyoteResponse = handler.getProtocol().popResponse();
+        this.coyoteResponse = coyoteResponse;
+        http2OutputBuffer = new Http2OutputBuffer(this.coyoteResponse, 
streamOutputBuffer);
+
         if (coyoteRequest == null) {
             // HTTP/2 new request
-            this.coyoteRequest = new Request();
+            coyoteRequest = handler.getProtocol().popRequest();
+            this.coyoteRequest = coyoteRequest;
             this.inputBuffer = new StandardStreamInputBuffer();
             this.coyoteRequest.setInputBuffer(inputBuffer);
         } else {
             // HTTP/2 Push or HTTP/1.1 upgrade
+            /*
+             * Implementation note. The request passed in is always newly 
created so it is safe to recycle it for re-use
+             * in the Stream.recyle() method
+             */
             this.coyoteRequest = coyoteRequest;
             this.inputBuffer =
                     new 
SavedRequestStreamInputBuffer((SavedRequestInputFilter) 
coyoteRequest.getInputBuffer());
@@ -795,6 +804,10 @@ class Stream extends AbstractNonZeroStream implements 
HeaderEmitter {
             remaining = inputByteBuffer.remaining();
         }
         handler.replaceStream(this, new RecycledStream(getConnectionId(), 
getIdentifier(), state, remaining));
+        coyoteRequest.recycle();
+        handler.getProtocol().pushRequest(coyoteRequest);
+        coyoteResponse.recycle();
+        handler.getProtocol().pushResponse(coyoteResponse);
     }
 
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0a983a0b1a..adf3e2ff82 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -141,6 +141,12 @@
         the Protocol level since the parser configuration depends on the
         protocol and the parser is, otherwise, stateless. (markt)
       </scode>
+      <add>
+        Align HTTP/2 with HTTP/1.1 and recycle the container internal request
+        and response processing objects by default. This behaviour can be
+        controlled via the new <code>discardRequestsAndResponses</code>
+        attribute on the HTTP/2 upgrade protocol. (markt)
+      </add>
     </changelog>
   </subsection>
   <subsection name="jdbc-pool">
diff --git a/webapps/docs/config/http2.xml b/webapps/docs/config/http2.xml
index 45b1b6a750..3a261cc328 100644
--- a/webapps/docs/config/http2.xml
+++ b/webapps/docs/config/http2.xml
@@ -126,6 +126,14 @@
       compressed. If not specified, this attribute is defaults to "2048".</p>
     </attribute>
 
+    <attribute name="discardRequestsAndResponses" required="false">
+      <p>A boolean value which can be used to enable or disable the recycling
+      of the container internal request and response processing objects. If set
+      to <code>true</code> the request and response objects will be set for
+      garbage collection after every request, otherwise they will be reused.
+      If not specified, this attribute is set to <code>false</code>.</p>
+    </attribute>
+
     <attribute name="initialWindowSize" required="false">
       <p>Controls the initial size of the flow control window for streams that
       Tomcat advertises to clients. If not specified, the default value of


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

Reply via email to