Re: (tomcat) branch main updated: Don't create a StringBuilder object until we know we have at least one Cookie value to log.
Mark,
On 4/18/24 11:12, Mark Thomas wrote:
On 18/04/2024 14:31, 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 23facd507d Don't create a StringBuilder object until we know
we have at least one Cookie value to log.
23facd507d is described below
commit 23facd507db72d583ed89a13f20ab1cb766f0221
Author: Christopher Schultz
AuthorDate: Thu Apr 18 09:30:50 2024 -0400
Don't create a StringBuilder object until we know we have at
least one Cookie value to log.
-1. veto. Please fix/revert ASAP.
Note: This veto applies to this commit and the back-ports.
This creates multiple paths where a NPE is possible.
OMG what the heck happened to this patch? Grr. I saw this while working
on the timestamp-style stuff and decided to separate it out into a
separate commit and but did I get it wrong. It NPEs on /every/ path :(
Sorry for such a low-quality commit.
I'm going to try a "correct" commit on top of it and would appreciate a
review. If it still looks like a no-go, I'll revert the whole thing.
This does not work if there are multiple cookies with the same name that
need to be logged.
ACK
Thanks,
-chris
---
java/org/apache/catalina/valves/AbstractAccessLogValve.java | 3 ++-
webapps/docs/changelog.xml | 4
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git
a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
index 5502d1c183..e13bb9e5ac 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -1479,7 +1479,7 @@ public abstract class AbstractAccessLogValve
extends ValveBase implements Access
@Override
public void addElement(CharArrayWriter buf, Date date,
Request request, Response response, long time) {
- StringBuilder value = new StringBuilder();
+ StringBuilder value = null;
boolean first = true;
Cookie[] cookies = request.getCookies();
if (cookies != null) {
@@ -1490,6 +1490,7 @@ public abstract class AbstractAccessLogValve
extends ValveBase implements Access
} else {
value.append(',');
}
+ value = new StringBuilder();
value.append(cookie.getValue());
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8ef77e52aa..f6c6c62962 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -123,6 +123,10 @@
including the removal of the trimCredentials
setting which
is now hard-coded to false. (markt)
+
+ Small performance optimization when logging cookies with no
values.
+ (schultz)
+
-
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
Re: (tomcat) branch main updated: Don't create a StringBuilder object until we know we have at least one Cookie value to log.
Mark,
On 4/19/24 08:38, Mark Thomas wrote:
Ping. Just making sure this veto hasn't been lost in the recent flurry
of commits.
ACK
I'll revert and re-evaluate.
Thanks,
-chris
On 18/04/2024 16:12, Mark Thomas wrote:
On 18/04/2024 14:31, 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 23facd507d Don't create a StringBuilder object until we
know we have at least one Cookie value to log.
23facd507d is described below
commit 23facd507db72d583ed89a13f20ab1cb766f0221
Author: Christopher Schultz
AuthorDate: Thu Apr 18 09:30:50 2024 -0400
Don't create a StringBuilder object until we know we have at
least one Cookie value to log.
-1. veto. Please fix/revert ASAP.
Note: This veto applies to this commit and the back-ports.
This creates multiple paths where a NPE is possible.
This does not work if there are multiple cookies with the same name
that need to be logged.
Mark
---
java/org/apache/catalina/valves/AbstractAccessLogValve.java | 3 ++-
webapps/docs/changelog.xml | 4
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git
a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
index 5502d1c183..e13bb9e5ac 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -1479,7 +1479,7 @@ public abstract class AbstractAccessLogValve
extends ValveBase implements Access
@Override
public void addElement(CharArrayWriter buf, Date date,
Request request, Response response, long time) {
- StringBuilder value = new StringBuilder();
+ StringBuilder value = null;
boolean first = true;
Cookie[] cookies = request.getCookies();
if (cookies != null) {
@@ -1490,6 +1490,7 @@ public abstract class AbstractAccessLogValve
extends ValveBase implements Access
} else {
value.append(',');
}
+ value = new StringBuilder();
value.append(cookie.getValue());
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8ef77e52aa..f6c6c62962 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -123,6 +123,10 @@
including the removal of the trimCredentials
setting which
is now hard-coded to false. (markt)
+
+ Small performance optimization when logging cookies with no
values.
+ (schultz)
+
-
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
Re: (tomcat) branch main updated: Don't create a StringBuilder object until we know we have at least one Cookie value to log.
Ping. Just making sure this veto hasn't been lost in the recent flurry
of commits.
Mark
On 18/04/2024 16:12, Mark Thomas wrote:
On 18/04/2024 14:31, 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 23facd507d Don't create a StringBuilder object until we know
we have at least one Cookie value to log.
23facd507d is described below
commit 23facd507db72d583ed89a13f20ab1cb766f0221
Author: Christopher Schultz
AuthorDate: Thu Apr 18 09:30:50 2024 -0400
Don't create a StringBuilder object until we know we have at
least one Cookie value to log.
-1. veto. Please fix/revert ASAP.
Note: This veto applies to this commit and the back-ports.
This creates multiple paths where a NPE is possible.
This does not work if there are multiple cookies with the same name that
need to be logged.
Mark
---
java/org/apache/catalina/valves/AbstractAccessLogValve.java | 3 ++-
webapps/docs/changelog.xml | 4
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git
a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
index 5502d1c183..e13bb9e5ac 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -1479,7 +1479,7 @@ public abstract class AbstractAccessLogValve
extends ValveBase implements Access
@Override
public void addElement(CharArrayWriter buf, Date date,
Request request, Response response, long time) {
- StringBuilder value = new StringBuilder();
+ StringBuilder value = null;
boolean first = true;
Cookie[] cookies = request.getCookies();
if (cookies != null) {
@@ -1490,6 +1490,7 @@ public abstract class AbstractAccessLogValve
extends ValveBase implements Access
} else {
value.append(',');
}
+ value = new StringBuilder();
value.append(cookie.getValue());
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8ef77e52aa..f6c6c62962 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -123,6 +123,10 @@
including the removal of the trimCredentials
setting which
is now hard-coded to false. (markt)
+
+ Small performance optimization when logging cookies with no
values.
+ (schultz)
+
-
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
Re: (tomcat) branch main updated: Don't create a StringBuilder object until we know we have at least one Cookie value to log.
On 18/04/2024 14:31, 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 23facd507d Don't create a StringBuilder object until we know we have
at least one Cookie value to log.
23facd507d is described below
commit 23facd507db72d583ed89a13f20ab1cb766f0221
Author: Christopher Schultz
AuthorDate: Thu Apr 18 09:30:50 2024 -0400
Don't create a StringBuilder object until we know we have at least one
Cookie value to log.
-1. veto. Please fix/revert ASAP.
Note: This veto applies to this commit and the back-ports.
This creates multiple paths where a NPE is possible.
This does not work if there are multiple cookies with the same name that
need to be logged.
Mark
---
java/org/apache/catalina/valves/AbstractAccessLogValve.java | 3 ++-
webapps/docs/changelog.xml | 4
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
index 5502d1c183..e13bb9e5ac 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -1479,7 +1479,7 @@ public abstract class AbstractAccessLogValve extends
ValveBase implements Access
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
-StringBuilder value = new StringBuilder();
+StringBuilder value = null;
boolean first = true;
Cookie[] cookies = request.getCookies();
if (cookies != null) {
@@ -1490,6 +1490,7 @@ public abstract class AbstractAccessLogValve extends
ValveBase implements Access
} else {
value.append(',');
}
+value = new StringBuilder();
value.append(cookie.getValue());
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8ef77e52aa..f6c6c62962 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -123,6 +123,10 @@
including the removal of the trimCredentials setting
which
is now hard-coded to false. (markt)
+
+Small performance optimization when logging cookies with no values.
+(schultz)
+
-
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
