I know this is a tad early to comment on the code in CVS, but I've just noticed code like this (in org.apache.geronimo.deployment.scanner.DeploymentScanner)
log.debug("Finished scan of " + url + ", " + result.size() + " deployment(s) found");
Could everyone make sure that log statements are wrapped (eg.):
if (log.isDebugEnabled()) {
log.debug("Finished scan of " + url + ", " + result.size() + " deployment(s) found");
}
The reason being that while profiling a J2EE application a while back we found that well over 50% of the processors' time was spent executing log statements just like the above - most of that with the VM messing around creating StringBuffer objects and then garbage collecting them. In the case above, this will happen regardless of the logging level in use i.e. *even if* debug messages aren't actually being output.
I agree completely. In fact, I've also been party to one such application that used this style of logging so badly that their application crawled. The problem was, when they put the conditional log statements in the application ran so fast that it showed up all their syncronization holes in the multi-threading parts of the code.
They put the log stuff back so that it ran slower to avoid debugging the multi-threaded spaghetti. :-)
I know this sounds incredibly picky at this stage but I think it'd be an idea to get into good habits now rather than later. If others agree with the above, I'll add it to the coding standards in the Wiki.
I'd say add it to the coding standards. Many people may not even be aware that there is such an issue, and having things overspecified in the coding standards is more likely to produce better results than those which are underspecified.
Alex.
