>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 <[email protected]> 
Sent: Donnerstag, 9. Juli 2020 17:04
To: Peter Hull <[email protected]>
Cc: Baesken, Matthias <[email protected]>; Jayathirth D v 
<[email protected]>; Langer, Christoph <[email protected]>; 
[email protected]
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<[email protected]>  
> wrote:
>> Thank's  for the review !
>>
>> May I get a second  review ?
>>
>>
>>
>>
>>
>> Best regards, Matthias
>>
>>
>>
>>
>>
>>
>>
>> From: Jayathirth D v<[email protected]>
>> Sent: Donnerstag, 9. Juli 2020 07:21
>> To: Baesken, Matthias<[email protected]>
>> Cc: [email protected]
>> 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<[email protected]>  
>> 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
>>
>>

Reply via email to