[jbosscache-dev] Caching log.isTraceEnabled() as instance variable

[Manik Surtani]

On 8 Jul 2008, at 18:25, Galder Zamarreno wrote:

> Hi all,
>
> Yesterday while looking at the JBC code, I spotted that  
> CommandInterceptor caches log.isTraceEnabled() as instance variable  
> and that pretty much all classes extending this use it.
>
> In a standalone world, where JBC runs on its own this is fine.  
> However, when running within AS, log4j settings could change at  
> runtime (conf/jboss-log4j.xml is hot deployable) and therefore,  
> someone could enable TRACE logging for org.jboss.cache at runtime  
> which wouldn't have any effect with the current JBC code.

One would think that changing the log level all the way down to TRACE  
would not really happen on a live system so a restart would be ok.  We  
don't do this for any other log level, just TRACE.

> As far as I can remember, it's generally recommended not to cache  
> things like this in code running within AS. Then again, I can see  
> how convenient caching such value is rather than calling  
> log.isTraceEnabled() all the time. I doubt using an instance  
> variable or calling log.isTraceEnabled() would have a real impact  
> performance wise when TRACE is not enabled.

No, log.isTraceEnabled() can be pretty expensive.  :-)

> Thinking about this, I wonder whether any callback(s) could be  
> implemented when the log4j settings are updated after a hot  
> deployment and within this callback, update interceptors' trace  
> settings...

Wouldn't that tie JBC to log4j?  I currently just use commons-logging.


--
Manik Surtani
Lead, JBoss Cache
manik at jboss.org