[OpenJDK 2D-Dev] RFR : 8248802: Add log helper methods to FontUtilities.java

[Baesken, Matthias]

>There always should be a call such as FontUtilities.isLogging() test
>protecting doing unnecessary work.

Hi,  in my patch I always  call  isLogging()  in the new methods in  

http://cr.openjdk.java.net/~mbaesken/webrevs/8248802.0/src/java.desktop/share/classes/sun/font/FontUtilities.java.frames.html

So are you talking about other places in the coding ?

Best regards, Matthias



-----Original Message-----
From: Philip Race <philip.race at oracle.com> 
Sent: Donnerstag, 9. Juli 2020 17:04
To: Peter Hull <peterhull90 at gmail.com>
Cc: Baesken, Matthias <matthias.baesken at sap.com>; Jayathirth D v <JAYATHIRTH.D.V at oracle.com>; Langer, Christoph <christoph.langer at sap.com>; 2d-dev at openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] RFR : 8248802: Add log helper methods to FontUtilities.java

I agree.

There always should be a call such as FontUtilities.isLogging() test
protecting doing unnecessary work.


-phil.

On 7/9/20, 3:24 AM, Peter Hull wrote:
> Probably not my place to comment, but, does it matter that it's doing
> unnecessary work evaluating the argument to logWarning et al, in the
> case where logging is not enabled? It only seems to be string
> concatenation and maybe would be optimised out anyway, I don't know.
> Peter
>
> On Thu, 9 Jul 2020 at 08:32, Baesken, Matthias<matthias.baesken at sap.com>  wrote:
>> Thank's  for the review !
>>
>> May I get a second  review ?
>>
>>
>>
>>
>>
>> Best regards, Matthias
>>
>>
>>
>>
>>
>>
>>
>> From: Jayathirth D v<JAYATHIRTH.D.V at ORACLE.COM>
>> Sent: Donnerstag, 9. Juli 2020 07:21
>> To: Baesken, Matthias<matthias.baesken at sap.com>
>> Cc: 2d-dev at openjdk.java.net
>> Subject: Re: [OpenJDK 2D-Dev] RFR : 8248802: Add log helper methods to FontUtilities.java
>>
>>
>>
>> Looks good to me.
>>
>>
>>
>> Thanks,
>>
>> Jay
>>
>>
>>
>> On 06-Jul-2020, at 12:43 PM, Baesken, Matthias<matthias.baesken at sap.com>  wrote:
>>
>>
>>
>> Hello, please review this small change to font related logging .
>>
>>
>>
>> We have a lot of font logging calls in java.desktop that look similar to this coding :
>>
>>          if (FontUtilities.isLogging()) {
>>              FontUtilities.getLogger().info("Here comes my important info");
>>          }
>>
>> This coding could be simplified by adding static log methods to FontUtilities.java
>>
>>
>>
>> public static void logWarning(String s);
>>
>> public static void logInfo(String s);
>>
>> public static void logSevere(String s);
>>
>>
>>
>>    doing the isLogging check + FontUtilities.getLogger(). ...
>>
>>
>>
>>
>>
>>
>>
>> Bug/webrev :
>>
>>
>>
>> https://bugs.openjdk.java.net/browse/JDK-8248802
>>
>>
>>
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8248802.0/
>>
>>
>>
>>
>>
>> Thanks, Matthias
>>
>>