Chuck,

On 4/19/24 10:48, Chuck Caldarale wrote:
On Apr 19, 2024, at 09:18, Christopher Schultz <ch...@christopherschultz.net> 
wrote:

Hopefully this patch has the intended effect. ;)


I’m not convinced this change will have any measurable performance
improvement. The JVM C2 compiler is pretty good with escape analysis,
so an unused StringBuilder object may not even get allocated.
It should get allocated, since the constructor needs to be called. But it may be allocated in a cheap memory region and immediately become speedily-collected garbage.

Also, there’s now an added comparison for each iteration of the
cookies loop, plus the additional code for an object allocation. This
enlarges the body of the loop, putting more pressure on the microcode
cache in the CPU, possibly making each iteration take longer.
That's a fair criticism.

Are there any practical examples that show a performance benefit or GC 
reduction?

None.

I made this change merely based upon code inspection. Since this code executes for every single request, I guessed without evidence that reduction of memory-churn would be beneficial.

-chris

On 4/19/24 10:17, schu...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository.
schultz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
      new cbefe8624e Fix disastrous cookie-logging patch.
cbefe8624e is described below
commit cbefe8624ee5d6255955134d08498f9926295126
Author: Christopher Schultz <ch...@christopherschultz.net>
AuthorDate: Fri Apr 19 10:16:36 2024 -0400
     Fix disastrous cookie-logging patch.
---
  java/org/apache/catalina/valves/AbstractAccessLogValve.java | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java 
b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
index 0576b83442..dd29a5ec37 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -1513,17 +1513,19 @@ public abstract class AbstractAccessLogValve extends 
ValveBase implements Access
              if (cookies != null) {
                  for (Cookie cookie : cookies) {
                      if (cookieNameToLog.equals(cookie.getName())) {
+                        if (value == null) {
+                            value = new StringBuilder();
+                        }
                          if (first) {
                              first = false;
                          } else {
                              value.append(',');
                          }
-                        value = new StringBuilder();
                          value.append(cookie.getValue());
                      }
                  }
              }
-            if (value.length() == 0) {
+            if (value == null) {
                  buf.append('-');
              } else {
                  escapeAndAppend(value.toString(), buf);
---------------------------------------------------------------------
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



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

Reply via email to