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

Reply via email to