[OpenJDK 2D-Dev] Review Request for 6879044

[Andrew John Hughes]

2009/9/14 Mandy Chung <Mandy.Chung at sun.com>:
> 6879044: Eliminate the dependency of logging from the JRE core/awt/swing
> classes
>
> Webrev:
>  http://cr.openjdk.java.net/~mchung/6879044/webrev.00/
>
> Summary:
> 1. A new sun.util.logging.PlatformLogger class that will handle the log
> messages in a similar way as Logger but it will only delegate to
> java.util.logging only when it is enabled.  LogManager and LogRecord are
> modified to support the platform loggers.  The users of PlatformLogger will
> continue to run if java.util.logging classes do not exist.
>
> 2. AWT, 2D, Swing, and a few java.util classes are modified to use
> PlatformLogger instead of Logger.  Although many files are modified, the
> change is mostly replacement with classname.
>
> AWT statically creates a number of loggers. Running a simple AWT Framer
> application with JDK 7 b71 creates 79 loggers on solaris-i586 and 34 loggers
> on windows-i586. SwingSet2 creates a total of 85 loggers including a few
> non-awt ones on solaris-i586 and 35 on windows-i586).
> Although the memory usage might not be very high (especially with this fix),
> I don't see the need of having many fine-grained loggers.  This fix doesn't
> address this the number of AWT loggers. I file a separate CR (6880089) to
> revisit it.
>
> Startup Performance:
> This change does not have significant startup performance improvement, as
> expected.  However, it does reduce the number of loaded classes (Framer app
> loads 16 fewer classes and jedit loads 13 fewer classes).
>
>
> Thanks
> Mandy
>
>
>

I'm curious as to why some of the initialisations are lazy and some
are eager.  Notably AWTEvent is changed from eager to lazy:

 public abstract class AWTEvent extends EventObject {
-    private static final Logger log = Logger.getLogger("java.awt.AWTEvent");
+    private static volatile PlatformLogger log;
+
...
+
+    // log fine message to the java.awt.AWTEvent logger
+    private static void fine(String msg, Throwable t) {
+        if (log == null) {
+            log = PlatformLogger.getLogger("java.awt.AWTEvent");
+        }
+        if (log.isLoggable(PlatformLogger.FINE)) {
+            log.fine(msg, t);
+        }
+    }
+


Although the use of volatile should ensure the correct ordering of
operations, there is still the possibility of a minor race here as the
if condition and its body are not atomic; thus there could be multiple
calls to PlatformLogger.getLogger should a thread be interrupted
between the check and the assignment.

Why not just use a straightforward static assignment to a final
variable as before?  Is it really that unlikely that fine() will be
called that we need not initialise this early?.  At the very least
consider using the lazy holder idiom over volatile.
-- 
Andrew :-)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net

PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8