[tomcat] branch master updated (8713546 -> 6f17800)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[tomcat] branch master updated (8713546 -> 6f17800)

Martin Grigorov
This is an automated email from the ASF dual-hosted git repository.

mgrigorov pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from 8713546  OWB 2.0.18
     new d2ed8ff  Small optimizations in HpackEncoder
     new 6f17800  http2: Add 'EndOfStream' to the logs for writeHeaders and writeBody

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 java/org/apache/coyote/http2/HpackEncoder.java     | 12 +++++++++---
 .../coyote/http2/Http2AsyncUpgradeHandler.java     |  6 +++---
 .../apache/coyote/http2/Http2UpgradeHandler.java   | 22 ++++++++++++----------
 .../apache/coyote/http2/LocalStrings.properties    |  4 ++--
 4 files changed, 26 insertions(+), 18 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[tomcat] 01/02: Small optimizations in HpackEncoder

Martin Grigorov
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

commit d2ed8ffc75c5e3b425888b456ffc51036d94ac39
Author: Martin Tzvetanov Grigorov <[hidden email]>
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
---
 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 dda5714..ae81033 100644
--- a/java/org/apache/coyote/http2/HpackEncoder.java
+++ b/java/org/apache/coyote/http2/HpackEncoder.java
@@ -44,7 +44,13 @@ 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 @@ 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<String, List<TableEntry>> entry : dynamicTable.entrySet()) {
-            for (TableEntry t : entry.getValue()) {
+        for (List<TableEntry> tableEntries : dynamicTable.values()) {
+            for (TableEntry t : tableEntries) {
                 t.position = t.getPosition();
             }
         }


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[tomcat] 02/02: http2: Add 'EndOfStream' to the logs for writeHeaders and writeBody

Martin Grigorov
In reply to this post by Martin Grigorov
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

commit 6f178006f246ce9248a349705ba79d054d189b70
Author: Martin Tzvetanov Grigorov <[hidden email]>
AuthorDate: Thu Sep 24 13:46:31 2020 +0300

    http2: Add 'EndOfStream' to the logs for writeHeaders and writeBody
   
    Mark Http2AsyncUpgradeHandler#errorCompletion and #applicationErrorCompletion as final.
    Call streams.size() just once.
    Extract local variables and reuse them instead of calling getters on every usage
---
 .../coyote/http2/Http2AsyncUpgradeHandler.java     |  6 +++---
 .../apache/coyote/http2/Http2UpgradeHandler.java   | 22 ++++++++++++----------
 .../apache/coyote/http2/LocalStrings.properties    |  4 ++--
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
index 951c21c..9c274ac 100644
--- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
@@ -51,7 +51,7 @@ public class Http2AsyncUpgradeHandler extends Http2UpgradeHandler {
         super(protocol, adapter, coyoteRequest);
     }
 
-    private CompletionHandler<Long, Void> errorCompletion = new CompletionHandler<Long, Void>() {
+    private final CompletionHandler<Long, Void> errorCompletion = new CompletionHandler<Long, Void>() {
         @Override
         public void completed(Long result, Void attachment) {
         }
@@ -60,7 +60,7 @@ public class Http2AsyncUpgradeHandler extends Http2UpgradeHandler {
             error = t;
         }
     };
-    private CompletionHandler<Long, Void> applicationErrorCompletion = new CompletionHandler<Long, Void>() {
+    private final CompletionHandler<Long, Void> applicationErrorCompletion = new CompletionHandler<Long, Void>() {
         @Override
         public void completed(Long result, Void attachment) {
         }
@@ -199,7 +199,7 @@ public class Http2AsyncUpgradeHandler extends Http2UpgradeHandler {
     void writeBody(Stream stream, ByteBuffer data, int len, boolean finished) throws IOException {
         if (log.isDebugEnabled()) {
             log.debug(sm.getString("upgradeHandler.writeBody", connectionId, stream.getIdAsString(),
-                    Integer.toString(len)));
+                    Integer.toString(len), Boolean.valueOf(finished)));
         }
         // Need to check this now since sending end of stream will change this.
         boolean writeable = stream.canWrite();
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index bc6fed0..152b2f3 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -665,7 +665,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
         if (log.isDebugEnabled()) {
             if (pushedStreamId == 0) {
                 log.debug(sm.getString("upgradeHandler.writeHeaders", connectionId,
-                        stream.getIdAsString()));
+                        stream.getIdAsString(), Boolean.valueOf(endOfStream)));
             } else {
                 log.debug(sm.getString("upgradeHandler.writePushHeaders", connectionId,
                         stream.getIdAsString(), Integer.valueOf(pushedStreamId),
@@ -1160,9 +1160,10 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
         // maximum number of concurrent streams
         long max = localSettings.getMaxConcurrentStreams();
 
+        final int size = streams.size();
         if (log.isDebugEnabled()) {
             log.debug(sm.getString("upgradeHandler.pruneStart", connectionId,
-                    Long.toString(max), Integer.toString(streams.size())));
+                    Long.toString(max), Integer.toString(size)));
         }
 
         // Allow an additional 10% for closed streams that are used in the
@@ -1172,7 +1173,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
             max = Integer.MAX_VALUE;
         }
 
-        int toClose = streams.size() - (int) max;
+        int toClose = size - (int) max;
         if (toClose < 1) {
             return;
         }
@@ -1199,16 +1200,17 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
                 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(entry.getKey());
+                candidatesStepThree.add(streamIdentifier);
             } else if (stream.getChildStreams().size() == 0) {
                 // Closed, no children
-                candidatesStepOne.add(entry.getKey());
+                candidatesStepOne.add(streamIdentifier);
             } else {
                 // Closed, with children
-                candidatesStepTwo.add(entry.getKey());
+                candidatesStepTwo.add(streamIdentifier);
             }
         }
 
@@ -1268,9 +1270,9 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
         // Move the removed Stream's children to the removed Stream's
         // parent.
         Set<Stream> children = streamToRemove.getChildStreams();
-        if (streamToRemove.getChildStreams().size() == 1) {
+        if (children.size() == 1) {
             // Shortcut
-            streamToRemove.getChildStreams().iterator().next().rePrioritise(
+            children.iterator().next().rePrioritise(
                     streamToRemove.getParentStream(), streamToRemove.getWeight());
         } else {
             int totalWeight = 0;
@@ -1278,13 +1280,13 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
                 totalWeight += child.getWeight();
             }
             for (Stream child : children) {
-                streamToRemove.getChildStreams().iterator().next().rePrioritise(
+                children.iterator().next().rePrioritise(
                         streamToRemove.getParentStream(),
                         streamToRemove.getWeight() * child.getWeight() / totalWeight);
             }
         }
         streamToRemove.detachFromParent();
-        streamToRemove.getChildStreams().clear();
+        children.clear();
     }
 
 
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties
index f8ceff1..c629a40 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -150,8 +150,8 @@ upgradeHandler.upgradeDispatch.entry=Entry, Connection [{0}], SocketStatus [{1}]
 upgradeHandler.upgradeDispatch.exit=Exit, Connection [{0}], SocketState [{1}]
 upgradeHandler.windowSizeReservationInterrupted=Connection [{0}], Stream [{1}], reservation for [{2}] bytes
 upgradeHandler.windowSizeTooBig=Connection [{0}], Stream [{1}], Window size too big
-upgradeHandler.writeBody=Connection [{0}], Stream [{1}], Data length [{2}]
-upgradeHandler.writeHeaders=Connection [{0}], Stream [{1}]
+upgradeHandler.writeBody=Connection [{0}], Stream [{1}], Data length [{2}], EndOfStream [{3}]
+upgradeHandler.writeHeaders=Connection [{0}], Stream [{1}], Writing the headers, EndOfStream [{2}]
 upgradeHandler.writePushHeaders=Connection [{0}], Stream [{1}], Pushed stream [{2}], EndOfStream [{3}]
 
 windowAllocationManager.dispatched=Connection [{0}], Stream [{1}], Dispatched


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] 01/02: Small optimizations in HpackEncoder

Martin Grigorov
In reply to this post by Martin Grigorov
On Thu, Sep 24, 2020 at 1:50 PM <[hidden email]> 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
>
> commit d2ed8ffc75c5e3b425888b456ffc51036d94ac39
> Author: Martin Tzvetanov Grigorov <[hidden email]>
> 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
>

this should read: ... no need to *create* Map.Entry


> ---
>  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 dda5714..ae81033 100644
> --- a/java/org/apache/coyote/http2/HpackEncoder.java
> +++ b/java/org/apache/coyote/http2/HpackEncoder.java
> @@ -44,7 +44,13 @@ 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 @@ 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<String, List<TableEntry>> entry :
> dynamicTable.entrySet()) {
> -            for (TableEntry t : entry.getValue()) {
> +        for (List<TableEntry> tableEntries : dynamicTable.values()) {
> +            for (TableEntry t : tableEntries) {
>                  t.position = t.getPosition();
>              }
>          }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>