Peter,
I am familiar with the double-checked lock pattern. It is typically
written as:
if ( thing != null) {
synchronized( this ) {
if ( thing == null ) {
thing = new Thing();
}
}
But that's not what I've written below.
1. The first check is to see if _logger is NOT null. If it's not
null, then it exists. It is (crucially) declared as 'volatile', so
all threads can see its current value.
2. Synchronized code cannot be moved up above the check for non-null
in step 1. That's my understanding from "Java Concurrency" at least.
3. The synchronized block synchronizes *before* it makes the check
for null.
So why won't the code snippet work?
Lloyd
On Dec 7, 2006, at 11:47 AM, Peter Williams wrote:
> Lloyd L Chambers wrote:
>> Please see:
>>
>> https://glassfish.dev.java.net/issues/post_bug.cgi
>>
>> ------------
>> Initializing all the Loggers at startup
>> [ServerLogManager.reInitializeServerLoggers] takes about half a
>> second even on a very fast machine.
>>
>> But observe that many Loggers are never used unless an exception
>> needs to be reported. The typical
>> idiom is: try { ... } catch( ... ) { _logger.log(...); }.
>>
>> Thus, a cost is paid even if the logger is never used.
>> Additionally, Loggers cannot be initialized at
>> startup because the ServerContext is not yet available. This
>> causes wasted cycles and leads to a
>> transient half-initialized state which much be protected against
>> internally.
>>
>> We might be see some performance improvements by deferring the
>> initialization of Loggers. This would
>> need to be done in a thread-safe manner. I think the following
>> code snippet is thread-safe and would
>> remain efficient, especially if applied only to cases where the
>> Logger is needed only for exceptions:
>>
> This is the double checked locking pattern and is NOT thread safe.
> See http://www.cs.umd.edu/~pugh/java/memoryModel/
> DoubleCheckedLocking.html for an exhaustive academic analysis if
> interested.
>
> -Peter
>>
>> private static Logger
>> getLogger() {
>> // if it's non-null, then it's safe to return it
>> if ( _logger != null ) {
>> return _logger;
>> }
>>
>> // null, synchronize before getting the logger
>> synchronized( StringManagerBase.class ) {
>> if ( _logger == null ) {
>> _logger=LogDomains.getLogger(LogDomains.UTIL_LOGGER)
>> }
>> }
>> return _logger;
>> }
>>
>> This could be captured in a class for convenience:
>>
>>
>> static private final class LoggerGetter {
>> private final String mLoggerName;
>> private volatile Logger mLogger;
>>
>> public LoggerGetter( final String loggerName ) {
>> mLoggerName = loggerName;
>> mLogger = null;
>> }
>> public Logger
>> getLogger() {
>> // if it's non-null, then it's safe to return it
>> if ( mLogger != null ) {
>> return mLogger;
>> }
>> else synchronized( this ) {
>> if ( mLogger == null ) {
>> mLogger=LogDomains.getLogger
>> (LogDomains.UTIL_LOGGER)
>> }
>> }
>> return mLogger;
>> }
>> }
>>
>> and used like this:
>> static private final LoggerGetter _loggerGetter = new LoggerGetter
>> ( "MyLogger" );
>> static private Logger _getLogger() { return _loggerGetter.getLogger
>> (); }
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>