I agree, it is a best practice and we should follow it. Can you work
out a patch & open an issue? I assume this means "if (infoStream !=
null)..." in this case.
Mike
Shai Erera wrote:
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
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
