Hey Curt, these are good points.

Actually, we do have guards on the legacy code for calling
OurLogger.log(...). I just didn't include it in the sample.

With respect to the log level mapping, a HashMap would be used since the
mapping is static and the map would be read-only.  So a OurLevel log
level would be mapped to a Log4j Level within a statically defined map.
Upon a log request, we would lookup the Log4j Level and perform the
appropriate logging operation.

At the moment, we are collecting performance data, but in a simulated
environment.  The data is one aspect of this refactoring. The
scalability, maintainability and usage paradigm of utilizing Log4j
Loggers are the reasons I mailed the Log4j user group.

The overall recommendation appears to be to use LogManager.getLogger()
versus keeping the Logger locally in a Concurrent HashMap.  The local
concurrent hashmap won't provide much of an improvement.  

Thanks for taking time to look at this issue.

-Faheem

-----Original Message-----
From: Curt Arnold [mailto:[EMAIL PROTECTED] 
Sent: Friday, March 09, 2007 1:05 PM
To: Log4J Users List
Subject: Re: Using LogManager.getLogger(String)



On Mar 9, 2007, at 9:03 AM, Zakaria, Faheem wrote:

> I would like to clarify the first paragraph further.
>
> Legacy Code: (cannot be modified)
> ...
> OurLogger.log(new LogObject("com.myclass", level.DEBUG, "Log Message
> 1"));
>
>       OurLogger Class: (can be modified)
>               public static void log(LogObject logObject) {
>                       // refactor to use concurrent hashmap to store
> and lookup Log4j Loggers
>                       // log the request through Log4j Logger
>               }
>
> Hope this helps.
>
> Faheem


 From that code snippet, it does not appear that you were critically  
interested in performance when you wrote the original code as each  
OurLogger.log() statement will result in the construction and  
destruction of a LogObject even if the threshold for that class or  
package is higher.  That makes me thing that your logging statements  
must be infrequently executed and trying to optimize OurLogger.log()  
may not result in an observable benefit.  Calling LogManager.getLogger 
() repeatedly should not add all that much more to the overhead that  
you are currently incurring.

To implement OurLogger.log you have to by some mechanism lookup the  
Logger and Level corresponding to the LogObject passed to you.  You  
have not mentioned anything about how you attempt to map the level  
parameter to a log4j Level.

I would run benchmarks or collect profiler data before trying to  
write anything more than a straightforward implementation of  
LogObject.info using LogManager.getLogger.  LogManager.getLogger()  
uses a JDK 1.0 Hashtable to store its map of loggers.  Obviously  
there are more efficient ways to do that introduced in later JDK's.   
Duplicating the Hashmap backing LogManager.getLogger() with your own  
map at code may not provide any noticeable performance improvement  
and would result in more code for bugs to lurk in and for you to  
maintain.

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to