I like the method, but how about the name verbose(), ie:
if (verbose())
...
Mike
Shai Erera wrote:
I'll open an issue and work out a patch.
Basically it means infoStream != null, although in LogMergePolicy I
might add a specific method for that, because the messages are
output if the IndexWriter member is not null and its infoStream is
not null (this check is done by IndexWriter).
Therefore I think I'll add a method to IndexWriter messagesEnabled()
which returns true if the infoStream is not null for use by other
classes (rather than the implicit iw.getInfoStream() != null). BTW,
getInfoStream() is not called by any class in Lucene, except one
test class.
What do you think about adding this method, and its name?
On Fri, Dec 5, 2008 at 3:35 PM, Michael McCandless <[EMAIL PROTECTED]
> wrote:
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
