>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.r...@oracle.com> Sent: Donnerstag, 9. Juli 2020 17:04 To: Peter Hull <peterhul...@gmail.com> Cc: Baesken, Matthias <matthias.baes...@sap.com>; Jayathirth D v <jayathirth....@oracle.com>; Langer, Christoph <christoph.lan...@sap.com>; 2d-dev@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.baes...@sap.com> > wrote: >> Thank's for the review ! >> >> May I get a second review ? >> >> >> >> >> >> Best regards, Matthias >> >> >> >> >> >> >> >> From: Jayathirth D v<jayathirth....@oracle.com> >> Sent: Donnerstag, 9. Juli 2020 07:21 >> To: Baesken, Matthias<matthias.baes...@sap.com> >> Cc: 2d-dev@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.baes...@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 >> >>