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.

Mike

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

Reply via email to