Review request: 8011380: FX dependency on PlatformLogger broken
bourges.laurent at gmail.com
Thu Apr 4 15:51:08 UTC 2013
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 ?
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):
> 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:
>> 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 (
>> 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
>> 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
>> JavaFX 8 is going to upgrade to use JDK 8 for JavaFX build (
>> 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.
More information about the core-libs-dev