You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2024/07/24 14:55:46 UTC
(tomcat) branch 10.1.x updated (bcc79ecda6 -> fa8c99b70d)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from bcc79ecda6 Restore only creating Stream input buffer when required
new 4aa508f0f1 Create an HttpParser per Protocol rather than per Processor
new fa8c99b70d Add discardRequestsAndResponses to HTTP/2 with a default of false
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/catalina/connector/CoyoteAdapter.java | 19 +++----
.../coyote/http11/AbstractHttp11Protocol.java | 9 +++
java/org/apache/coyote/http11/Http11Processor.java | 9 +--
java/org/apache/coyote/http2/Http2Protocol.java | 64 ++++++++++++++++++++++
java/org/apache/coyote/http2/Stream.java | 19 ++++++-
java/org/apache/coyote/http2/StreamProcessor.java | 3 +-
webapps/docs/changelog.xml | 11 ++++
webapps/docs/config/http2.xml | 8 +++
8 files changed, 120 insertions(+), 22 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 01/02: Create an HttpParser per Protocol rather than per Processor
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4aa508f0f1b9eea08bf8eb812eb7de35db91e986
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jul 23 15:10:52 2024 +0100
Create an HttpParser per Protocol rather than per Processor
Parser configuration depends on Protocol so this reduces the number of
HttpParser instances created. Particularly helpful for HTTP/2 which does
not recycle Processors.
---
java/org/apache/coyote/http11/AbstractHttp11Protocol.java | 9 +++++++++
java/org/apache/coyote/http11/Http11Processor.java | 9 ++-------
java/org/apache/coyote/http2/StreamProcessor.java | 3 +--
webapps/docs/changelog.xml | 5 +++++
4 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
index e6fa58663d..561e6d322d 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -45,6 +45,7 @@ import org.apache.coyote.http11.upgrade.UpgradeGroupInfo;
import org.apache.coyote.http11.upgrade.UpgradeProcessorExternal;
import org.apache.coyote.http11.upgrade.UpgradeProcessorInternal;
import org.apache.tomcat.util.buf.StringUtils;
+import org.apache.tomcat.util.http.parser.HttpParser;
import org.apache.tomcat.util.modeler.Registry;
import org.apache.tomcat.util.modeler.Util;
import org.apache.tomcat.util.net.AbstractEndpoint;
@@ -58,6 +59,7 @@ public abstract class AbstractHttp11Protocol<S> extends AbstractProtocol<S> {
private final CompressionConfig compressionConfig = new CompressionConfig();
+ private HttpParser httpParser = null;
public AbstractHttp11Protocol(AbstractEndpoint<S,?> endpoint) {
super(endpoint);
@@ -82,6 +84,8 @@ public abstract class AbstractHttp11Protocol<S> extends AbstractProtocol<S> {
for (UpgradeProtocol upgradeProtocol : upgradeProtocols) {
upgradeProtocol.setHttp11Protocol(this);
}
+
+ httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
}
@@ -120,6 +124,11 @@ public abstract class AbstractHttp11Protocol<S> extends AbstractProtocol<S> {
}
+ public HttpParser getHttpParser() {
+ return httpParser;
+ }
+
+
// ------------------------------------------------ HTTP specific properties
// ------------------------------------------ managed in the ProtocolHandler
diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java
index fd7d94664c..50fc2b5dff 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -92,9 +92,6 @@ public class Http11Processor extends AbstractProcessor {
private final Http11OutputBuffer outputBuffer;
- private final HttpParser httpParser;
-
-
/**
* Tracks how many internal filters are in the filter library so they are skipped when looking for pluggable
* filters.
@@ -154,10 +151,8 @@ public class Http11Processor extends AbstractProcessor {
super(adapter);
this.protocol = protocol;
- httpParser = new HttpParser(protocol.getRelaxedPathChars(), protocol.getRelaxedQueryChars());
-
inputBuffer = new Http11InputBuffer(request, protocol.getMaxHttpRequestHeaderSize(),
- protocol.getRejectIllegalHeader(), httpParser);
+ protocol.getRejectIllegalHeader(), protocol.getHttpParser());
request.setInputBuffer(inputBuffer);
outputBuffer = new Http11OutputBuffer(response, protocol.getMaxHttpResponseHeaderSize());
@@ -769,7 +764,7 @@ public class Http11Processor extends AbstractProcessor {
// Validate the characters in the URI. %nn decoding will be checked at
// the point of decoding.
for (int i = uriBC.getStart(); i < uriBC.getEnd(); i++) {
- if (!httpParser.isAbsolutePathRelaxed(uriB[i])) {
+ if (!protocol.getHttpParser().isAbsolutePathRelaxed(uriB[i])) {
badRequest("http11processor.request.invalidUri");
break;
}
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java
index b7d6e9e6ce..78a676e271 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -483,8 +483,7 @@ class StreamProcessor extends AbstractProcessor {
* The checks performed below are based on the checks in Http11InputBuffer.
*/
private boolean validateRequest() {
- HttpParser httpParser = new HttpParser(handler.getProtocol().getHttp11Protocol().getRelaxedPathChars(),
- handler.getProtocol().getHttp11Protocol().getRelaxedQueryChars());
+ HttpParser httpParser = handler.getProtocol().getHttp11Protocol().getHttpParser();
// Method name must be a token
String method = request.method().toString();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3a6fa24740..fbb289e803 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -129,6 +129,11 @@
Ensure that HTTP/2 stream input buffers are only created when there is a
request body to be read. (markt)
</fix>
+ <scode>
+ Refactor creation of HttpParser instances from the Processor level to
+ the Protocol level since the parser configuration depends on the
+ protocol and the parser is, otherwise, stateless. (markt)
+ </scode>
</changelog>
</subsection>
<subsection name="jdbc-pool">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 02/02: Add discardRequestsAndResponses to HTTP/2 with a default of false
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fa8c99b70da876a50f9b5aec4320a093f9d012db
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 e0ed1b9b07..38468b8321 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -309,21 +309,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 8287af0e69..dc9e38eeb3 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -34,6 +34,7 @@ import org.apache.coyote.http11.upgrade.InternalHttpUpgradeHandler;
import org.apache.coyote.http11.upgrade.UpgradeProcessorInternal;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
+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;
@@ -99,6 +100,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) {
@@ -390,4 +404,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 e24297985d..e72ce95064 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -88,10 +88,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
@@ -118,13 +118,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());
@@ -797,6 +806,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 fbb289e803..52de3c6ad6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -134,6 +134,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 f78aa03538..97b4a4b73d 100644
--- a/webapps/docs/config/http2.xml
+++ b/webapps/docs/config/http2.xml
@@ -80,6 +80,14 @@
<attributes>
+ <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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org