geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dain Sundstrom <d...@coredevelopers.net>
Subject Re: [Logs] Re: Logging code and Garbage Collection
Date Fri, 15 Aug 2003 00:06:41 GMT
Man did you have me pissed off when I read this.  You almost got a 
flaming response, but I trashed it and you are getting this one...


Alex,

I boil the issue down as follows, excessive calls to isXEnable is bad 
because of the excessive overhead.

Agreed?



There are two solutions to this problem (notice both involve caching 
the value):

1) cache the value and invalidate based on a notification from the 
logging framework
2) cache the value and invalidate on an interval bases

Agreed?



Since as far as I recall, the log4j production release does not send 
notifications we can not do option 1.  Option 2 is easy and simple.  It 
took literally 20 minutes to write and test.

Now you seem to be arguing that this code will not be faster, and is a 
complete waste of effort and will only make geronimo harder to 
maintain.  If this is not your point please clarify.  If it is, can you 
tell me how this code would *ever* be slower unless the interval is too 
aggressive (currently 3 minutes).



BTW here is my justification.  isDebugEnabled looks like this:

public boolean isDebugEnabled() {
     if (repository.isDisabled(Level.DEBUG_INT)) {
         return false;
     }
     return Level.DEBUG.isGreaterOrEqual(this.getEffectiveLevel());
}

This says that if your log hierarchy has debug enabled at all then call 
getEffectiveLevel which has the following code:

public Level getEffectiveLevel() {
     for (Category c = this; c != null; c = c.parent) {
         if (c.level != null) {
             return c.level;
         }
     }
     return null; // If reached will cause an NullPointerException.
  }

If you take a look at level it is defined as follows:

protected volatile Level level;

According to the extensive testing Jeremy and I did on the fastest way 
to implement the locking system, the most expensive thing in raw java 
is volatile variables.  It was actually faster to synchronize and get 
the value, but that may reduce concurrency.  This was not a problem for 
locking as it is designed to control concurrency.  My code on the other 
hand, caches the value of this expensive call and keeps it in an 
non-synchronized non-volatile variable.  At noted in the code this is 
OK because boolean assignment is atomic and at the worst case we will 
have running threads with an outdated processor cache of the value, 
which is OK as this is designed to be an out of date cache.  So unless 
you turn off debug for the entire hierarchy there is no way my code can 
be slower and if you have turned it off for the entire hierarchy, my 
code should be no slower.


At this point, I am going to stop posting to this thread, because this 
type of detailed justification for such a small non-critical, zero 
impact commit eating up all of my development time.

-dain


Mime
View raw message