[
https://issues.apache.org/jira/browse/HADOOP-6884?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12899127#action_12899127
]
Luke Lu commented on HADOOP-6884:
---------------------------------
We can save ~296 lines from the patch if we exclude the test code:
{code}
[...hadoop]$ find */src/java -name \*.java | xargs grep '\.debug(' | wc -l
410
[...hadoop]$ find */src/test -name \*.java | xargs grep '\.debug(' | wc -l
148
{code}
Also #3 seems to be fairly simple with a single "around" advice in aspectj and
a couple of lines for ajc target in build.xml. Why use an ad-hoc script that's
known (according Erik himself) to have false positive that require manual
inspection (which is also not part of the version controlled source tree), when
you we have a built-in (aspectj*.jars are already included as build/runtime
dependencies) tool specifically for such tasks?
> 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: 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.
