Review request: 8011380: FX dependency on PlatformLogger broken

[Laurent Bourgès]

Well done: binary search to find the closest matching level !

However, I still do not see any concrete use case for custom Levels in such
closed API as PlatformLogger is: KISS, please !

I mean as PlatformLogger is only used (available) to JDK and related
projects, is there any RFE or will to use custom JUL levels here ?

Laurent

2013/4/4 Peter Levart <peter.levart at gmail.com>

> Hi Mandy, Laurent,
>
> What do you think of this variation (changed just PlatformLogger, other
> files exactly equal to your webrev):
>
> http://dl.dropbox.com/u/**101777488/jdk8-tl/**
> PlatformLogger/webrev.enumapi.**02/index.html<http://dl.dropbox.com/u/101777488/jdk8-tl/PlatformLogger/webrev.enumapi.02/index.html>
>
> Moved the nearest >= intValue search to PlatformLogger.Level.valueOf(**int)
> since it is used also by deprecated API. With this change the same logic is
> used for mapping from j.u.l.Level to PlatformLogger.Level in all cases.
>
> I wonder if PlatformLogger.Level.intValue(**) should be exposed as public
> API. Currently it is used by the test (which is not in the same package).
> But this could be circumvented with reflection. I only see the use of it if
> we also make the PlatformLogger.Level.valueOf(**int) public with
> anticipation of enabling PlatformLogger to use custom
> PlatformLogger.Level(s) like j.u.logging. This extension could be performed
> compatibly by transforming Level enum into a plain class like j.u.l.Level.
> But until then, I don't see the use of Level.intValue() as a public API.
> What do you think?
>
> Regards, Peter
>
>
> On 04/04/2013 06:52 AM, Mandy Chung wrote:
>
>> Peter, Laurent,
>>
>> History and details are described below.
>>
>> Webrev at:
>> http://cr.openjdk.java.net/~**mchung/jdk8/webrevs/8011380/**webrev.00/<http://cr.openjdk.java.net/%7Emchung/jdk8/webrevs/8011380/webrev.00/>
>>
>> I add back the getLevel(int), setLevel(int) and isLoggable(int) methods
>> but marked deprecated and also revert the static final variables to resolve
>> the regression. They can be removed when JavaFX transitions to use the
>> Level enums (I'll file one issue for FX to use PlatformLogger.Level and one
>> for core-libs to remove the deprecated methods).  The performance overhead
>> is likely small since it's direct mapping from int to the Level enum that
>> was used in one of your previous performance measurement.
>>
>> Laurent - you have a patch to add isLoggable calls in the awt/swing code.
>> Can you check if there is any noticeable performance difference?
>>
>> I also take the opportunity to reconsider what JavaLoggerProxy.getLevel()
>> should return when it's a custom Level. Use of logging is expected not to
>> cause any fatal error to the running application.  The previous patch
>> throwing IAE needs to be fixed.  I propose to return the first
>> Level.intValue() >= the custom level value since the level value is used to
>> evaluate if it's loggable.
>>
>> History and details:
>> JavaFX 8 has converted to use sun.util.logging.**PlatformLogger (
>> https://javafx-jira.kenai.**com/browse/RT-24458<https://javafx-jira.kenai.com/browse/RT-24458>).
>> I was involved in the early discussion but wasn't aware of the decision
>> made. Thanks to Alan for catching this regression out before it's
>> integrated to jdk8.  jfxrt.jar is cobundled with jdk8 during the installer
>> build step.  My build doesn't build installer and thus we didn't see this
>> regression.
>>
>> I ran jdeps on jdk8/lib/ext/jfxrt.jar (windows-i586) that shows 112
>> references to PlatformLogger and on jdk7/lib/jfxrt.jar that shows no
>> reference to sun.util.logging.
>>
>> I have a method finder tool (planning to include it in jdk8) to search
>> for use of PlatformLogger.getLevel and it did find 2 references from
>> jdk8/lib/ext/jfxrt.jar.
>>
>> JavaFX 8 is going to upgrade to use JDK 8 for JavaFX build (
>> https://javafx-jira.kenai.**com/browse/RT-27794<https://javafx-jira.kenai.com/browse/RT-27794>)
>> soon (currently it's built with JDK 7).  As soon as JavaFX code are changed
>> to reference PlatformLogger.Level enum instead, we can remove the
>> deprecated methods and change the PlatformLogger constants.
>>
>> JavaFX 2.2.x doesn't use sun.util.logging and so this has no impact to
>> it.  In any case, JavaFX 2.2.x only runs either bundled with a
>> corresponding JDK 7u release, or as a standalone library for JDK 6 only.
>>
>> Thanks
>> Mandy
>>
>>
>