Review request: 8011380: FX dependency on PlatformLogger broken (original) (raw)

Laurent Bourgès 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 ?

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

More information about the core-libs-dev mailing list