[infinispan-dev] Logger lookup performance

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[infinispan-dev] Logger lookup performance

Sanne Grinovero-3
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.

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).

To focus on this performance issue from the forums [1], Chris Meunier
reports that with this last fix the boot performance is back to 10
seconds from the 3 minutes it was taking, so much better, but are
still 10 seconds looks like a lot, as they are spent in the components
registry - not in network operations.
I'm creating another JIRA but won't be able to work on it soon - have
to focus on Hibernate - could anybody take this over from me? I
wouldn't postpone it, so that Chris on the forums can keep helping us
with it. Please ask him for more profiler details if needed:

1 - http://community.jboss.org/thread/170415?tstart=0

Cheers,
Sanne
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Logger lookup performance

David M. Lloyd
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.

--
- DML
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Logger lookup performance

Sanne Grinovero-3
2011/8/8 David M. Lloyd <[hidden email]>:

> 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

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev