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-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
