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 83f5e03b52c54a9b213e979c9bb47e0967530cbe
Author: Mark Thomas <ma...@apache.org>
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 aebb70b..9f71af9 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<Integer,AbstractNonZeroStream> streams = new
ConcurrentHashMap<>();
+ private final ConcurrentMap<Integer,AbstractNonZeroStream> 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<Integer> candidatesStepOne = new TreeSet<>();
TreeSet<Integer> candidatesStepTwo = new TreeSet<>();
TreeSet<Integer> candidatesStepThree = new TreeSet<>();
- for (Entry<Integer, AbstractNonZeroStream> 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));
+ log.debug(sm.getString("upgradeHandler.pruned",
connectionId, stream.getIdAsString()));
+ }
+ if (--toClose < 1) {
+ return;
}
- // Also need to remove this stream from the p2 list
- candidatesStepTwo.remove(parent.getIdentifier());
- parent = parent.getParentStream();
+
+ // If removing this child made the parent childless then see if
+ // the parent can be removed.
+ // Don't try and remove Stream 0 as that is the connection
+ // Don't try and remove 'newer' streams. We'll get to them as
we
+ // work through the ordered list of streams.
+ while (toClose > 0 && parent.getIdAsInt() > 0 &&
parent.getIdAsInt() < stream.getIdAsInt() &&
+ parent.getChildStreams().isEmpty()) {
+ // This case is safe since we know parent ID > 0 therefore
+ // this isn't the connection
+ stream = (AbstractNonZeroStream) parent;
+ parent = stream.getParentStream();
+ streams.remove(stream.getIdentifier());
+ stream.detachFromParent();
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("upgradeHandler.pruned",
connectionId, stream.getIdAsString()));
+ }
+ if (--toClose < 1) {
+ return;
+ }
+ // Also need to remove this stream from the step 2 list
+ candidatesStepTwo.remove(stream.getIdentifier());
+ }
+ } else {
+ // Closed, with children. Candidate for step 2.
+ candidatesStepTwo.add(stream.getIdentifier());
}
}
// Process the P2 list
for (Integer streamIdToRemove : candidatesStepTwo) {
removeStreamFromPriorityTree(streamIdToRemove);
- toClose--;
if (log.isDebugEnabled()) {
log.debug(sm.getString("upgradeHandler.pruned", connectionId,
streamIdToRemove));
}
+ if (--toClose < 1) {
+ return;
+ }
}
while (toClose > 0 && candidatesStepThree.size() > 0) {
Integer streamIdToRemove = candidatesStepThree.pollLast();
removeStreamFromPriorityTree(streamIdToRemove);
- toClose--;
if (log.isDebugEnabled()) {
log.debug(sm.getString("upgradeHandler.prunedPriority",
connectionId, streamIdToRemove));
}
+ if (--toClose < 1) {
+ return;
+ }
}
if (toClose > 0) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
