lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McCandless <>
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.


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:
For additional commands, e-mail:

View raw message