2006-05-19
Contented Monitor on a T2000
I've ranted about it before, the Jakarta JSP EL-Interpreter up to Tomcat 5.5 / JSTL 1.1 does not parse EL expressions at page compile time but goes through a parser cache on each evaluation in ELEvaluator.java:
static Map sCachedExpressionStrings = Collections.synchronizedMap (new HashMap ());
We've been doing some performance testing on a Sun T2000 machine and this showed to be a bottleneck. A typical thread dump would have many threads waiting for monitor entry on this parser cache here:
// See if it's in the cache Object ret = mBypassCache ? null : sCachedExpressionStrings.get (pExpressionString); if (ret == null) { // Parse the expression Reader r = new StringReader (pExpressionString); ELParser parser = new ELParser (r); try { ret = parser.ExpressionString (); sCachedExpressionStrings.put (pExpressionString, ret); }
There's a simple trick I've used on several occasions to reduce contention on this monitor. I made the parser cache read-only; any thread that wants to add to the cache must allocate a new copy and substitute it for the old version. Since we're talking about EL expressions here that you'll only have a few hundred of in your application(s), the process will quickly converge. Reading the cache now only costs an additional memory barrier for reading the volatile and is fully concurrent otherwise. In this particular case it boosted the performance by 40% from 400 to 550 page impressions / s.
static volatile Map sCachedExpressionStrings = new HashMap (); ... // copy on write Map updatedCache = new HashMap(sCachedExpressionStrings); updatedCache.put(pExpressionString, ret); sCachedExpressionStrings = updatedCache;
Note that there's a race, if two threads both want to update the cache at once, one might lose. In this
scenario it doesn't really matter, though, and the cache will quickly be complete. Also note, that this
code is only really legal under the revised Java Memory Model in Java 5. Up to 1.4.2, writing the reference of the new map to the volatile variable
'sCachedExpressionStrings' would not garantuee visibility of the update cache's contents; actually anything
could happen. I seem to recall though, that on Sun JVM 1.4.2 this code will actually be correct, too.
Update: it just dawned on me that I could combine copy-on-write with a traditional monitor-protected
field instead of the volatile. This would still be a much shorter critical section than the synchronized
map AND entirely legal even according to the 1.4 memory model. Something that might have a chance to
be accepted as a patch iff the gains are high enough.
I expect more of this kind of problems the more cores we see in servers. Luckily guys like Doug Lea and Bill Pugh make sure we can write code that avoids them. Can't wait for my copy of this ...