Hi
As I looked at the code in LogMergePolicy (and its sub-classes), I came
across such lines:
message("findMergesToExpungeDeletes: " + numSegments + " segments");
Those lines print to the info stream (eventually) if it's not null.
If one follows Java logging best practices, then any logging message should
look similar to this:
if (logger.isLoggable(Level)) {
logger.log(Level, msg);
}
The reason is that when logging messages, one usually does not pay attention
to any performance issues, like String concatenation. Therefore, checking if
logging is enabled saves building the String just to discover later that
logging is not enabled.
I haven't checked other places in the code, because I'd like to get the
committers opinion on this first. Imo, those strings are created
unnecessarily (the above message creates 5 strings) since most of the time
the info stream is null.
I can provide a patch to fix it by first checking if logging (or whatever
other name you'd like to give it) is enabled before attempting to output any
message. The LogMergePolicy classes are one example that I've run at, but
I'm sure there are other places in the code.
I don't foresee any great performance improvements by this fix, except just
following best practices.
What do you think?
Shai
