[SMALLFIX] Avoid new string creation in debug log #15671
Conversation
Replace `String.format` calls with slf4j's original formatting ability
| LOG.debug(String.format("Open path %s with flag 0x%x for overwriting. " | ||
| + "Alluxio deleted the old file and created a new file for writing", path, flags)); | ||
| LOG.debug("Open path {} with flag {} for overwriting. " | ||
| + "Alluxio deleted the old file and created a new file for writing", path, flags); |
There was a problem hiding this comment.
Now I see why the original author chose String.format instead of LOG.debug("{}"). The slf4j placeholder does not support formatting integers as hexadecimals. I think what you can do is surround this debug log with a check:
if (LOG.isDebugEnabled()) {
LOG.debug(String.format(...));
}There was a problem hiding this comment.
I see. Submitted another patch that does this.
| LOG.error(String.format("Failed to open %s: Not supported open flag 0x%x. " | ||
| LOG.error("Failed to open {}: Not supported open flag {}. " |
There was a problem hiding this comment.
These are okay since error level logging is almost always enabled, so the strings need to be created anyway.
There was a problem hiding this comment.
Got it. Error level logs are left untouched now.
|
alluxio-bot, merge this please |
|
Merge unsuccessful: |
|
never mind, the change is not relevant any more. |
dbw9580
changed the title
|
Closing as the code has been refactored away. |
What changes are proposed in this pull request?
Check the logging level before creating a new string.
Why are the changes needed?
Fixes Alluxio/new-contributor-tasks#640
Does this PR introduce any user facing changes?
No