lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McCandless <luc...@mikemccandless.com>
Subject Re: Unnecessary messages creation by LogMergePolicy
Date Fri, 05 Dec 2008 13:35:25 GMT

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


Mime
View raw message