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

[Galder Zamarreno]

Ok, that sounds fair enough.

Cheers,

Manik Surtani wrote:
> 
> 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
> 
> 
> 
> 
> 
> 

-- 
Galder Zamarreño
Sr. Software Maintenance Engineer
JBoss, a division of Red Hat