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
