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 <lucene@mikemccandless.com
> > 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: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-help@lucene.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
|