[ 
https://issues.apache.org/jira/browse/HADOOP-6884?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12903524#action_12903524
 ] 

Erik Steffl commented on HADOOP-6884:
-------------------------------------

Doug, I don't see your point - the patch only makes the code consistent, as I 
mentioned previously lot of debug() calls are already guarded by 
isDebugEnabled().

If I started a new practice or tried to bring in new coding style/practice I 
would understand your point. But defending inconsistent code?

Patch might not be nice to look at but:

  - it's best from performance viewpoint

  - even though it adds code still LESS code is executed

  - code is less readable on small scale (individual statement is longer) but 
more readable on big scale - no surprises (all debug messages are same), easy 
to maintain (easy to find debug messages that are not guarded thus potential 
performance problems), it's obvious that debug message is optimal (don't need 
to investigate whether it's on critical path, whether it's good enough - in 
some cases the arguments themselves might be expensive, i.e. it's a function 
call or a new object is created)

  - it's relatively standard practice (i.e. java language doesn't offer much 
better solution, as seen by this long discussion)

> Add LOG.isDebugEnabled() guard for each LOG.debug("...")
> --------------------------------------------------------
>
>                 Key: HADOOP-6884
>                 URL: https://issues.apache.org/jira/browse/HADOOP-6884
>             Project: Hadoop Common
>          Issue Type: Improvement
>    Affects Versions: 0.22.0
>            Reporter: Erik Steffl
>            Assignee: Erik Steffl
>             Fix For: 0.22.0
>
>         Attachments: FunAgain.java, FunAgain.java, HADOOP-6884-0.22-1.patch, 
> HADOOP-6884-0.22.patch
>
>
> Each LOG.debug("...") should be executed only if LOG.isDebugEnabled() is 
> true, in some cases it's expensive to construct the string that is being 
> printed to log. It's much easier to always use LOG.isDebugEnabled() because 
> it's easier to check (rather than in each case reason whether it's necessary 
> or not).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to