On Wed, Mar 18, 2009 at 1:27 PM, Mike McCune <[email protected]> wrote:
> Devan Goodwin wrote:
>>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On Wed, 18 Mar 2009 11:57:00 -0400
>> Justin Sherrill <[email protected]> wrote:
>>
>>> Might be a nice thing to do after the next fork for Sat.  Having to
>>> use log.isDebugEnabled() is ugly IMO and it'd be nice not to have to
>>> do it where we don't have to.
>>
>> I hate doing this too. I was gonna take issue with it as it's ugly and
>> seemed pretty silly until I googled and saw that it is best practice if
>> the statement is doing string contact.
>>
>
> This the rule I followed as well.  Wrap the log.debug() in the if statement
> only if it was doing string concatenation of variables in your method.
>
> I'd rather have lots of nice debug statements and a performance hit than
> people skipping adding logging to our classes because it looks ugly.
>
> I'd propose you only wrap if you are doing a:
>
> log.debug("something to debug: " + somevariable);
>
> otherwise wrapping this:
>
> if (log.isDebugEnabled()) {
>    log.debug("Some message");
> }
>
> is largely pointless.
>
>        StopWatch st = new StopWatch();
>        st.start();
>        for (int i = 0; i < 100000000; i++) {
>            log.debug("Some line of debug");
>        }
>        st.stop();
>        System.out.println("Time: " + st.getTime());
>
> output is ~500ms for a hundred million executions.  And this:
>
>        StopWatch st = new StopWatch();
>        st.start();
>        for (int i = 0; i < 100000000; i++) {
>            if (log.isDebugEnabled()) {
>                log.debug("Some line of debug");
>            }
>        }
>        st.stop();
>        System.out.println("Time: " + st);
>
> is actually 600ms! The if statement actually costs more than the simple call
> to log.debug()
>
> Concatenation does cost ya ...
>
>        StopWatch st = new StopWatch();
>        String rand = TestUtils.randomString();
>        st.start();
>        for (int i = 0; i < 100000000; i++) {
>            log.debug("Some line of debug" + rand);
>        }
>        st.stop();
>
> is actually around 12 seconds.  Not horrible but could be bad if the strings
> are large.
>

Ok I can live with the only wrap them if your doing calculations i.e.
concats, etc.

I wrap them all simply because it's easier for me to remember, but if we can be
consistent that's better.

jesus

_______________________________________________
Spacewalk-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Reply via email to