Hi Laurent, Looks good.
Is there some way to reduce the impact of not hard-coding doMonitors for future benefit? Or is that constant only for debugging modifications?
...jim On 12/5/15 8:54 AM, Laurent Bourgès wrote:
Phil & Jim, Here is the updated webrev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144654.1/ Changes: - isEnableLogs() renamed to isLoggingEnabled(); I used the convention "is<MemberName>" even if it is not english ! - restored doMonitors to false (as it reduces the bytecode count for inlining) Comments below: 2015-12-05 0:05 GMT+01:00 Jim Graham <james.gra...@oracle.com <mailto:james.gra...@oracle.com>>: Perhaps "isLoggingEnabled()"? Fixed. This turns a bunch of constants from having compile-time values to run-time values so the javac compiler will no longer be able to compile the code out of the classfiles. On the other hand, the fields are still final so the Hotspot compiler should still be able to eliminate the associated code at run-time, so hopefully this had no impact on performance? No On 12/4/15 2:43 PM, Phil Race wrote: 139 public static boolean isEnableLogs() { 140 return getBoolean("sun.java2d.renderer.log", "false"); 141 } All of these begin with sun.java2d.renderer but they are all marlin specific. I guess it is OK though if we expect that ultimately marlin is the only renderer. I can later rename all Marlin system properties at once to use the prefix "sun.java2d.marlin" if you prefer (already discussed?). "isEnable" does not read well but I know that the same pattern is used for everything so I don't have a firm objection although the "is" seems likeit is could be removed in most cases. Some may need a "do" adding Fixed using Jim's proposal. You have deleted getCallerInfo so probably you can close https://bugs.openjdk.java.net/browse/JDK-8144530 as a dup of this bug. Done. Cheers, Laurent