On 6/22/2010 2:34 PM, Alan Bateman wrote:
Daniel D. Daugherty wrote:
:
Here is the URL for the webrev:
http://cr.openjdk.java.net/~dcubed/6942989-webrev/1/
Thanks, in advance, for any reviews.
Sorry for the late reply. Using the reference queue is much better.
I've looked through the new webrev and the approach seems reasonable.
Thanks!
One nit is that the style is perhaps a bit different to the existing
code. For example, the alignment of the declarations in LoggerWeakRef
is different to the enclosing class and the other classes in this
package, the new package-private methods are final whereas the
existing package-private methods aren't. Nothing wrong, just looks a
bit different.
I got the idea for the "final" stuff from Tony's paper. I also
don't think these are things that subclasses need to override.
One question, in drainLoggerRefQueueBounded I'm curious about the
check for loggerRefQueue being null. Is that needed?
This check:
522 if (loggerRefQueue == null) {
523 // haven't finished loading LogManager yet
524 break;
525 }
Yup. Without that check we crash during LogManager.<clinit> if I
remember the name correctly. The whole LogManager/Logger relationship
relationship is fraught with such warts...
Thanks for the review!
Dan