[infinispan-dev] Logger lookup performance

[Sanne Grinovero]

2011/8/8 David M. Lloyd <david.lloyd at redhat.com>:
> On 08/08/2011 05:06 AM, Sanne Grinovero wrote:
>> Hi all,
>> raising some attention here on Pete's suggestions;
>> over the weekend we fixed this bootstrap performance issue ISPN-1315,
>> which I had given the name "Reduce number of Logger instances being
>> created",
>> after looking at this profiler screenshot:
>> http://community.jboss.org/servlet/JiveServlet/showImage/2-619820-16841/hotspot.jpg
>>
>> Now Pete suggested that the JBoss Logging should return the same
>> instance; TBH from the screenshot all we know is that
>> LogFactory.getLog(Class) invocations are taking 280 seconds, so I
>> guess they are actually returning always the same instance, but and
>> likely the time is spent in finding it. I'll rename the issue.
>
> JBoss Logging definitely *tries* to return the same logger but there's
> no guarantee that it can (depending on the log backend).  In any case
> though, it's definitely better to use static final loggers as you say;
> this advice applies to every log framework I know of, not just JBoss
> Logging.
>
> If you _really_ want covariant loggers (which generally I disapprove of,
> btw) then use a protected method which returns a static final logger.
> Note that most of the time the reason people use covariant loggers is so
> that the category name can reflect the instance class name.  However, I
> recommend against this for various reasons.  If the user wants the
> logging class name they can add it to the format.  If the user wants the
> instance class name, it should be part of the log message.  The logger
> category should be named to reflect the task being performed, not a
> class name.
>
>> IMO we should stick with "static final" loggers, unless there are
>> special reasons (and then please add a comment to clarify it was an
>> intentional choice).
>
> Right on.

thanks for you comments David.

I've created https://issues.jboss.org/browse/ISPN-1321 to track the
remaining performance issue,
and suggested the user to try proposing a fix himself, since he has a
good test it should be easier for him to verify if any patch fixes the
issue.

Sanne