<AWT Dev> Review Request for 6879044

[mandy.chung]

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

[Oleg Sukhodolsky-4]

On Mon, Sep 14, 2009 at 5:32 AM, Mandy Chung <Mandy.Chung@...> wrote:

> 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.

evaluation for 6879044 says:

Add a PlatformLogger class that mimics the default logging behavior
(output the log message to System.err with simple formatting) and
creates a java.util.logging.Logger only when the logging facility is
used by the application or a user-defined configuration is supplied.

which of two descriptions is correct one?

Also I wonder if it is ok to get possible better modularization by
adding dependency to Sun's internal API into public packages.

> 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.

Well, at least in AWTEvent you have changed static initialization of
logger to lazy one.
Perhaps it is better to keep a static one to minimize your changes.

Oleg.

>
> 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
>
>
>

[Alan Bateman]

Mandy Chung wrote:
> 6879044: Eliminate the dependency of logging from the JRE
> core/awt/swing classes
>
> Webrev:
>   http://cr.openjdk.java.net/~mchung/6879044/webrev.00/
The approach seems reasonable to me. It is a bit strange to redirect the
platform logging to j.u.logging if something up the stack starts logging
but I think we can live with this because these loggers are for
debugging purposes.

The changes to the AWT code are mostly mechanical, so I've only skimmed
these (assuming that someone from the AWT team with do a detailed check).

Have you tested this with a security manager set? I'm just wondering if
PlatformLogger's static initializer should do the property lookup in a
doPrivileged block. Also in the LoggerProxy for the line separator (BTW:
lineSeparator can be final).

Is isLoggingEnabled() used anywhere? I can't see it and wonder if it
should be removed. If it is used, then I assume it needs to be
synchronized with redirectPlatformLoggers.

You allow the PlatformLoggers to be GC'ed but the entries will remain in
the loggers map. I don't think this is a big deal because the AWT
classes will not be unloaded but might be worth putting a note in a
comment to re-visit this some time.

The static initializer in the forwarding proxy seems a bit messy. It
might look neater if changed to something like:
    private static final Class<?> loggerClass =
getClass("java.lang.Logger");                      
    private static final Method getLoggerMethod = getMethod(loggerClass,
"getLogger", String.class);
where getClass does the Class.forName, returning null if CNF, getMethod
returns null if passed a null class, etc.

Minor nit but there are a few style/formatting issues in PlatformLogger.
For example, in JavaLogger there isn't a blank line between methods. It
might be worthing taking a pass over this to have it as neat as possible.

Do you have a bugID to track updating the http protocol handler? Jessie
did push some changes to eliminate the static dependency on logging and
it would be good if that code used PlatformLogger.

-Alan.

[Artem Ananiev-2]

Mandy Chung wrote:

> 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.

I have quickly looked through AWT/2D changes and haven't found anything
suspicious. A funny typo is noticed in X11 key logging (see XToolkit or
XWindow for examples): logger name is sun.awt.X11.kye instead of
sun.awt.X11.key - but anyway it's not your fault :)

> 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.

Thanks.

> 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

Thanks,

Artem

[yuri.nesterenko]

Artem Ananiev wrote:
>
> Mandy Chung wrote:
>> 6879044: Eliminate the dependency of logging from the JRE
>> core/awt/swing classes
>
...

> I have quickly looked through AWT/2D changes and haven't found anything
> suspicious. A funny typo is noticed in X11 key logging (see XToolkit or
> XWindow for examples): logger name is sun.awt.X11.kye instead of
> sun.awt.X11.key - but anyway it's not your fault :)

It's not a typo, it's a decision!!! I forgot why I did it though but
there was a good reason.
...Oh, there it is: it's extremely easy to find
in the code full of everything "key".

-yan

[mandy.chung]

Oleg Sukhodolsky wrote:

> On Mon, Sep 14, 2009 at 5:32 AM, Mandy Chung <Mandy.Chung@...> wrote:
>> 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.
>
> evaluation for 6879044 says:
>
> Add a PlatformLogger class that mimics the default logging behavior
> (output the log message to System.err with simple formatting) and
> creates a java.util.logging.Logger only when the logging facility is
> used by the application or a user-defined configuration is supplied.
>
> which of two descriptions is correct one?

Both are correct.  Can you elaborate what issue;/confusion you see
between the descriptions?

The class description of sun.util.logging.PlatformLogger has the details:
http://cr.openjdk.java.net/~mchung/6879044/webrev.00/src/share/classes/sun/util/logging/PlatformLogger.java.html

> Also I wonder if it is ok to get possible better modularization by
> adding dependency to Sun's internal API into public packages.

This fix is to eliminate the dependency to logging API.   This internal
PlatformLogger class is an implementation class only used by the JRE and
always be present.  Does this answer your question?

>> 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.
>
> Well, at least in AWTEvent you have changed static initialization of
> logger to lazy one.
> Perhaps it is better to keep a static one to minimize your changes.

This fix shows one way of lazy initializing an AWT logger to be
referenced when fixing 6880089.  The AWTEvent log messages are only
output in the error conditions.  In the normal cases, no message will be
logged in this logger is not needed.   I would expect that the fix for
6880089 may make some AWT loggers to be initialized lazily as this one.

Thanks for the review.
Mandy

> Oleg.
>> 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
>>
>>
>>

[gnu_andrew]

2009/9/14 Mandy Chung <Mandy.Chung@...>:

> 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

[Oleg Sukhodolsky-4]

On Mon, Sep 14, 2009 at 9:19 PM, Mandy Chung <Mandy.Chung@...> wrote:

> Oleg Sukhodolsky wrote:
>>
>> On Mon, Sep 14, 2009 at 5:32 AM, Mandy Chung <Mandy.Chung@...> wrote:
>>>
>>> 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.
>>
>> evaluation for 6879044 says:
>>
>> Add a PlatformLogger class that mimics the default logging behavior
>> (output the log message to System.err with simple formatting) and
>> creates a java.util.logging.Logger only when the logging facility is
>> used by the application or a user-defined configuration is supplied.
>>
>> which of two descriptions is correct one?
>
> Both are correct.  Can you elaborate what issue;/confusion you see between
> the descriptions?

I thought that it is possible to log int a file using default logger,
thus I was surprised that
description of PlatformLogger says about System.err.  I'd expect
PlatformLogger to be a wrapper around the default logger,
and as far as I can see from provided sources it is.

> The class description of sun.util.logging.PlatformLogger has the details:
> http://cr.openjdk.java.net/~mchung/6879044/webrev.00/src/share/classes/sun/util/logging/PlatformLogger.java.html
>
>> Also I wonder if it is ok to get possible better modularization by
>> adding dependency to Sun's internal API into public packages.
>
> This fix is to eliminate the dependency to logging API.   This internal
> PlatformLogger class is an implementation class only used by the JRE and
> always be present.  Does this answer your question?

internal PlatformLogger is present in Sun's implementation of JRE
only, so such changes will
complicate porting JRE implementation.  Also I wonder how big logging
module is, what advantage we do receive
by removing dependency on it?  How many Mb we do not need to load in this case?

>>> 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.
>>
>> Well, at least in AWTEvent you have changed static initialization of
>> logger to lazy one.
>> Perhaps it is better to keep a static one to minimize your changes.
>
> This fix shows one way of lazy initializing an AWT logger to be referenced
> when fixing 6880089.  The AWTEvent log messages are only output in the error
> conditions.  In the normal cases, no message will be logged in this logger
> is not needed.   I would expect that the fix for 6880089 may make some AWT
> loggers to be initialized lazily as this one.

I'd suggest to keep all logger's initialization as is to simplify the fix.

Oleg.

>
> Thanks for the review.
> Mandy
>
>> Oleg.
>>>
>>> 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
>>>
>>>
>>>
>

[mandy.chung]

Hi Andrew,

 > Why not just use a straightforward static assignment to a final
 > variable as before?

A simple AWT app creates 79 loggers on solaris-i586 and 34 loggers on
windows-i586.  SwingSet2 creates a total of 85 loggers on solaris-i586
and 35 on windows-i586.

Improvement on client startup performance and memory footprint has
always been one of our top features in the past releases.

As I noted in CR 6880089 (Revisit the number of AWT loggers to reduce
the memory usage), there is no strong reason why we need such
fine-grained loggers.

I fixed AWTEvent since it's one obvious candidate for lazy
initialization. With my fix, I reduced the number of loggers by 4.  Alex
Potochkin also brought up a consistency issue that will be addressed by
6880089 or a new CR.

 > Is it really that unlikely that fine() will be
 > called that we need not initialise this early?

AWT team, can you confirm?

 > At the very least consider using the lazy holder idiom over volatile.

Good point.
Mandy

Andrew John Hughes wrote:

> 2009/9/14 Mandy Chung <Mandy.Chung@...>:
>
> 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.

[mandy.chung]

Oleg Sukhodolsky wrote:
> On Mon, Sep 14, 2009 at 9:19 PM, Mandy Chung <Mandy.Chung@...> wrote:
>
> I thought that it is possible to log int a file using default logger,

Not the default logging configuration.

> thus I was surprised that
> description of PlatformLogger says about System.err.  I'd expect
> PlatformLogger to be a wrapper around the default logger,
> and as far as I can see from provided sources it is.

System.err is the default configuration that ConsoleHandler will output
to.  The default handler specified in <JRE>/lib/logging.properties is:

# By default we only configure a ConsoleHandler, which will only
# show messages at the INFO and above levels.
handlers= java.util.logging.ConsoleHandler

# To also add the FileHandler, use the following line instead.
#handlers= java.util.logging.FileHandler, java.util.logging.ConsoleHandler

If you want to log to a file, you need to supply a logging.properties to
alter the handles property.   If you specify
-Djava.util.logging.config.file=<your logging.properties>, all log
messages will go to the file as described by your logging.properties.
The only difference with the fix is that the platform loggers doesn't
check if the <JRE>/lib/logging.properties is modified.

> internal PlatformLogger is present in Sun's implementation of JRE
> only, so such changes will
> complicate porting JRE implementation.  

How does it complicate the porting?

The issue we are dealing with is that the JDK is big and deeply
interconnected [1].  A command-line hello program loads 300 classes and
a Framer (awt version of helloworld) loads 834 classes running with JDK
7 b71.

Without module support  (JSR 294 + jigsaw) - like we are today,
continual JDK development could cause startup of a simple awt app to
load over 1000 classes very easily (I actually recalled that one point
in time it was over 1000 classes but we put a fix to reduce the number
of loaded classes).

> Also I wonder how big logging
> module is, what advantage we do receive
> by removing dependency on it?  How many Mb we do not need to load in this case?

Not only the MB of the java.util.logging classes but the classes they
pull in at runtime.

> I'd suggest to keep all logger's initialization as is to simplify the fix.

My goal is to reduce the number of logger instances created at startup.
  In addition, I think the fix is very simple.  As I mentioned in my
reply to Andrew, Alex Potochkin also brought up a consistency issue that
will be addressed by 6880089 or a new CR.

I'm ok to take out the lazy initialization of AWT loggers in my fix as
long as the AWT team is going to fix 6880089 soon.  Artem, Alex, what do
you think?

Thanks
Mandy

[1] http://blogs.sun.com/mr/entry/massive_monolithic_jdk

[mandy.chung]

Alan, Thanks for the review.  I'll send out a new webrev to incorporate
your feedback. Yes, I file a CR for the http protocol handler:
    6882384: Update http protocol handler to use PlatformLogger

Several 2D and sun.font classes are refactored in the 2D repository that
will be integrated for b74.  Some classes I modified in the 2D repo are
not in TL repo.  To simplify the integration, I file a new CR 6882376
for the core-libs changes including changes in java.util classes that
will go in TL repo that I think I can make b73.  The AWT/2D/Swing
changes will go in 2D repo as a fix for 6879044.

I'll generate two new webrevs to separate the core-libs and awt/2d/swing
stuff for the review.

Thanks
Mandy

Alan Bateman wrote:

> Mandy Chung wrote:
>> 6879044: Eliminate the dependency of logging from the JRE
>> core/awt/swing classes
>>
>> Webrev:
>>   http://cr.openjdk.java.net/~mchung/6879044/webrev.00/
> The approach seems reasonable to me. It is a bit strange to redirect the
> platform logging to j.u.logging if something up the stack starts logging
> but I think we can live with this because these loggers are for
> debugging purposes.
>
> The changes to the AWT code are mostly mechanical, so I've only skimmed
> these (assuming that someone from the AWT team with do a detailed check).
>
> Have you tested this with a security manager set? I'm just wondering if
> PlatformLogger's static initializer should do the property lookup in a
> doPrivileged block. Also in the LoggerProxy for the line separator (BTW:
> lineSeparator can be final).
>
> Is isLoggingEnabled() used anywhere? I can't see it and wonder if it
> should be removed. If it is used, then I assume it needs to be
> synchronized with redirectPlatformLoggers.
>
> You allow the PlatformLoggers to be GC'ed but the entries will remain in
> the loggers map. I don't think this is a big deal because the AWT
> classes will not be unloaded but might be worth putting a note in a
> comment to re-visit this some time.
>
> The static initializer in the forwarding proxy seems a bit messy. It
> might look neater if changed to something like:
>    private static final Class<?> loggerClass =
> getClass("java.lang.Logger");                          private static
> final Method getLoggerMethod = getMethod(loggerClass, "getLogger",
> String.class);
> where getClass does the Class.forName, returning null if CNF, getMethod
> returns null if passed a null class, etc.
>
> Minor nit but there are a few style/formatting issues in PlatformLogger.
> For example, in JavaLogger there isn't a blank line between methods. It
> might be worthing taking a pass over this to have it as neat as possible.
>
> Do you have a bugID to track updating the http protocol handler? Jessie
> did push some changes to eliminate the static dependency on logging and
> it would be good if that code used PlatformLogger.
>
> -Alan.

[Anthony Petrov]

Hi Mandy,

On 09/15/2009 10:43 PM, Mandy Chung wrote:
>  > Is it really that unlikely that fine() will be
>  > called that we need not initialise this early?
>
> AWT team, can you confirm?

I didn't examine this particular AWTEvent class. I can confirm that in
many places we call the fine() method directly. In most cases we only
wrap the call with an if (logger.isLoggable(...)) when the string that
we log contains some expressions (obviously, for performance reasons.)

--
best regards,
Anthony

[mandy.chung]

Here are the new webrevs:

core-libs changes that include java.util.Currency:
    http://cr.openjdk.java.net/~mchung/6882376/webrev.00/

    - Added a new jtreg test
    - Cleaned up PlatformLogger.java per Alan's feedback.

awt/2d/swing changes:
    http://cr.openjdk.java.net/~mchung/6879044/webrev.01/

    - Keep the logger in AWTEvent.java and Cursor.java be statically
initialized.
    - I updated 6880089 to consider lazy initialization or reduce to a
few loggers vs 85 on unix and 35 on windows.  I decide to leave it to
the fix for 6880089 to make a consistent change.

Files other than the one listed above are not changed since the last
version.

Thanks
Mandy

Mandy Chung wrote:

> 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
>
>

[Oleg Sukhodolsky-4]

On Tue, Sep 15, 2009 at 11:13 PM, Mandy Chung <Mandy.Chung@...> wrote:

>
>
> Oleg Sukhodolsky wrote:
>>
>> On Mon, Sep 14, 2009 at 9:19 PM, Mandy Chung <Mandy.Chung@...> wrote:
>>
>> I thought that it is possible to log int a file using default logger,
>
> Not the default logging configuration.
>
>> thus I was surprised that
>> description of PlatformLogger says about System.err.  I'd expect
>> PlatformLogger to be a wrapper around the default logger,
>> and as far as I can see from provided sources it is.
>
> System.err is the default configuration that ConsoleHandler will output to.
>  The default handler specified in <JRE>/lib/logging.properties is:
>
> # By default we only configure a ConsoleHandler, which will only
> # show messages at the INFO and above levels.
> handlers= java.util.logging.ConsoleHandler
>
> # To also add the FileHandler, use the following line instead.
> #handlers= java.util.logging.FileHandler, java.util.logging.ConsoleHandler
>
> If you want to log to a file, you need to supply a logging.properties to
> alter the handles property.   If you specify
> -Djava.util.logging.config.file=<your logging.properties>, all log messages
> will go to the file as described by your logging.properties. The only
> difference with the fix is that the platform loggers doesn't check if the
> <JRE>/lib/logging.properties is modified.
>
>> internal PlatformLogger is present in Sun's implementation of JRE
>> only, so such changes will
>> complicate porting JRE implementation.
>
> How does it complicate the porting?

I'm not sure that IBM's or some other's version of JDK is allowed to
contain such
classes, thus it may be harder to port our RI to their implementation .

> The issue we are dealing with is that the JDK is big and deeply
> interconnected [1].  A command-line hello program loads 300 classes and a
> Framer (awt version of helloworld) loads 834 classes running with JDK 7 b71.
>
> Without module support  (JSR 294 + jigsaw) - like we are today, continual
> JDK development could cause startup of a simple awt app to load over 1000
> classes very easily (I actually recalled that one point in time it was over
> 1000 classes but we put a fix to reduce the number of loaded classes).

What if an application will use logging? How many Logger.getLogger()
user need to add to his(her)
code to completely initialize logging and make your changes useless?  How often
we will have user's code which doesn't use logging?
Why we have to remove all usages of logging in our code instead of
changing logging package to be
more startup friendly?

Oleg.

>> Also I wonder how big logging
>> module is, what advantage we do receive
>> by removing dependency on it?  How many Mb we do not need to load in this
>> case?
>
> Not only the MB of the java.util.logging classes but the classes they pull
> in at runtime.
>
>> I'd suggest to keep all logger's initialization as is to simplify the fix.
>
> My goal is to reduce the number of logger instances created at startup.  In
> addition, I think the fix is very simple.  As I mentioned in my reply to
> Andrew, Alex Potochkin also brought up a consistency issue that will be
> addressed by 6880089 or a new CR.
>
> I'm ok to take out the lazy initialization of AWT loggers in my fix as long
> as the AWT team is going to fix 6880089 soon.  Artem, Alex, what do you
> think?
>
> Thanks
> Mandy
>
> [1] http://blogs.sun.com/mr/entry/massive_monolithic_jdk
>

[Alan Bateman]

Oleg Sukhodolsky wrote:

> On Tue, Sep 15, 2009 at 11:13 PM, Mandy Chung <Mandy.Chung@...> wrote:
>  
>> :
>>> complicate porting JRE implementation.
>>>      
>> How does it complicate the porting?
>>    
>
> I'm not sure that IBM's or some other's version of JDK is allowed to
> contain such
> classes, thus it may be harder to port our RI to their implementation .
>  

I don't see a problem here. These are implementation classes (you'll see
that AWT already makes use of lot of implementation classes from
sun.awt, sun.security, sun.java2d, and more). Furthermore, these changes
aren't introducing any platform dependent or native code that increases
porting efforts. If there are ports that already remove these loggers
then the effort, once Mandy's changes are in, isn't any different.

:

> Why we have to remove all usages of logging in our code instead of
> changing logging package to be
> more startup friendly?
>  
I haven't seen any proposals to eliminate the logging but rather the
suggestion is that this logging should be re-examined because there are
way too many loggers created at startup. For example, one of the
suggestions that Mandy has put in 6880089 [1] is that there be a logger
per core component rather than class.

-Alan.

[1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6880089

[Oleg Sukhodolsky-4]

On Thu, Sep 17, 2009 at 12:56 PM, Alan Bateman <Alan.Bateman@...> wrote:

> Oleg Sukhodolsky wrote:
>>
>> On Tue, Sep 15, 2009 at 11:13 PM, Mandy Chung <Mandy.Chung@...> wrote:
>>
>>>
>>> :
>>>>
>>>> complicate porting JRE implementation.
>>>>
>>>
>>> How does it complicate the porting?
>>>
>>
>> I'm not sure that IBM's or some other's version of JDK is allowed to
>> contain such
>> classes, thus it may be harder to port our RI to their implementation .
>>
>
> I don't see a problem here. These are implementation classes (you'll see
> that AWT already makes use of lot of implementation classes from sun.awt,
> sun.security, sun.java2d, and more). Furthermore, these changes aren't
> introducing any platform dependent or native code that increases porting
> efforts. If there are ports that already remove these loggers then the
> effort, once Mandy's changes are in, isn't any different.

Ok, they just add one more issue to solve for porters :)

>> Why we have to remove all usages of logging in our code instead of
>> changing logging package to be
>> more startup friendly?
>>
>
> I haven't seen any proposals to eliminate the logging but rather the
> suggestion is that this logging should be re-examined because there are way
> too many loggers created at startup. For example, one of the suggestions
> that Mandy has put in 6880089 [1] is that there be a logger per core
> component rather than class.

By "remove all usages of logging" I meant "remove all usages of
standard logging package".
And, again, I do not understand why we can not change the logging
package instead (perhaps we can not,
but no body has explained why).  BTW if we can change the logging
package to resolve this problem for our code,
such changes will also help to other java developers.

Also since it look like all suggested changes are about startup
performance it would be nice to see results of
performance testing which would prove that suggested changes will
help, and show what kind of applications
will benefit from the changes.  Without such results we can not be
sure that the changes will help and
they can be treated as "premature optimization"

Oleg.

>
> -Alan.
>
> [1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6880089
>
>
>

[Anthony Petrov]

On 09/17/2009 12:56 PM, Alan Bateman wrote:
> I haven't seen any proposals to eliminate the logging but rather the
> suggestion is that this logging should be re-examined because there are
> way too many loggers created at startup. For example, one of the
> suggestions that Mandy has put in 6880089 [1] is that there be a logger
> per core component rather than class.

I have to say that that is not the best possible solution. For instance,
the sun.awt.X11 classes have many different loggers: for focus-related
code, for insets-related code, and so on. If a developer debugs a
particular kind of problem, (s)he doesn't need to look through all the
garbage that other loggers generate: it's just enough to enable a
particular logging facility (such as the
"sun.awt.X11.insets.XDecoratedPeer" for example) and examine what (s)he
really needs.
Combining all the output to just one logger will make debugging a nightmare.

I would second to Oleg: improving the performance/design of the existing
logging classes at java.util.logging package would help all applications
at once.

--
best regards,
Anthony

[Alan Bateman]

Mandy Chung wrote:
> Here are the new webrevs:
>
> core-libs changes that include java.util.Currency:
>    http://cr.openjdk.java.net/~mchung/6882376/webrev.00/
>
>    - Added a new jtreg test
>    - Cleaned up PlatformLogger.java per Alan's feedback.
This looks much better. A couple of additional comments:

I see the lookup of the logging properties is now in a doPrivileged
block - do you need to do the same for the line.separator?

In LoggerProxy, should levelValue and effectiveLevel be volatile?

In JavaLogger.getMethod I see that you return null if the method is not
found. Should it be better to throw an InternalError or AssertionError
here? That is, if java.util.logging is present then something is
seriously wrong is the Logger methods don't exist.

Otherwise, I think I'm okay with this.

-Alan.

[Alan Bateman]

Anthony Petrov wrote:

> :
> I have to say that that is not the best possible solution. For
> instance, the sun.awt.X11 classes have many different loggers: for
> focus-related code, for insets-related code, and so on. If a developer
> debugs a particular kind of problem, (s)he doesn't need to look
> through all the garbage that other loggers generate: it's just enough
> to enable a particular logging facility (such as the
> "sun.awt.X11.insets.XDecoratedPeer" for example) and examine what
> (s)he really needs.
> Combining all the output to just one logger will make debugging a
> nightmare.
>
> I would second to Oleg: improving the performance/design of the
> existing logging classes at java.util.logging package would help all
> applications at once.

I'm not familiar with the AWT implementation to have a strong view as to
how 6880089 is addressed. However, Mandy does raise a good question as
to why there is a need for so many loggers. I think one mail mentioned
there 85 loggers setup when running simple "hello world" Framer test.
Maybe they can be created lazily; maybe some of them aren't needed, but
at least there is a bug created so that someone can re-visit this. I
agree that any improvements to j.u.logging would be welcome too but that
doesn't solve the desire to decouple the dependency. For example, if the
libraries are broken up into a set of fine grain modules then why would
I need to have a logging module installed to run a simple client
application?

-Alan.

[Igor Nekrestyanov]

> I would second to Oleg: improving the performance/design of the
> existing logging classes at java.util.logging package would help all
> applications at once.
That certainly will be best thing to do but unfortunately it is not easy
(possible?) maintaining backward compatibility.

Several people had been looking into this in the past.
Please feel free to advise if you see how it can be improved.

-igor