[tomcat] branch master updated: Fix a typo in changelog
This is an automated email from the ASF dual-hosted git repository. mgrigorov 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 519f6f8 Fix a typo in changelog 519f6f8 is described below commit 519f6f89550208d020c18622ca7b870edf3a2602 Author: Martin Tzvetanov Grigorov AuthorDate: Fri Sep 25 10:02:15 2020 +0300 Fix a typo in changelog --- webapps/docs/changelog.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 85b7eee..319ae06 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -117,7 +117,7 @@ Improve the error handling for the HTTP/2 connection preface when the -Connector is configured with useAsycIO="true". +Connector is configured with useAsyncIO="true". (markt) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Fix a typo in changelog
This is an automated email from the ASF dual-hosted git repository. mgrigorov pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new d6089d8 Fix a typo in changelog d6089d8 is described below commit d6089d8d0855e49ac48b2e5836078edf85fbdcf0 Author: Martin Tzvetanov Grigorov AuthorDate: Fri Sep 25 10:02:15 2020 +0300 Fix a typo in changelog (cherry picked from commit 519f6f89550208d020c18622ca7b870edf3a2602) --- webapps/docs/changelog.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 06ec832..9fea913 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -116,7 +116,7 @@ Improve the error handling for the HTTP/2 connection preface when the -Connector is configured with useAsycIO="true". +Connector is configured with useAsyncIO="true". (markt) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch master updated: Fix a typo in changelog
This is an automated email from the ASF dual-hosted git repository. mgrigorov 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 519f6f8 Fix a typo in changelog 519f6f8 is described below commit 519f6f89550208d020c18622ca7b870edf3a2602 Author: Martin Tzvetanov Grigorov AuthorDate: Fri Sep 25 10:02:15 2020 +0300 Fix a typo in changelog --- webapps/docs/changelog.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 85b7eee..319ae06 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -117,7 +117,7 @@ Improve the error handling for the HTTP/2 connection preface when the -Connector is configured with useAsycIO="true". +Connector is configured with useAsyncIO="true". (markt) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Small optimizations in HpackEncoder
This is an automated email from the ASF dual-hosted git repository.
mgrigorov 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 685a58e Small optimizations in HpackEncoder
685a58e is described below
commit 685a58eb4155135ffe9d8f28acfb1561beb6d3f3
Author: Martin Tzvetanov Grigorov
AuthorDate: Thu Sep 24 13:42:18 2020 +0300
Small optimizations in HpackEncoder
1) Use switch(String) instead of series of String.equals() calls. The
switch uses String.hashCode() and falls back to .equals() only if there are
cases with the same hash code.
2) Reduce memory allocations: no need to Map.Entry since the key is never
used
(cherry picked from commit d2ed8ffc75c5e3b425888b456ffc51036d94ac39)
---
java/org/apache/coyote/http2/HpackEncoder.java | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/coyote/http2/HpackEncoder.java
b/java/org/apache/coyote/http2/HpackEncoder.java
index a024ca2..7331ca6 100644
--- a/java/org/apache/coyote/http2/HpackEncoder.java
+++ b/java/org/apache/coyote/http2/HpackEncoder.java
@@ -44,7 +44,13 @@ public class HpackEncoder {
public boolean shouldUseIndexing(String headerName, String value) {
//content length and date change all the time
//no need to index them, or they will churn the table
-return !headerName.equals("content-length") &&
!headerName.equals("date");
+switch (headerName) {
+case "content-length":
+case "date":
+return false;
+default:
+return true;
+}
}
@Override
@@ -258,8 +264,8 @@ public class HpackEncoder {
private void preventPositionRollover() {
//if the position counter is about to roll over we iterate all the
table entries
//and set their position to their actual position
-for (Map.Entry> entry :
dynamicTable.entrySet()) {
-for (TableEntry t : entry.getValue()) {
+for (List tableEntries : dynamicTable.values()) {
+for (TableEntry t : tableEntries) {
t.position = t.getPosition();
}
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Fix a typo in changelog
This is an automated email from the ASF dual-hosted git repository. mgrigorov pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new d6089d8 Fix a typo in changelog d6089d8 is described below commit d6089d8d0855e49ac48b2e5836078edf85fbdcf0 Author: Martin Tzvetanov Grigorov AuthorDate: Fri Sep 25 10:02:15 2020 +0300 Fix a typo in changelog (cherry picked from commit 519f6f89550208d020c18622ca7b870edf3a2602) --- webapps/docs/changelog.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 06ec832..9fea913 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -116,7 +116,7 @@ Improve the error handling for the HTTP/2 connection preface when the -Connector is configured with useAsycIO="true". +Connector is configured with useAsyncIO="true". (markt) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
buildbot success in on tomcat-9-trunk
The Buildbot has detected a restored build on builder tomcat-9-trunk while building tomcat. Full details are available at: https://ci.apache.org/builders/tomcat-9-trunk/builds/465 Buildbot URL: https://ci.apache.org/ Buildslave for this Build: asf946_ubuntu Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-9-commit' triggered this build Build Source Stamp: [branch 9.0.x] d6089d8d0855e49ac48b2e5836078edf85fbdcf0 Blamelist: Martin Tzvetanov Grigorov Build succeeded! Sincerely, -The Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch master updated: Fix a typo in changelog
On 25/09/2020 08:06, mgrigo...@apache.org wrote: > This is an automated email from the ASF dual-hosted git repository. > > mgrigorov 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 519f6f8 Fix a typo in changelog > 519f6f8 is described below > > commit 519f6f89550208d020c18622ca7b870edf3a2602 > Author: Martin Tzvetanov Grigorov > AuthorDate: Fri Sep 25 10:02:15 2020 +0300 > > Fix a typo in changelog Thanks for fixing these. Mark > --- > webapps/docs/changelog.xml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml > index 85b7eee..319ae06 100644 > --- a/webapps/docs/changelog.xml > +++ b/webapps/docs/changelog.xml > @@ -117,7 +117,7 @@ > > > Improve the error handling for the HTTP/2 connection preface when the > -Connector is configured with useAsycIO="true". > +Connector is configured with > useAsyncIO="true". > (markt) > > > > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
buildbot success in on tomcat-85-trunk
The Buildbot has detected a restored build on builder tomcat-85-trunk while building tomcat. Full details are available at: https://ci.apache.org/builders/tomcat-85-trunk/builds/2484 Buildbot URL: https://ci.apache.org/ Buildslave for this Build: asf946_ubuntu Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-85-commit' triggered this build Build Source Stamp: [branch 8.5.x] 685a58eb4155135ffe9d8f28acfb1561beb6d3f3 Blamelist: Martin Tzvetanov Grigorov Build succeeded! Sincerely, -The Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on a change in pull request #351: Remove White Spaces from the JSP files
kamnani commented on a change in pull request #351:
URL: https://github.com/apache/tomcat/pull/351#discussion_r494418331
##
File path: java/org/apache/jasper/compiler/Generator.java
##
@@ -2095,6 +2101,20 @@ public void visit(Node.JspElement n) throws
JasperException {
public void visit(Node.TemplateText n) throws JasperException {
String text = n.getText();
+// If the flag is active, attempt to minimize the frequency of
+// regex operations.
+if ((ctxt!=null) &&
+ctxt.getOptions().getJSPWhiteSpaceTrimFlag() &&
+text.contains("\n")) {
+// Ensure there are no or tags embedded in this
+// text - if there are, we want to NOT modify the whitespace.
+Matcher preMatcher = PRE_TAG_PATTERN.matcher(text);
+if (!preMatcher.matches()) {
+Matcher matcher = BLANK_LINE_PATTERN.matcher(text);
+String revisedText = matcher.replaceAll("\n");
Review comment:
yes, it is intentional. We replace it with just one "\n.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on a change in pull request #351: Remove White Spaces from the JSP files
martin-g commented on a change in pull request #351:
URL: https://github.com/apache/tomcat/pull/351#discussion_r494077383
##
File path: java/org/apache/jasper/compiler/NewlineReductionServletWriter.java
##
@@ -0,0 +1,41 @@
+package org.apache.jasper.compiler;
+
+import java.io.PrintWriter;
+
+/**
+ * This class filters duplicate newlines instructions from the compiler output,
+ * and therefore from the runtime JSP. The duplicates typically happen because
+ * the compiler has multiple branches that write them, but they operate
+ * independently and don't realize that the previous output was identical.
+ *
+ * Removing these lines makes the JSP more efficient by executing fewer
operations during runtime.
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ *
+ */
+public class NewlineReductionServletWriter extends ServletWriter {
+private static final String NEWLINE_WRITE_TEXT = "out.write('\\n');";
Review comment:
This one is a bit strange.
I haven't written JSP code in more than 10 years now but if I want to
produce several new lines I'd do `out.write("\n\n\n\n")` instead of 4 times
`out.write('\\n');`.
In what cases one would have several `out.write('\\n');` ?
##
File path: java/org/apache/jasper/compiler/Generator.java
##
@@ -2095,6 +2101,20 @@ public void visit(Node.JspElement n) throws
JasperException {
public void visit(Node.TemplateText n) throws JasperException {
String text = n.getText();
+// If the flag is active, attempt to minimize the frequency of
+// regex operations.
+if ((ctxt!=null) &&
+ctxt.getOptions().getJSPWhiteSpaceTrimFlag() &&
+text.contains("\n")) {
+// Ensure there are no or tags embedded in this
+// text - if there are, we want to NOT modify the whitespace.
+Matcher preMatcher = PRE_TAG_PATTERN.matcher(text);
+if (!preMatcher.matches()) {
+Matcher matcher = BLANK_LINE_PATTERN.matcher(text);
+String revisedText = matcher.replaceAll("\n");
Review comment:
This will drop also any `\r`s in the string. Is this intentional ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on pull request #351: Remove White Spaces from the JSP files
martin-g commented on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-698157237 Since there is no new test for this I am not sure but looking at the code I think "Remove white space" is not quite accurate. It is rather "Remove empty lines". Because BLANK_LINE_PATTERN would match only if there zero or more empty spaces, followed by at least one new line, followed by 0 or more empty spaces. So the example above: ``` | | Quick Servlet Demo | ``` won't produce: ``` | | Quick Servlet Demo | ``` In addition: why preserve 1 new line ? HTML minifiers would produce something like `.` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #351: Remove White Spaces from the JSP files
kamnani edited a comment on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-698430292 > Since there is no new test for this I am not sure but looking at the code I think "Remove white space" is not quite accurate. > It is rather "Remove empty lines". Because BLANK_LINE_PATTERN would match only if there zero or more empty spaces, followed by at least one new line, followed by 0 or more empty spaces. > So the example above: > > ``` > | > | Quick Servlet Demo > | > ``` > > won't produce: > > ``` > | > | Quick Servlet Demo > | > ``` > > In addition: why preserve 1 new line ? HTML minifiers would produce something like `.` I think the node text comes like this `\n` and this will be caught by the BLANK_LINE_PATTERN and thus will later be turned down to `\n` after the processing. I assume you are not taking the node correctly. But on a broader aspect ``` | | Quick Servlet Demo | ``` Will produce ``` | | Quick Servlet Demo | ``` I agree to the minifying part, so we can rather just replace it with an empty character in the end instead of new line character and that will do the job too and will thus be more optimized. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #351: Remove White Spaces from the JSP files
kamnani commented on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-698430292 > Since there is no new test for this I am not sure but looking at the code I think "Remove white space" is not quite accurate. > It is rather "Remove empty lines". Because BLANK_LINE_PATTERN would match only if there zero or more empty spaces, followed by at least one new line, followed by 0 or more empty spaces. > So the example above: > > ``` > | > | Quick Servlet Demo > | > ``` > > won't produce: > > ``` > | > | Quick Servlet Demo > | > ``` > > In addition: why preserve 1 new line ? HTML minifiers would produce something like `.` I think the node text comes like this `\n` and this will be caught by the BLANK_LINE_PATTERN and thus will later be turned down to `\n` after the processing. I assume you are not taking the node correctly. But on a broader aspect ``` | | Quick Servlet Demo | ``` Will produce ``` | | Quick Servlet Demo | ``` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/09: Reduce memory footprint of closed http/2 streams
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit f6e656e4b1076a8b7d77d7eded2290a6fe926fb8
Author: Mark Thomas
AuthorDate: Thu Sep 24 17:32:49 2020 +0100
Reduce memory footprint of closed http/2 streams
This refactoring replaces closed streams with a new RecycledStream
object and changes the mechanism used to look up known streams.
Pull up re-prioritisation
---
.../apache/coyote/http2/AbstractNonZeroStream.java | 71 ++
java/org/apache/coyote/http2/AbstractStream.java | 6 +-
.../apache/coyote/http2/Http2UpgradeHandler.java | 6 +-
java/org/apache/coyote/http2/RecycledStream.java | 10 +--
java/org/apache/coyote/http2/Stream.java | 55 -
5 files changed, 78 insertions(+), 70 deletions(-)
diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
index 63fb5e7..ce08cc5 100644
--- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
+++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
@@ -16,6 +16,11 @@
*/
package org.apache.coyote.http2;
+import java.util.Iterator;
+
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
/**
* Base class for all streams other than stream 0, the connection. Primarily
@@ -23,7 +28,73 @@ package org.apache.coyote.http2;
*/
abstract class AbstractNonZeroStream extends AbstractStream {
+private static final Log log =
LogFactory.getLog(AbstractNonZeroStream.class);
+private static final StringManager sm =
StringManager.getManager(AbstractNonZeroStream.class);
+
+private volatile int weight;
+
+
AbstractNonZeroStream(Integer identifier) {
+this(identifier, Constants.DEFAULT_WEIGHT);
+}
+
+
+AbstractNonZeroStream(Integer identifier, int weight) {
super(identifier);
+this.weight = weight;
+}
+
+
+@Override
+final int getWeight() {
+return weight;
}
+
+
+final void rePrioritise(AbstractStream parent, boolean exclusive, int
weight) {
+if (log.isDebugEnabled()) {
+log.debug(sm.getString("stream.reprioritisation.debug",
+getConnectionId(), getIdAsString(),
Boolean.toString(exclusive),
+parent.getIdAsString(), Integer.toString(weight)));
+}
+
+// Check if new parent is a descendant of this stream
+if (isDescendant(parent)) {
+parent.detachFromParent();
+// Cast is always safe since any descendant of this stream must be
+// an instance of Stream
+getParentStream().addChild((Stream) parent);
+}
+
+if (exclusive) {
+// Need to move children of the new parent to be children of this
+// stream. Slightly convoluted to avoid concurrent modification.
+Iterator parentsChildren =
parent.getChildStreams().iterator();
+while (parentsChildren.hasNext()) {
+AbstractNonZeroStream parentsChild = parentsChildren.next();
+parentsChildren.remove();
+this.addChild(parentsChild);
+}
+}
+detachFromParent();
+parent.addChild(this);
+this.weight = weight;
+}
+
+
+/*
+ * Used when removing closed streams from the tree and we know there is no
+ * need to check for circular references.
+ */
+final void rePrioritise(AbstractStream parent, int weight) {
+if (log.isDebugEnabled()) {
+log.debug(sm.getString("stream.reprioritisation.debug",
+getConnectionId(), getIdAsString(), Boolean.FALSE,
+parent.getIdAsString(), Integer.toString(weight)));
+}
+
+parent.addChild(this);
+this.weight = weight;
+}
+
}
diff --git a/java/org/apache/coyote/http2/AbstractStream.java
b/java/org/apache/coyote/http2/AbstractStream.java
index 21963ee..c7374b6 100644
--- a/java/org/apache/coyote/http2/AbstractStream.java
+++ b/java/org/apache/coyote/http2/AbstractStream.java
@@ -37,7 +37,7 @@ abstract class AbstractStream {
private final String idAsString;
private volatile AbstractStream parentStream = null;
-private final Set childStreams = Collections.newSetFromMap(new
ConcurrentHashMap<>());
+private final Set childStreams =
Collections.newSetFromMap(new ConcurrentHashMap<>());
private long windowSize =
ConnectionSettingsBase.DEFAULT_INITIAL_WINDOW_SIZE;
@@ -70,7 +70,7 @@ abstract class AbstractStream {
}
-final void addChild(Stream child) {
+final void addChild(AbstractNonZeroStream child) {
child.setParentStream(this);
childStreams.add(child);
}
@@ -97,7 +97,7 @@ abstract class AbstractStream {
}
-final Set
[tomcat] 01/09: Reduce memory footprint of closed http/2 streams
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit ee2571013a5204b7774bed80c938921a2f504427
Author: Mark Thomas
AuthorDate: Wed Sep 23 19:59:13 2020 +0100
Reduce memory footprint of closed http/2 streams
This refactoring replaces closed streams with a new RecycledStream
object and changes the mechanism used to look up known streams.
Initial changes to object hierarchy in preparation for further
changes.
---
.../apache/coyote/http2/AbstractNonZeroStream.java | 29 ++
java/org/apache/coyote/http2/AbstractStream.java | 3 +-
java/org/apache/coyote/http2/RecycledStream.java | 45 ++
java/org/apache/coyote/http2/Stream.java | 2 +-
4 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
new file mode 100644
index 000..63fb5e7
--- /dev/null
+++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.coyote.http2;
+
+
+/**
+ * Base class for all streams other than stream 0, the connection. Primarily
+ * provides functionality shared between full Stream and RecycledStream.
+ */
+abstract class AbstractNonZeroStream extends AbstractStream {
+
+AbstractNonZeroStream(Integer identifier) {
+super(identifier);
+}
+}
diff --git a/java/org/apache/coyote/http2/AbstractStream.java
b/java/org/apache/coyote/http2/AbstractStream.java
index 088d5b0..21963ee 100644
--- a/java/org/apache/coyote/http2/AbstractStream.java
+++ b/java/org/apache/coyote/http2/AbstractStream.java
@@ -25,7 +25,8 @@ import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.res.StringManager;
/**
- * Used to managed prioritisation.
+ * Base class for all streams including the connection (referred to as Stream
0)
+ * and is used primarily when managing prioritization.
*/
abstract class AbstractStream {
diff --git a/java/org/apache/coyote/http2/RecycledStream.java
b/java/org/apache/coyote/http2/RecycledStream.java
new file mode 100644
index 000..f500646
--- /dev/null
+++ b/java/org/apache/coyote/http2/RecycledStream.java
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.coyote.http2;
+
+/**
+ * Represents a closed stream in the priority tree. Used in preference to the
+ * full {@link Stream} as has much lower memory usage.
+ */
+class RecycledStream extends AbstractNonZeroStream {
+
+private final String connectionId;
+private int weight;
+
+RecycledStream(Stream stream) {
+super(stream.getIdentifier());
+connectionId = stream.getConnectionId();
+weight = stream.getWeight();
+}
+
+
+@Override
+String getConnectionId() {
+return connectionId;
+}
+
+
+@Override
+int getWeight() {
+return weight;
+}
+}
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 7625624..d236206 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -46,7 +46,7 @@ import org.apache.tomcat.util.net.ApplicationBufferHandler;
import org.apache.tomcat.util.net.WriteBuffer;
import org.apa
[tomcat] 03/09: Reduce memory footprint of closed http/2 streams
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 0f667057e72cbec7badbb1cca520b4b3d5d60b53
Author: Mark Thomas
AuthorDate: Thu Sep 24 17:50:43 2020 +0100
Reduce memory footprint of closed http/2 streams
This refactoring replaces closed streams with a new RecycledStream
object and changes the mechanism used to look up known streams.
Pull-up isClosedFinal()
---
java/org/apache/coyote/http2/AbstractNonZeroStream.java | 1 +
java/org/apache/coyote/http2/RecycledStream.java| 8
java/org/apache/coyote/http2/Stream.java| 1 +
3 files changed, 10 insertions(+)
diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
index ce08cc5..70cfe35 100644
--- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
+++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
@@ -97,4 +97,5 @@ abstract class AbstractNonZeroStream extends AbstractStream {
this.weight = weight;
}
+abstract boolean isClosedFinal();
}
diff --git a/java/org/apache/coyote/http2/RecycledStream.java
b/java/org/apache/coyote/http2/RecycledStream.java
index 1e630df..dbbdc10 100644
--- a/java/org/apache/coyote/http2/RecycledStream.java
+++ b/java/org/apache/coyote/http2/RecycledStream.java
@@ -23,10 +23,12 @@ package org.apache.coyote.http2;
class RecycledStream extends AbstractNonZeroStream {
private final String connectionId;
+private final boolean closedFinal;
RecycledStream(Stream stream) {
super(stream.getIdentifier(), stream.getWeight());
connectionId = stream.getConnectionId();
+closedFinal = stream.isClosedFinal();
}
@@ -34,4 +36,10 @@ class RecycledStream extends AbstractNonZeroStream {
String getConnectionId() {
return connectionId;
}
+
+
+@Override
+boolean isClosedFinal() {
+return closedFinal;
+}
}
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 766e5a2..33f151a 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -646,6 +646,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
+@Override
final boolean isClosedFinal() {
return state.isClosedFinal();
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 05/09: Reduce memory footprint of closed http/2 streams
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 9ee7d6afbfd0c0fe0a144a251dc7d14dfbb7a1b3
Author: Mark Thomas
AuthorDate: Thu Sep 24 21:36:19 2020 +0100
Reduce memory footprint of closed http/2 streams
This refactoring replaces closed streams with a new RecycledStream
object and changes the mechanism used to look up known streams.
Refactor StreamStateMachine to remove reference to Stream
---
java/org/apache/coyote/http2/Stream.java| 2 +-
.../org/apache/coyote/http2/StreamStateMachine.java | 21 -
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 29cea54..2c1b714 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -97,7 +97,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
this.handler = handler;
handler.addChild(this);
setWindowSize(handler.getRemoteSettings().getInitialWindowSize());
-state = new StreamStateMachine(this);
+state = new StreamStateMachine(getConnectionId(), getIdAsString());
if (coyoteRequest == null) {
// HTTP/2 new request
this.coyoteRequest = new Request();
diff --git a/java/org/apache/coyote/http2/StreamStateMachine.java
b/java/org/apache/coyote/http2/StreamStateMachine.java
index 851e1a8..72028fd 100644
--- a/java/org/apache/coyote/http2/StreamStateMachine.java
+++ b/java/org/apache/coyote/http2/StreamStateMachine.java
@@ -39,12 +39,15 @@ class StreamStateMachine {
private static final Log log = LogFactory.getLog(StreamStateMachine.class);
private static final StringManager sm =
StringManager.getManager(StreamStateMachine.class);
-private final Stream stream;
+private final String connectionId;
+private final String streamId;
+
private State state;
-StreamStateMachine(Stream stream) {
-this.stream = stream;
+StreamStateMachine(String connectionId, String streamId) {
+this.connectionId = connectionId;
+this.streamId = streamId;
stateChange(null, State.IDLE);
}
@@ -92,7 +95,7 @@ class StreamStateMachine {
public synchronized void sendReset() {
if (state == State.IDLE) {
throw new
IllegalStateException(sm.getString("streamStateMachine.debug.change",
-stream.getConnectionId(), stream.getIdAsString(), state));
+connectionId, streamId, state));
}
if (state.canReset()) {
stateChange(state, State.CLOSED_RST_TX);
@@ -109,8 +112,8 @@ class StreamStateMachine {
if (state == oldState) {
state = newState;
if (log.isDebugEnabled()) {
-log.debug(sm.getString("streamStateMachine.debug.change",
stream.getConnectionId(),
-stream.getIdAsString(), oldState, newState));
+log.debug(sm.getString("streamStateMachine.debug.change",
connectionId,
+streamId, oldState, newState));
}
}
}
@@ -122,12 +125,12 @@ class StreamStateMachine {
if (!isFrameTypePermitted(frameType)) {
if (state.connectionErrorForInvalidFrame) {
throw new
ConnectionException(sm.getString("streamStateMachine.invalidFrame",
-stream.getConnectionId(), stream.getIdAsString(),
state, frameType),
+connectionId, streamId, state, frameType),
state.errorCodeForInvalidFrame);
} else {
throw new
StreamException(sm.getString("streamStateMachine.invalidFrame",
-stream.getConnectionId(), stream.getIdAsString(),
state, frameType),
-state.errorCodeForInvalidFrame, stream.getIdAsInt());
+connectionId, streamId, state, frameType),
+state.errorCodeForInvalidFrame,
Integer.parseInt(streamId));
}
}
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch master updated (519f6f8 -> 18e0e3f)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 519f6f8 Fix a typo in changelog
new ee25710 Reduce memory footprint of closed http/2 streams
new f6e656e Reduce memory footprint of closed http/2 streams
new 0f66705 Reduce memory footprint of closed http/2 streams
new fa6df26 Reduce memory footprint of closed http/2 streams
new 9ee7d6a Reduce memory footprint of closed http/2 streams
new 30df6a0 Reduce memory footprint of closed http/2 streams
new 0a78ae9 Fully replace Stream with RecycledStream
new 954cbff Refactor the pruning so more stream info is retained for
longer
new 18e0e3f Update changelog
The 9 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/coyote/http2/AbstractNonZeroStream.java | 143 +++
java/org/apache/coyote/http2/AbstractStream.java | 9 +-
.../apache/coyote/http2/Http2UpgradeHandler.java | 264 +
...tionSettingsRemote.java => RecycledStream.java} | 23 +-
java/org/apache/coyote/http2/Stream.java | 90 +--
.../apache/coyote/http2/StreamStateMachine.java| 21 +-
webapps/docs/changelog.xml | 5 +
7 files changed, 348 insertions(+), 207 deletions(-)
create mode 100644 java/org/apache/coyote/http2/AbstractNonZeroStream.java
copy java/org/apache/coyote/http2/{ConnectionSettingsRemote.java =>
RecycledStream.java} (61%)
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 08/09: Refactor the pruning so more stream info is retained for longer
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 954cbffa73efe6e546a07c84ade97a3b9b38a893
Author: Mark Thomas
AuthorDate: Fri Sep 25 14:14:06 2020 +0100
Refactor the pruning so more stream info is retained for longer
The memory impact of this is mitigated by the previous changes to
replace closed Stream instances with RecycledStream instances.
---
.../apache/coyote/http2/Http2UpgradeHandler.java | 89 --
1 file changed, 49 insertions(+), 40 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index a023fe0..49115d3 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -30,6 +30,7 @@ import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
@@ -123,7 +124,7 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
private HpackDecoder hpackDecoder;
private HpackEncoder hpackEncoder;
-private final ConcurrentMap streams = new
ConcurrentHashMap<>();
+private final ConcurrentMap streams = new
ConcurrentSkipListMap<>();
protected final AtomicInteger activeRemoteStreamCount = new
AtomicInteger(0);
// Start at -1 so the 'add 2' logic in closeIdleStreams() works
private volatile int maxActiveRemoteStreamId = -1;
@@ -1207,78 +1208,86 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
// 2. Completed streams used for a request with children
// 3. Closed final streams
//
-// Steps 1 and 2 will always be completed.
-// Step 3 will be completed to the minimum extent necessary to bring
the
-// total number of streams under the limit.
+// The pruning halts as soon as enough streams have been pruned.
// Use these sets to track the different classes of streams
-TreeSet candidatesStepOne = new TreeSet<>();
TreeSet candidatesStepTwo = new TreeSet<>();
TreeSet candidatesStepThree = new TreeSet<>();
-for (Entry entry : streams.entrySet())
{
-AbstractNonZeroStream stream = entry.getValue();
+// Step 1
+// Iterator is in key order so we automatically have the oldest streams
+// first
+for (AbstractNonZeroStream stream : streams.values()) {
// Never remove active streams
if (stream instanceof Stream && ((Stream) stream).isActive()) {
continue;
}
-final Integer streamIdentifier = entry.getKey();
if (stream.isClosedFinal()) {
// This stream went from IDLE to CLOSED and is likely to have
// been created by the client as part of the priority tree.
-candidatesStepThree.add(streamIdentifier);
+// Candidate for steo 3.
+candidatesStepThree.add(stream.getIdentifier());
} else if (stream.getChildStreams().size() == 0) {
-// Closed, no children
-candidatesStepOne.add(streamIdentifier);
-} else {
-// Closed, with children
-candidatesStepTwo.add(streamIdentifier);
-}
-}
-
-// Process the step one list
-for (Integer streamIdToRemove : candidatesStepOne) {
-// Remove this childless stream
-AbstractNonZeroStream removedStream =
streams.remove(streamIdToRemove);
-removedStream.detachFromParent();
-toClose--;
-if (log.isDebugEnabled()) {
-log.debug(sm.getString("upgradeHandler.pruned", connectionId,
streamIdToRemove));
-}
-
-// Did this make the parent childless?
-AbstractStream parent = removedStream.getParentStream();
-while (parent instanceof Stream && !((Stream) parent).isActive() &&
-!((Stream) parent).isClosedFinal() &&
parent.getChildStreams().size() == 0) {
-streams.remove(parent.getIdentifier());
-parent.detachFromParent();
-toClose--;
+// Prune it
+AbstractStream parent = stream.getParentStream();
+streams.remove(stream.getIdentifier());
+stream.detachFromParent();
if (log.isDebugEnabled()) {
-log.debug(sm.getString("upgradeHandler.pruned",
connectionId, streamIdToRemove));
+
[tomcat] 07/09: Fully replace Stream with RecycledStream
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 0a78ae92e75e8eb4f80d672af31d59c9706d0382
Author: Mark Thomas
AuthorDate: Fri Sep 25 12:26:40 2020 +0100
Fully replace Stream with RecycledStream
Profiler (YourKit) suggests retain memory for Stream ~60k whereas
RecycledStream ins ~300 bytes.
---
.../apache/coyote/http2/AbstractNonZeroStream.java | 37 +++--
.../apache/coyote/http2/Http2UpgradeHandler.java | 64 +-
java/org/apache/coyote/http2/RecycledStream.java | 4 +-
java/org/apache/coyote/http2/Stream.java | 2 +-
4 files changed, 74 insertions(+), 33 deletions(-)
diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
index 084de33..bbdfa33 100644
--- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
+++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
@@ -33,19 +33,17 @@ abstract class AbstractNonZeroStream extends AbstractStream
{
protected final StreamStateMachine state;
-private volatile int weight;
+private volatile int weight = Constants.DEFAULT_WEIGHT;
AbstractNonZeroStream(String connectionId, Integer identifier) {
super(identifier);
-this.weight = Constants.DEFAULT_WEIGHT;
this.state = new StreamStateMachine(connectionId, getIdAsString());
}
-AbstractNonZeroStream(Integer identifier, int weight, StreamStateMachine
state) {
+AbstractNonZeroStream(Integer identifier, StreamStateMachine state) {
super(identifier);
-this.weight = weight;
this.state = state;
}
@@ -56,6 +54,13 @@ abstract class AbstractNonZeroStream extends AbstractStream {
}
+/*
+ * General method used when reprioritising a stream and care needs to be
+ * taken not to create circular references.
+ *
+ * Changes to the priority tree need to be sychronized at the connection
+ * level. This is the caller's responsibility.
+ */
final void rePrioritise(AbstractStream parent, boolean exclusive, int
weight) {
if (log.isDebugEnabled()) {
log.debug(sm.getString("stream.reprioritisation.debug",
@@ -90,6 +95,9 @@ abstract class AbstractNonZeroStream extends AbstractStream {
/*
* Used when removing closed streams from the tree and we know there is no
* need to check for circular references.
+ *
+ * Changes to the priority tree need to be sychronized at the connection
+ * level. This is the caller's responsibility.
*/
final void rePrioritise(AbstractStream parent, int weight) {
if (log.isDebugEnabled()) {
@@ -103,6 +111,27 @@ abstract class AbstractNonZeroStream extends
AbstractStream {
}
+/*
+ * Used when "recycling" a stream and replacing a Stream instance with a
+ * RecycledStream instance.
+ *
+ * Replace this stream with the provided stream in the parent/child
+ * hierarchy.
+ *
+ * Changes to the priority tree need to be sychronized at the connection
+ * level. This is the caller's responsibility.
+ */
+void replaceStream(AbstractNonZeroStream replacement) {
+replacement.setParentStream(getParentStream());
+detachFromParent();
+for (AbstractNonZeroStream child : getChildStreams()) {
+replacement.addChild(child);
+}
+getChildStreams().clear();
+replacement.weight = weight;
+}
+
+
final boolean isClosedFinal() {
return state.isClosedFinal();
}
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 77dc339..a023fe0 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -94,6 +94,8 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
private static final HeaderSink HEADER_SINK = new HeaderSink();
+private final Object priorityTreeLock = new Object();
+
protected final String connectionId;
protected final Http2Protocol protocol;
@@ -104,8 +106,7 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
private volatile Http2Parser parser;
// Simple state machine (sequence of states)
-private AtomicReference connectionState =
-new AtomicReference<>(ConnectionState.NEW);
+private AtomicReference connectionState = new
AtomicReference<>(ConnectionState.NEW);
private volatile long pausedNanoTime = Long.MAX_VALUE;
/**
@@ -1175,7 +1176,7 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
newStreamsSinceLastPrune = 0;
// RFC 7540, 5.3.4 endpoints should maintain state for at least the
-// maxi
[tomcat] 06/09: Reduce memory footprint of closed http/2 streams
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 30df6a08eb6ab45b409cc2fc4732d31bdcfbb521
Author: Mark Thomas
AuthorDate: Thu Sep 24 21:47:09 2020 +0100
Reduce memory footprint of closed http/2 streams
This refactoring replaces closed streams with a new RecycledStream
object and changes the mechanism used to look up known streams.
Pull up state
---
.../apache/coyote/http2/AbstractNonZeroStream.java | 21 -
java/org/apache/coyote/http2/RecycledStream.java| 16 +---
java/org/apache/coyote/http2/Stream.java| 16 +---
3 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
index 582ab1e..084de33 100644
--- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
+++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
@@ -31,17 +31,22 @@ abstract class AbstractNonZeroStream extends AbstractStream
{
private static final Log log =
LogFactory.getLog(AbstractNonZeroStream.class);
private static final StringManager sm =
StringManager.getManager(AbstractNonZeroStream.class);
+protected final StreamStateMachine state;
+
private volatile int weight;
-AbstractNonZeroStream(Integer identifier) {
-this(identifier, Constants.DEFAULT_WEIGHT);
+AbstractNonZeroStream(String connectionId, Integer identifier) {
+super(identifier);
+this.weight = Constants.DEFAULT_WEIGHT;
+this.state = new StreamStateMachine(connectionId, getIdAsString());
}
-AbstractNonZeroStream(Integer identifier, int weight) {
+AbstractNonZeroStream(Integer identifier, int weight, StreamStateMachine
state) {
super(identifier);
this.weight = weight;
+this.state = state;
}
@@ -97,7 +102,13 @@ abstract class AbstractNonZeroStream extends AbstractStream
{
this.weight = weight;
}
-abstract boolean isClosedFinal();
-abstract void checkState(FrameType frameType) throws Http2Exception;
+final boolean isClosedFinal() {
+return state.isClosedFinal();
+}
+
+
+final void checkState(FrameType frameType) throws Http2Exception {
+state.checkFrameType(frameType);
+}
}
diff --git a/java/org/apache/coyote/http2/RecycledStream.java
b/java/org/apache/coyote/http2/RecycledStream.java
index 1915dff..9d6177c 100644
--- a/java/org/apache/coyote/http2/RecycledStream.java
+++ b/java/org/apache/coyote/http2/RecycledStream.java
@@ -23,12 +23,10 @@ package org.apache.coyote.http2;
class RecycledStream extends AbstractNonZeroStream {
private final String connectionId;
-private final StreamStateMachine state;
RecycledStream(String connectionId, Integer identifier, int weight,
StreamStateMachine state) {
-super(identifier, weight);
+super(identifier, weight, state);
this.connectionId = connectionId;
-this.state = state;
}
@@ -38,18 +36,6 @@ class RecycledStream extends AbstractNonZeroStream {
}
-@Override
-boolean isClosedFinal() {
-return state.isClosedFinal();
-}
-
-
-@Override
-final void checkState(FrameType frameType) throws Http2Exception {
-state.checkFrameType(frameType);
-}
-
-
@SuppressWarnings("sync-override")
@Override
void incrementWindowSize(int increment) throws Http2Exception {
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 2c1b714..e8a27ea 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -69,7 +69,6 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
private volatile long contentLengthReceived = 0;
private final Http2UpgradeHandler handler;
-private final StreamStateMachine state;
private final WindowAllocationManager allocationManager = new
WindowAllocationManager(this);
// State machine would be too much overhead
@@ -93,11 +92,10 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
Stream(Integer identifier, Http2UpgradeHandler handler, Request
coyoteRequest) {
-super(identifier);
+super(handler.getConnectionId(), identifier);
this.handler = handler;
handler.addChild(this);
setWindowSize(handler.getRemoteSettings().getInitialWindowSize());
-state = new StreamStateMachine(getConnectionId(), getIdAsString());
if (coyoteRequest == null) {
// HTTP/2 new request
this.coyoteRequest = new Request();
@@ -194,12 +192,6 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
@Override
-final void checkState(FrameType frameType) throws H
[tomcat] 09/09: Update changelog
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git commit 18e0e3f282dc016d59195eedf013bc3ee4fe0de2 Author: Mark Thomas AuthorDate: Fri Sep 25 14:33:45 2020 +0100 Update changelog --- webapps/docs/changelog.xml | 5 + 1 file changed, 5 insertions(+) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 319ae06..4929b46 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -120,6 +120,11 @@ Connector is configured with useAsyncIO="true". (markt) + +Refactor the handling of closed HTTP/2 streams to reduce the heap usage +associated with used streams and to retain information for more streams +in the priority tree. (markt) + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch master updated (519f6f8 -> 18e0e3f)
On 25/09/2020 14:34, ma...@apache.org wrote: > This is an automated email from the ASF dual-hosted git repository. > > markt pushed a change to branch master > in repository https://gitbox.apache.org/repos/asf/tomcat.git. > > > from 519f6f8 Fix a typo in changelog > new ee25710 Reduce memory footprint of closed http/2 streams > new f6e656e Reduce memory footprint of closed http/2 streams > new 0f66705 Reduce memory footprint of closed http/2 streams > new fa6df26 Reduce memory footprint of closed http/2 streams > new 9ee7d6a Reduce memory footprint of closed http/2 streams > new 30df6a0 Reduce memory footprint of closed http/2 streams > new 0a78ae9 Fully replace Stream with RecycledStream > new 954cbff Refactor the pruning so more stream info is retained for > longer > new 18e0e3f Update changelog All, This set of changes provided multiple improvements to the HTTP/2 implementation: 1. The memory used by closed streams is significantly reduced (hopefully without the NPEs triggered by my previous attempt). 2. We retain information on more streams in the priority tree. This enables greater latitude for clients that send frames for streams that have been closed (while not increasing memory). 3. We no longer aggressively prune the priority tree to just active streams (this was causing problems in some usage patterns). My plan is to let this run on the CI for a few days and then - assuming no issues - back-port it early next week. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kdillane commented on pull request #351: Remove White Spaces from the JSP files
kdillane commented on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-698458135 > > Since there is no new test for this I am not sure but looking at the code I think "Remove white space" is not quite accurate. > > It is rather "Remove empty lines". Because BLANK_LINE_PATTERN would match only if there zero or more empty spaces, followed by at least one new line, followed by 0 or more empty spaces. > > So the example above: > > ``` > > | > > | Quick Servlet Demo > > | > > ``` > > > > > > won't produce: > > ``` > > | > > | Quick Servlet Demo > > | > > ``` > > > > > > In addition: why preserve 1 new line ? HTML minifiers would produce something like `.` > > I think the node text comes like this `\n` and this will be caught by the BLANK_LINE_PATTERN and thus will later be turned down to `\n` after the processing. I assume you are not taking the node correctly. > > But on a broader aspect > > ``` > | > | Quick Servlet Demo > | > ``` > > Will produce > > ``` > | > | Quick Servlet Demo > | > ``` > > I agree to the minifying part, so we can rather just replace it with an empty character in the end instead of new line character and that will do the job too and will thus be more optimized. I'd recommend adding a test to cover this use-case. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 04/09: Reduce memory footprint of closed http/2 streams
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fa6df26815cea9a429291f4452711b96b00f02b0
Author: Mark Thomas
AuthorDate: Thu Sep 24 18:11:37 2020 +0100
Reduce memory footprint of closed http/2 streams
This refactoring replaces closed streams with a new RecycledStream
object and changes the mechanism used to look up known streams.
Refactoring getStream to handle differences between Stream and
RecycledStream
---
.../apache/coyote/http2/AbstractNonZeroStream.java | 2 +
.../apache/coyote/http2/Http2UpgradeHandler.java | 125 ++---
java/org/apache/coyote/http2/RecycledStream.java | 26 -
java/org/apache/coyote/http2/Stream.java | 20 ++--
4 files changed, 111 insertions(+), 62 deletions(-)
diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
index 70cfe35..582ab1e 100644
--- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
+++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
@@ -98,4 +98,6 @@ abstract class AbstractNonZeroStream extends AbstractStream {
}
abstract boolean isClosedFinal();
+
+abstract void checkState(FrameType frameType) throws Http2Exception;
}
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index cc07372..77dc339 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -29,6 +29,7 @@ import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
@@ -121,7 +122,7 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
private HpackDecoder hpackDecoder;
private HpackEncoder hpackEncoder;
-private final Map streams = new ConcurrentHashMap<>();
+private final ConcurrentMap streams = new
ConcurrentHashMap<>();
protected final AtomicInteger activeRemoteStreamCount = new
AtomicInteger(0);
// Start at -1 so the 'add 2' logic in closeIdleStreams() works
private volatile int maxActiveRemoteStreamId = -1;
@@ -1089,7 +1090,22 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
private Stream getStream(int streamId, boolean unknownIsError) throws
ConnectionException {
Integer key = Integer.valueOf(streamId);
-Stream result = streams.get(key);
+AbstractStream result = streams.get(key);
+if (result instanceof Stream) {
+return (Stream) result;
+}
+if (unknownIsError) {
+// Stream has been closed and removed from the map
+throw new
ConnectionException(sm.getString("upgradeHandler.stream.closed",
key.toString()),
+Http2Error.PROTOCOL_ERROR);
+}
+return null;
+}
+
+
+private AbstractNonZeroStream getStreamMayBeClosed(int streamId, boolean
unknownIsError) throws ConnectionException {
+Integer key = Integer.valueOf(streamId);
+AbstractNonZeroStream result = streams.get(key);
if (result == null && unknownIsError) {
// Stream has been closed and removed from the map
throw new
ConnectionException(sm.getString("upgradeHandler.stream.closed",
key.toString()),
@@ -1133,10 +1149,12 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
return;
}
-for (Stream stream : streams.values()) {
-// The connection is closing. Close the associated streams as no
-// longer required (also notifies any threads waiting for
allocations).
-stream.receiveReset(Http2Error.CANCEL.getCode());
+for (AbstractNonZeroStream stream : streams.values()) {
+if (stream instanceof Stream) {
+// The connection is closing. Close the associated streams as
no
+// longer required (also notifies any threads waiting for
allocations).
+((Stream) stream).receiveReset(Http2Error.CANCEL.getCode());
+}
}
try {
socketWrapper.close();
@@ -1193,10 +1211,10 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
TreeSet candidatesStepTwo = new TreeSet<>();
TreeSet candidatesStepThree = new TreeSet<>();
-for (Entry entry : streams.entrySet()) {
-Stream stream = entry.getValue();
+for (Entry entry : streams.entrySet())
{
+AbstractNonZeroStream stream = entry.ge
[Bug 60030] Run away CPU with JSSE / OpenSSL with IE8
https://bz.apache.org/bugzilla/show_bug.cgi?id=60030 popol777 changed: What|Removed |Added Resolution|FIXED |REMIND --- Comment #5 from popol777 --- try this https://gipuzkoa2.net https://domigado.com https://linktr.ee/Berita.Artis.Korea https://linktr.ee/Berita_Kpop -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64770] New: java.lang.IllegalStateException: Invalid request line parse phase [6] Tomcat 9.0.36
https://bz.apache.org/bugzilla/show_bug.cgi?id=64770
Bug ID: 64770
Summary: java.lang.IllegalStateException: Invalid request line
parse phase [6] Tomcat 9.0.36
Product: Tomcat 9
Version: 9.0.36
Hardware: PC
Status: NEW
Severity: blocker
Priority: P2
Component: Connectors
Assignee: dev@tomcat.apache.org
Reporter: devidutt...@gmail.com
Target Milestone: -
[https-jsse-nio-9443-exec-3] org.apache.coyote.http11.Http11Processor.service
Error parsing HTTP request header
Note: further occurrences of HTTP request parsing errors will be logged at
DEBUG level.
java.lang.IllegalStateException: Invalid request line parse phase [6]
Already have tried following options.
in server.xml
__
relaxedPathChars='[]|{|}'
relaxedQueryChars='{|}[]|{}^\`"'<>~;`;!;@;#;$;%;°;^;&;*;(;);-;_;+;=;{;};[;];|;\;/;:;";,;.;?'
in catalina.bat.
-Dorg.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true
-Dorg.apache.catalina.connector.CoyoteAdapter.ALLOW_BACKSLASH=true
Please help us as its a blocker now.
This issue is occuring after upgrading from Tocmat 8.5 to Tomcat 9.0.36
Tomcat Access logs:
10.1.135.53 - - [26/Sep/2020:11:27:52 +0530] "GET /ecpix/CCHInfo null" 400 1815
10.1.135.53 - - [26/Sep/2020:11:27:57 +0530] "GET /ecpix/CCHInfo null" 400 1815
10.1.135.53 - - [26/Sep/2020:11:28:02 +0530] "GET /ecpix/CCHInfo null" 400 1815
10.1.135.53 - - [26/Sep/2020:11:28:07 +0530] "GET /ecpix/CCHInfo null" 400 1815
10.1.135.53 - - [26/Sep/2020:11:28:12 +0530] "GET /ecpix/CCHInfo null" 400 1815
10.1.135.53 - - [26/Sep/2020:11:28:17 +0530] "GET /ecpix/CCHInfo null" 400 1815
Stderr file:
26-Sep-2020 11:27:50.167 INFO [main] org.apache.catalina.startup.Catalina.start
Server startup in [70,002]milliseconds
26-Sep-2020 11:27:52.307 INFO [https-jsse-nio-9443-exec-4]
org.apache.coyote.http11.Http11Processor.service Error parsing
HTTP request header
Note: further occurrences of HTTP request parsing errors will be logged at
DEBUG level.
java.lang.IllegalStateException: Invalid request line parse phase [6]
at
org.apache.coyote.http11.Http11InputBuffer.parseRequestLine(Http11InputBuffer.java:579)
at
org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:260)
at
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
at
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:868)
at
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1590)
at
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
at java.lang.Thread.run(Thread.java:748)
--
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64770] java.lang.IllegalStateException: Invalid request line parse phase [6] Tomcat 9.0.36
https://bz.apache.org/bugzilla/show_bug.cgi?id=64770 Deviduttapanda changed: What|Removed |Added OS||All CC||devidutt...@gmail.com -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
