[Bug 60030] Run away CPU with JSSE / OpenSSL with IE8

2020-09-25 Thread bugzilla
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



[GitHub] [tomcat] kdillane commented on pull request #351: Remove White Spaces from the JSP files

2020-09-25 Thread GitBox


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

2020-09-25 Thread markt
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 = 

Re: [tomcat] branch master updated (519f6f8 -> 18e0e3f)

2020-09-25 Thread Mark Thomas
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



[tomcat] 09/09: Update changelog

2020-09-25 Thread markt
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



[tomcat] 06/09: Reduce memory footprint of closed http/2 streams

2020-09-25 Thread markt
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 

[tomcat] 07/09: Fully replace Stream with RecycledStream

2020-09-25 Thread markt
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
-// 

[tomcat] 08/09: Refactor the pruning so more stream info is retained for longer

2020-09-25 Thread markt
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] branch master updated (519f6f8 -> 18e0e3f)

2020-09-25 Thread markt
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] 03/09: Reduce memory footprint of closed http/2 streams

2020-09-25 Thread markt
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

2020-09-25 Thread markt
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] 01/09: Reduce memory footprint of closed http/2 streams

2020-09-25 Thread markt
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 

[tomcat] 02/09: Reduce memory footprint of closed http/2 streams

2020-09-25 Thread markt
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 

[GitHub] [tomcat] kamnani commented on pull request #351: Remove White Spaces from the JSP files

2020-09-25 Thread GitBox


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



[GitHub] [tomcat] kamnani edited a comment on pull request #351: Remove White Spaces from the JSP files

2020-09-25 Thread GitBox


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] martin-g commented on pull request #351: Remove White Spaces from the JSP files

2020-09-25 Thread GitBox


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] martin-g commented on a change in pull request #351: Remove White Spaces from the JSP files

2020-09-25 Thread GitBox


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] kamnani commented on a change in pull request #351: Remove White Spaces from the JSP files

2020-09-25 Thread GitBox


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



buildbot success in on tomcat-85-trunk

2020-09-25 Thread buildbot
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



Re: [tomcat] branch master updated: Fix a typo in changelog

2020-09-25 Thread Mark Thomas
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-9-trunk

2020-09-25 Thread buildbot
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



[tomcat] branch 9.0.x updated: Fix a typo in changelog

2020-09-25 Thread mgrigorov
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 8.5.x updated: Small optimizations in HpackEncoder

2020-09-25 Thread mgrigorov
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

2020-09-25 Thread mgrigorov
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

2020-09-25 Thread mgrigorov
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 master updated: Fix a typo in changelog

2020-09-25 Thread mgrigorov
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