You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2025/01/28 17:46:06 UTC
[Bug 69552] New: Performance regression in MessageBytes.toBytes() when using non-default charset
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
Bug ID: 69552
Summary: Performance regression in MessageBytes.toBytes() when
using non-default charset
Product: Tomcat 9
Version: 9.0.98
Hardware: All
OS: All
Status: NEW
Severity: normal
Priority: P2
Component: Util
Assignee: dev@tomcat.apache.org
Reporter: jengebr@amazon.com
Target Milestone: -----
Created attachment 39982
--> https://bz.apache.org/bugzilla/attachment.cgi?id=39982&action=edit
Speed test
Our high-volume, latency-sensitive application shows that
MessageBytes.toBytes() is a bottleneck on our critical path, with an aggregate
impact of 0.06% (small but real). Investigation shows there is a fast path
within the method that activates only when the charset is
ByteChunk.DEFAULT_CHARSET (ISO_8859_1), so applications using UTF-8 (such as
ours) are always sent to the slow path. I don't immediately see a reason for
the bias towards the default charset.
The attached speed test demonstrates a 40x difference between the fast and slow
paths, as well as significantly different memory allocation (fast path
allocates nothing).
MessageBytes.toBytes() is usually called by jsps referenced similar to this:
<jsp:include page="myNestedJsp">
<jsp:param .../>
</jsp:include>
which forces the nested JSP to re-parse the full set of parameters.
Ideally we can find a way to move the common case (UTF-8) to the fast path, but
I hesitate to suggest anything without understanding the bias towards
ISO_8859_1.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
--- Comment #3 from John Engebretson <je...@amazon.com> ---
Thank you, that explains ISO_8859_1 vs. UTF_8.
Where can I find TestMessageBytesPerformance.java?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
--- Comment #1 from John Engebretson <je...@amazon.com> ---
Created attachment 39983
--> https://bz.apache.org/bugzilla/attachment.cgi?id=39983&action=edit
Helper class for speed test
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
--- Comment #8 from Mark Thomas <ma...@apache.org> ---
Is this addressing the correct problem?
That patch suggests that the issue is multiple calls to getBytes() for the same
MessageBytes instance that is currently set to the String type. Hence repeated
String to Bytes conversion causing the delay.
We already have a mechanism to cache the bytes - they are saved in the
ByteChunk. The subsequent calls should be made to getByteChunk().getBytes() /
getStart() / getLength()
Alternatively, we could add getBytesToType() method that would convert to bytes
and then change the type.
The complicating factor here is that the caller is responsible for keeping
track of which cached conversions are valid and which ones are out of date.
I think we need a better understanding of the callers to getBytes() to decide
on the best solution.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
--- Comment #5 from John Engebretson <je...@amazon.com> ---
Thank you - my forked repo doesn't have it for some reason, despite syncing.
Time to wipe it out and recreate. :( Nice to see you found the same behavior.
Clearly UTF_8 can't go to the fast path; I'll chew on this a little while
longer to see if there are any other optimizations available.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
--- Comment #7 from John Engebretson <je...@amazon.com> ---
Looks like the right move here is to add an extra pointer on MessageBytes and
reuse the utf8 payload. For example, modify the last part of toBytes() with an
extra branch:
ByteBuffer bb;
if (type == T_CHARS) {
bb = getCharset().encode(CharBuffer.wrap(charC));
/* next lines are new */
} else if (getCharset() == StandardCharsets.UTF_8) {
if (utf8_bytes == null) {
utf8_bytes = StandardCharsets.UTF_8.encode(strValue);
}
bb = utf8_bytes;
} else {
// Must be T_STR
bb = getCharset().encode(strValue);
}
This eliminates the redundant parsing of the message and allocating/populating
of the ByteBuffer... a.k.a. large reduction in object allocation and duplicate
work.
The tradeoff is that it extends the lifetime of the currently-created
ByteBuffers from this method to the lifetime of the MessageBytes instance,
which is typically the duration of the current request. That seems acceptable
to me.
Also, this optimization is applicable only to applications outputting utf-8.
Various sources on the web say utf-8 is by far the most common, so we should be
okay; the second-most common is ISO_8859_1 and that is already optimized. Any
other character set should see net-neutral performance from this change.
Thoughts?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
--- Comment #2 from Mark Thomas <ma...@apache.org> ---
The fast path only works because of the one to one direct mapping of bytes to
characters in ISO_8859_1. That doesn't apply to UTF-8.
See also TestMessageBytesPerformance and the note re changes in Java 16
onwards.
Just from looking at the code for that test and the test case attached to this
bug, I suspect there are two parts to this:
- the efficiency of the conversion processes
- the overhead of the fast vs slow path
I don't see how UTF-8 could be moved to the fast path given there isn't a one
to one direct mapping between bytes and characters for UTF-8.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|normal |enhancement
--- Comment #6 from Mark Thomas <ma...@apache.org> ---
Moving this to enhancement while you ponder.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
--- Comment #4 from Mark Thomas <ma...@apache.org> ---
https://github.com/apache/tomcat/blob/9.0.x/test/org/apache/tomcat/util/buf/TestMessageBytesPerformance.java
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
--- Comment #9 from John Engebretson <je...@amazon.com> ---
Here's the stack trace that accounts for 90% of the impact:
sun.nio.cs.UTF_8$Encoder.encodeBufferLoop
sun.nio.cs.UTF_8$Encoder.encodeLoop
java.nio.charset.CharsetEncoder.encode
java.nio.charset.CharsetEncoder.encode
java.nio.charset.Charset.encode
java.nio.charset.Charset.encode
org.apache.tomcat.util.buf.MessageBytes.toBytes
org.apache.tomcat.util.http.Parameters.processParameters
org.apache.tomcat.util.http.Parameters.handleQueryParameters
org.apache.catalina.core.ApplicationHttpRequest.mergeParameters
org.apache.catalina.core.ApplicationHttpRequest.parseParameters
org.apache.catalina.core.ApplicationHttpRequest.getParameter
com.amazon.core.platform.runtime.ThreadSafeRequestFilter$ThreadSafeRequestWrapper.getParameter
javax.servlet.ServletRequestWrapper.getParameter
javax.servlet.ServletRequestWrapper.getParameter
com.amazon.horizonte.servlet.io.MutableParametersHttpServletRequest.getParameter
javax.servlet.jsp.el.ImplicitObjectELResolver$ScopeManager$7.getAttribute
javax.servlet.jsp.el.ImplicitObjectELResolver$ScopeManager$7.getAttribute
javax.servlet.jsp.el.ImplicitObjectELResolver$ScopeMap.get
javax.el.MapELResolver.getValue
org.apache.jasper.el.JasperELResolver.getValue
org.apache.el.parser.AstValue.getValue
org.apache.el.parser.AstAbstractEmpty.getValue
org.apache.el.ValueExpressionImpl.getValue
org.apache.jasper.runtime.PageContextImpl.proprietaryEvaluate
(org.apache.jsp.WEB_002dINF.<proprietary>_jsp._jspx_meth_c_005fif_005f0)|(org.apache.jsp.WEB_002dINF.<proprietary>_jsp._jspx_meth_c_005fif_005f0)
And here's the code for the relevant custom methods:
ThreadSafeRequestWrapper.getParameter:
public synchronized String getParameter(String name) {
return getRequest().getParameter(name);
}
MutableParametersHttpServletRequest.getParameter
public MutableParametersHttpServletRequest(HttpServletRequest request,
Map<String, String[]> parameters) {
super(request);
this.params = new HashMap<String, String[]>(parameters);
}
public String getParameter(String paramName) {
if (params.containsKey(paramName)) {
String[] values = params.get(paramName);
return (values != null) && (values.length > 0) ? values[0] : null;
} else {
return super.getParameter(paramName);
}
}
Both JSP tags:
<c:if test="${not empty param.renderedContent}" >
Both JSPs are called via several layers of <jsp:includes>, and one of the
upstream inclusions adds a <jsp:param> tag. These particular JSPs are the
first place the params are read after the <jsp:param> tag and are the unlucky
trigger of parameter reparsing (ApplicationHttpRequest.parseParameters).
So the perf opportunity comes from eliminating this part of the re-parsing
work.
That's a lot of text, sorry, I hope it answers your questions.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 69552] Performance regression in MessageBytes.toBytes() when using non-default charset
Posted by "bugzilla.apache.org via dev" <de...@tomcat.apache.org>.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69552
--- Comment #10 from abernal <ab...@apache.org> ---
Proposed fix: https://github.com/apache/tomcat/pull/1159
It skips the CharsetEncoder when the charset is UTF-8 and the value is ASCII.
Other cases keep the current path and its error handling. JMH on JDK 21 shows
105 -> 22 ns/op and 280 -> 64 B/op for an ASCII String.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org