-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Added isXXXEnabled for logging statements #3814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added isXXXEnabled for logging statements #3814
Conversation
Guarded the logging statements which do concatenation of strings or calling toString on objects. When a log level isn't enabled this still would produce garbage that would need to be collected. Guarded all logging up to info, warn and error can be assumed to be enabled on a production system.
if (logger.isDebugEnabled()) { | ||
logger.debug(resultingQuery); | ||
} | ||
logger.debug(resultingQuery); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems an unintentional change (as it goes against the idea of the PR). I will revert it on merge.
@@ -61,6 +61,8 @@ public void write(List<? extends Object> items) throws Exception { | |||
} | |||
|
|||
stepExecution.setWriteCount(++count); | |||
LOG.info("Executing infinite loop, at count=" + count); | |||
if (logger.isInfoEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be if (LOG.isInfoEnabled()) {
. I will fix it on merge.
public void doStronglyTypedLogging(Object item){ | ||
log.info("Processed: " + item); | ||
public void doStronglyTypedLogging(Object item) { | ||
if (logger.isInfoEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be if (log.isInfoEnabled()) {
. I will fix it on merge.
StringBuffer valueBuffer = new StringBuffer(); | ||
StringBuilder valueBuffer = new StringBuilder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change creates a compilation error in jobParameterMatcher.appendReplacement
and jobParameterMatcher.appendTail
. Those methods accept a StringBuilder
starting from Java 9 onward. I will revert that change on merge. Please make sure to use Java 8 when building your PR.
Rebased and merged as b7d144c. Thank you for your contribution! |
Guarded the logging statements which do concatenation of strings
or calling toString on objects. When a log level isn't enabled
this still would produce garbage that would need to be collected.
Guarded all logging up to info, warn and error can be assumed to be
enabled on a production system.