A high-volume, latency-sensitive application shows a large amount of String.equals() calls buried inside tomcat processing. Detailed examination shows that much of it comes from ApplicationHttpRequest.getSpecial() and removeSpecial(), which are called once per call to request.setAttribute() or request.removeAttribute(). In our application these calls are from certain Filters or, more commonly, AstExpressions, especially those which are not otherwise resolved. The relevant code is: protected static final String specials[] = { ... }; protected int getSpecial(String name) { for (int i = 0; i < specials.length; i++) { if (specials[i].equals(name)) { return i; } } return -1; } The array contains 12 specials, so each call to request.getAttribute(), etc. performs 12 String comparisons. The occasional search of an actual special will reduce that, but very few of our calls match. Suggestion: use a HashMap<String, Integer> or enums to achieve a similar purpose. Comparable performance data may be found at https://bz.apache.org/bugzilla/show_bug.cgi?id=67080. These scans are largely inlined into calling methods so I cannot reliably estimate the cpu or clock impact, however the sheer number of calls is impressive.
> These scans are largely inlined into calling methods so I cannot reliably > estimate the cpu or clock impact, however the sheer number of calls is > impressive. I understand the concern, but it sounds doubtful this is actually a problem given the nature of the checks. Please provide actual data, also demonstrating the proposed technique would provide any performance benefits.
A crude performance test shows that using a map makes the request attribute lookup about 2.5x faster. I should have a patch for this once I have got my head around how AttributeNamesEnumerator works.
Good idea to measure ApplicationHttpRequest.getAttribute()! Our production environment shows 0.27% cpu spent in that method, of which 0.1% cpu is spent in method-local time (including the inlined array scan). ApplicationHttpRequest.setAttribute and removeAttribute() are also present but 10x smaller.
Fixed in: - 11.0.x for 11.0.0-M14 onwards - 10.1.x for 10.1.16 onwards - 9.0.x for 9.0.83 onwards - 8.5.x for 8.5.96 onwards
For posterity: this issue also dominates the runtime of org.apache.catalina.core.ApplicationHttpRequest$AttributeNamesEnumerator.findNext().
Production data confirms the behavior change but shows minimal actual performance improvement - it appears the extra work of hashing the string and doing the HashMap lookup is comparable to the wasted effort of 12 string comparisons. I refined the approach by checking whether the attribute name was shorter than the shortest special (length 30) and that eliminates the search cost for the vast majority of calls. The attached speed test compares the cost of searching for attributes with long vs. short names, and the current implementation of getAttribute() is indeed slightly faster for short names (520ms vs. 540ms on my PC). With the length check, the short names are orders of magnitudes faster. Presumably the JIT optimized it close to a no-op. Checking for length 30 is an easy modification, but not the most maintainable. Perhaps modify the static initializer to calculate the length of the shortest special?
Created attachment 39574 [details] Speed test
Created attachment 39575 [details] Support class for the speed test
I did look at using !startsWith("jakarta") but that is ~2x slower than a length check. I've coded up the length check and am just running some tests locally.
Fixed in: - 11.0.x for 11.0.0-M17 onwards - 10.1.x for 10.1.19 onwards - 9.0.x for 9.0.86 onwards - 8.5.x for 8.5.99 onwards
(In reply to Mark Thomas from comment #9) > I did look at using !startsWith("jakarta") but that is ~2x slower than a > length check. > > I've coded up the length check and am just running some tests locally. What about charAt(0) == 'j'? I suppose that's the second thing String.equals() will do after checking the length, so maybe it won't help at all.
String compaction made charAt() more expensive, thanks to checks on format and range. String.length() remains an integer check. That said, charAt() isn't that bad, so it's an option if functionally required.
This changed reached prod and improved performance. Low impact.