It'd be better to not overuse logging, that will improve performance and readability. We'd take a closer look how we use logging, as it was mentioned on the last Devoxx - "logging should be threaded in the same way as UI for users" (more or less ;-) )
And keeping our own Logger is the best option, then we can always change underlaying implementation (slf4j, log4j, etc) Kind regards -- Łukasz + 48 606 323 122 http://www.lenart.org.pl/ Warszawa JUG conference - Confitura http://confitura.pl/ 2011/5/6 Rene Gielen <gie...@it-neering.net>: > See inline comments (initial mail wrongly addressed) > > On 05.05.11 07:02, Lukasz Lenart wrote: >> 2011/5/5 Martin Cooper <mart...@apache.org>: >>> On Wed, May 4, 2011 at 9:32 PM, Lukasz Lenart >>> <lukasz.len...@googlemail.com> wrote: >>>> Hi, >>>> >>>> I've removed the parts where concatenating was over constants - >>>> constant message plus constant as a string. I'm pretty sure that the >>>> modern JIT compilers would optimize that as well. And the code is more >>>> readable IMHO. >>>> Another thing, debug(), info(), etc checks if the given log level is >>>> set up or not and then performs logging (IO request, sending e-mail, >>>> etc). So, it will not affect performance as well (except if there is a >>>> bug ;-) ) >>> >>> Not true. In order to call the method in the first place, all of the >>> arguments must be evaluated. >>> >>> If you do this, the expensive function will _always_ be invoked, >>> whether 'info' level logging is enabled or not: >>> >>> log.info("And the answer is: " + doSomethingExpensive()); >> >> log.info("And the answer is: " + DO_SOMETHING_CONSTANT_STRING) will be >> optimized by JIT and that's why I've removed logging gates. There >> wasn't any call to a method and object concatenation. >> >>> However, if you guard it, like this, the expensive function is _only_ >>> evaluated when the guard is passed: >>> >>> if (log.isInfoEnabled()) { >>> log.info("And the answer is: " + doSomethingExpensive()); >>> } >>> >> >> Yes, I agree, but it wan't the case. >> >>> The usual argument I've seen for getting rid of the guards is that >>> most of the calls don't do anything expensive, so there's little >>> reason to guard them. However, it's far too easy for someone else to >>> come along later and modify the log message without thinking about it >>> perhaps now needing a guard, because it's now more expensive than it >>> used to be. Better, then, to always guard rather than kill performance >>> later by accident. >> >> That's why I prefer a code review like this and discussion over it >> instead of a common rule that we should blindly follow. Because rule >> don't teach how to be a good programmer, discussion do. And if someone >> did something that can impact performance, asking and explaining is >> far better, than saying we have a rule. Because quite often what was >> true for Java 1.4, isn't for 1.5 any more. >> > > I'm with you on readability. > > I would agree with you regarding reviews if we had some mandatory code > review mechanism in place, which is not the case. Actually I stumbled by > accident about this commit, and I doubt that each commit gets reviewed > either intensively enough or even at all. I've seen far to many > programmers not knowing about parameter evaluation costs, and while we > create some awareness here with this discussion, I'm sure it will be > mostly forgot0ten six months from now :) > > If a "not so aware" committer would come across those guards all over > the code base, I would tend to think that he'd either be lazy, by just > adding his additional code within an existing guarded log statement, or > ask in the dev list why he found all this crazy code that seems to be > intentional. > > Personally, if I had to chose between improved readability and improved > performance, I'd rather drop some readability... > > - René > > -- > René Gielen > IT-Neering.net > Saarstrasse 100, 52062 Aachen, Germany > Tel: +49-(0)241-4010770 > Fax: +49-(0)241-4010771 > Cel: +49-(0)163-2844164 > http://twitter.com/rgielen > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org > For additional commands, e-mail: dev-h...@struts.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org
