https://bz.apache.org/bugzilla/show_bug.cgi?id=69285
Bug ID: 69285
Summary: Performance improvement to
ApplicationHttpRequest.parseParameters()
Product: Tomcat 9
Version: 9.0.x
Hardware: All
OS: All
Status: NEW
Severity: enhancement
Priority: P2
Component: Catalina
Assignee: dev@tomcat.apache.org
Reporter: jeng...@amazon.com
Target Milestone: -----
Created attachment 39847
--> https://bz.apache.org/bugzilla/attachment.cgi?id=39847&action=edit
Reproducer & speed test
ApplicationHttpRequest.parseParameters() is called whenever a JSP EL expression
references a param, and the cached parameters are outdated. The runtime scales
by (among other things) the number of parameters copied. JSPs that use
<jsp:param> cause headaches because they force re-parsing of the parameters,
and the number of parameters increase.
This method is a bottleneck in a high-traffic, latency-critical application,
estimated at around 0.3%. The impact is focused on a section of the
application that nests many JSP calls, each with a <jsp:param>. In addition to
application modifications (to use request attributes instead of parameters),
it's possible to accelerate Tomcat's implementation.
Specifically, in 9.x, parseParameters() includes these two lines:
parameters = new ParameterMap<>();
parameters.putAll(getRequest().getParameterMap());
Under-the-covers the call to parameters.putAll() reads the
java.util.Collections$UnmodifiableMap$UnmodifiableEntrySet associated with the
ParameterMap's field unmodifiableDelegatedMap. putAll() iterates across that
entrySet() and in so doing triggers extra instructions, possibly vtable/itable
lookups (depending on context), and extra reads as an additional pointer must
be chased. It's intuitive to think that the JVM can optimize this away, but in
practice HashMap.putAll() encounters too many distinct implementations of
Map.entrySet() to remove the virtual method lookups.
Solution: create a new constructor for ParameterMap that receives a
ParameterMap and reads from the unlocked (modifiable) map, thus bypassing all
code related to CollectionsRUnmodifiableMap. Safety is not compromised since
the modifiable field remains private, and we clearly are not modifying it here.
Example:
public ParameterMap(ParameterMap<K,V> map) {
delegatedMap = new LinkedHashMap<>(map.delegatedMap);
unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap);
}
Note that this is nearly identical to the existing Map-based constructor:
public ParameterMap(Map<K,V> map) {
delegatedMap = new LinkedHashMap<>(map);
unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap);
}
A performance test is attached; my Graviton-based computer demonstrates a
20-25% time reduction, near-constant across the number of parameters.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
