[OpenJDK 2D-Dev] RFR 8144654: Improve Marlin logging

[Philip Race]

+1

-phil.


On 12/8/15, 12:03 AM, Laurent Bourgès wrote:
>
> Jim,
>
> > 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?
>
> Durin Marlin development, I occasionally use doMonitor to get internal 
> timings so it is a developper feature that should remain confidential 
> but interesting to keep in the code (disabled by default).
>
> The JMH perfasm profiler is better as it provides more accurate 
> timings and lower overhead!
>
> Laurent
>
> > 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/ 
> <http://cr.openjdk.java.net/%7Elbourges/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.graham at oracle.com 
> <mailto:james.graham at oracle.com>
> >> <mailto:james.graham at oracle.com <mailto:james.graham at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20151210/a213ef9c/attachment.html>