[GitHub] [hadoop] virajjasani commented on pull request #4480: HDFS-16638. Add isDebugEnabled check for debug blockLogs in BlockManager

[GitBox <gi...@apache.org>]

virajjasani commented on PR #4480:
URL: https://github.com/apache/hadoop/pull/4480#issuecomment-1166705882

   I am not sure if having `isLogEnabled()` would help with much perf improvement so long as we use arguments `{}` to let the logger library take care of the final String concat.
   
   Looking at `slf4j-api` and `log4j-slf4j-impl`, Log4jLogger has this implementation:
   
   ```
       @Override
       public void debug(final String format, final Object... args) {
           logger.logIfEnabled(FQCN, Level.DEBUG, null, format, args);
       }
   ```
   which would call `log4j-api` implementation of `AbstractLogger`:
   
   ```
       @Override
       public void logIfEnabled(final String fqcn, final Level level, final Marker marker, final String message,
               final Object... params) {
           if (isEnabled(level, marker, message, params)) {
               logMessage(fqcn, level, marker, message, params);
           }
       }
   ```
   
   Hence, as long as we use parameterized args `{}`, the final log concat operation happens only if the logger level (DEBUG in our case) is enabled.
   The best practice to use `isDebugEnabled` or `isTraceEnabled` is when we don't use parameterized args `{}` and instead use full string concat all by ourselves, in which case string cancat with big values happen regardless of whether log level is enabled, but we are already using params for these loggers, hence I don't think we would gain much here.
   Thoughts?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org