This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 50a931abd48271d80ad1d32536ae773d87dfddce
Author: Mark Thomas <ma...@apache.org>
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 | 8 +--
.../apache/coyote/http2/Http2UpgradeHandler.java | 10 +--
java/org/apache/coyote/http2/RecycledStream.java | 10 +--
java/org/apache/coyote/http2/Stream.java | 59 +-----------------
5 files changed, 83 insertions(+), 75 deletions(-)
diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
index 63fb5e7..e76496d 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
+ protected 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<AbstractNonZeroStream> 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 9c6054e..43b8cde 100644
--- a/java/org/apache/coyote/http2/AbstractStream.java
+++ b/java/org/apache/coyote/http2/AbstractStream.java
@@ -37,8 +37,8 @@ abstract class AbstractStream {
private final String idAsString;
private volatile AbstractStream parentStream = null;
- private final Set<Stream> childStreams =
- Collections.newSetFromMap(new ConcurrentHashMap<Stream,Boolean>());
+ private final Set<AbstractNonZeroStream> childStreams =
+ Collections.newSetFromMap(new
ConcurrentHashMap<AbstractNonZeroStream,Boolean>());
private long windowSize =
ConnectionSettingsBase.DEFAULT_INITIAL_WINDOW_SIZE;
@@ -71,7 +71,7 @@ abstract class AbstractStream {
}
- final void addChild(Stream child) {
+ final void addChild(AbstractNonZeroStream child) {
child.setParentStream(this);
childStreams.add(child);
}
@@ -98,7 +98,7 @@ abstract class AbstractStream {
}
- final Set<Stream> getChildStreams() {
+ final Set<AbstractNonZeroStream> getChildStreams() {
return childStreams;
}
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 1ff8b3e..4e15e9d 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1260,18 +1260,18 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
Stream streamToRemove = streams.remove(streamIdToRemove);
// Move the removed Stream's children to the removed Stream's
// parent.
- Set<Stream> children = streamToRemove.getChildStreams();
- if (streamToRemove.getChildStreams().size() == 1) {
+ Set<AbstractNonZeroStream> children = streamToRemove.getChildStreams();
+ if (children.size() == 1) {
// Shortcut
streamToRemove.getChildStreams().iterator().next().rePrioritise(
streamToRemove.getParentStream(),
streamToRemove.getWeight());
} else {
int totalWeight = 0;
- for (Stream child : children) {
+ for (AbstractNonZeroStream child : children) {
totalWeight += child.getWeight();
}
- for (Stream child : children) {
-
streamToRemove.getChildStreams().iterator().next().rePrioritise(
+ for (AbstractNonZeroStream child : children) {
+ children.iterator().next().rePrioritise(
streamToRemove.getParentStream(),
streamToRemove.getWeight() * child.getWeight() /
totalWeight);
}
diff --git a/java/org/apache/coyote/http2/RecycledStream.java
b/java/org/apache/coyote/http2/RecycledStream.java
index 4cd3e79..17a02cb 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 int weight;
RecycledStream(Stream stream) {
- super(stream.getIdentifier());
+ super(stream.getIdentifier(), stream.getWeight());
connectionId = stream.getConnectionId();
- weight = stream.getWeight();
}
@@ -39,12 +37,6 @@ class RecycledStream extends AbstractNonZeroStream {
@Override
- protected int getWeight() {
- return weight;
- }
-
-
- @Override
@Deprecated
protected synchronized void doNotifyAll() {
// NO-OP. Unused.
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index d4cfa0d..be33904 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -22,7 +22,6 @@ import java.nio.charset.StandardCharsets;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
-import java.util.Iterator;
import java.util.Locale;
import org.apache.coyote.ActionCode;
@@ -64,7 +63,6 @@ public class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
ACK_HEADERS = response.getMimeHeaders();
}
- private volatile int weight = Constants.DEFAULT_WEIGHT;
private volatile long contentLengthReceived = 0;
private final Http2UpgradeHandler handler;
@@ -173,54 +171,7 @@ public class Stream extends AbstractNonZeroStream
implements HeaderEmitter {
}
- 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<Stream> parentsChildren =
parent.getChildStreams().iterator();
- while (parentsChildren.hasNext()) {
- Stream 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;
- }
-
-
- void receiveReset(long errorCode) {
+ final void receiveReset(long errorCode) {
if (log.isDebugEnabled()) {
log.debug(sm.getString("stream.reset.receive", getConnectionId(),
getIdAsString(),
Long.toString(errorCode)));
@@ -534,13 +485,7 @@ public class Stream extends AbstractNonZeroStream
implements HeaderEmitter {
}
- @Override
- protected int getWeight() {
- return weight;
- }
-
-
- Request getCoyoteRequest() {
+ final Request getCoyoteRequest() {
return coyoteRequest;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
