On 27/04/2015 10:21 PM, David Holmes wrote:
On 27/04/2015 5:28 PM, Daniel Fuchs wrote:
Hi David,
On 4/27/15 3:21 AM, David Holmes wrote:
Hi Daniel,
On 25/04/2015 3:07 AM, Daniel Fuchs wrote:
Hi,
Please find below a patch that tries to improve the locking
strategy in LogManager.
The patch proposes to use a Reantrant lock to deal with
configurations changes in reset() and readConfiguration(),
and avoids lock contention in initializeGlobalHandlers()
http://cr.openjdk.java.net/~dfuchs/webrev_8077846/webrev.00/
How early in VM startup can logging be initialized and used? I just
wonder whether the use of ReentrantLock changes that at all?
The initialization will not happen before the first class calls
Logger.getLogger() or LogManager.getLogManager().
In particular, it does not happen when the LogManager.class is loaded
(that is: it no longer happens as part of the class static
initialization).
I see Alan replied to that question as well...
I'm just wondering what anyone might be running with a customized core
class that uses logging, and which may now fail due to the different
requirements.
But then I just recalled that we used to use ReentrantLock via
ConcurrentHashMap in Class, so introducing it early is not likely to
cause any issue.
Cheers,
David
Took me a while to understand the use of initializeGlobalHandlersDone.
I think this comment:
1622 // If we are in the process of initializing global
handlers we
1623 // also need to lock & wait (this case is indicated by
1624 // initializeGlobalHandlersDone == false).
would be a little clearer as:
1622 // If we are in the process of initializing global
handlers we
1623 // also need to acquire the lock to wait until it is
complete
1624 // (this case is indicated by
initializeGlobalHandlersDone == false).
Exactly. Thanks!
But why are these initialized to true and not false ??
184 private volatile boolean initializeGlobalHandlersCalled = true;
185 private volatile boolean initializeGlobalHandlersDone = true;
It doesn't make sense to try & create global handlers before the
configuration is read for the first time. In addition the first thing
that readConfiguration does is to call reset() which switches
initializeGlobalHandlersCalled to true to avoid that it gets initialized
before the Properties is later loaded.
I agree that this initialization to 'true' is not intuitive.
My fingers had been itching several times to change it to false ;-)
but the 'true' value makes more sense once you look at the
initialization code path.
I should probably add a comment:
// These variables are initialized to true, to avoid trying
// to initialize global handlers before the configuration is read
// for the first time.
Sounds good to me.
Thanks,
David
I think this code/comment:
1629 if (initializeGlobalHandlersCalled) return; //
recursive call
would also be clearer as:
1629 if (initializeGlobalHandlersCalled)
return; // recursive call or another thread did
it already
Thanks David!
best regards,
-- daniel
Thanks,
David
comments welcome,
best regards,
-- daniel
