[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR 8144654: Improve Marlin logging
From: Philip Race <philip.race () oracle ! com>
Date: 2015-12-10 23:41:54
Message-ID: 566A0DC2.8030801 () oracle ! com
[Download RAW message or body]
+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@oracle.com
> <mailto:james.graham@oracle.com>
> >> <mailto:james.graham@oracle.com <mailto:james.graham@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
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
+1<br>
<br>
-phil.<br>
<br>
<br>
On 12/8/15, 12:03 AM, Laurent Bourgès wrote:
<blockquote
cite="mid:CAKjRUT5ywmsFAZ4Cu1rsESsb-DZ174BoZMz+AmTNYHqwg-WLVQ@mail.gmail.com"
type="cite">
<p dir="ltr">Jim,</p>
<p dir="ltr">> Looks good.<br>
><br>
> Is there some way to reduce the impact of not hard-coding
doMonitors for future benefit? Or is that constant only for
debugging modifications?</p>
<p dir="ltr">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).</p>
<p dir="ltr">The JMH perfasm profiler is better as it provides
more accurate timings and lower overhead!</p>
<p dir="ltr">Laurent</p>
<p dir="ltr">> On 12/5/15 8:54 AM, Laurent Bourgès wrote:<br>
>><br>
>> Phil & Jim,<br>
>><br>
>> Here is the updated webrev:<br>
>> <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Elbourges/marlin/marlin-8144654.1/">http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144654.1/</a><br>
>><br>
>> Changes:<br>
>> - isEnableLogs() renamed to isLoggingEnabled(); I used
the convention<br>
>> "is<MemberName>" even if it is not english !<br>
>> - restored doMonitors to false (as it reduces the
bytecode count for<br>
>> inlining)<br>
>><br>
>> Comments below:<br>
>><br>
>> 2015-12-05 0:05 GMT+01:00 Jim Graham <<a
moz-do-not-send="true" \
href="mailto:james.graham@oracle.com">james.graham@oracle.com</a><br> >> \
<mailto:<a moz-do-not-send="true"
href="mailto:james.graham@oracle.com">james.graham@oracle.com</a>>>:<br>
>><br>
>><br>
>> Perhaps "isLoggingEnabled()"?<br>
>><br>
>><br>
>> Fixed.<br>
>><br>
>><br>
>> This turns a bunch of constants from having
compile-time values to<br>
>> run-time values so the javac compiler will no
longer be able to<br>
>> compile the code out of the classfiles. On the
other hand, the<br>
>> fields are still final so the Hotspot compiler
should still be able<br>
>> to eliminate the associated code at run-time, so
hopefully this had<br>
>> no impact on performance?<br>
>><br>
>><br>
>> No<br>
>><br>
>><br>
>><br>
>> On 12/4/15 2:43 PM, Phil Race wrote:<br>
>><br>
>> 139 public static boolean isEnableLogs() {<br>
>> 140 return
getBoolean("sun.java2d.renderer.log", "false");<br>
>> 141 }<br>
>><br>
>> All of these begin with sun.java2d.renderer but
they are all marlin<br>
>> specific.<br>
>> I guess it is OK though if we expect that
ultimately marlin is<br>
>> the only<br>
>> renderer.<br>
>><br>
>><br>
>> I can later rename all Marlin system properties at once
to use the<br>
>> prefix "sun.java2d.marlin" if you prefer (already
discussed?).<br>
>><br>
>><br>
>> "isEnable" does not read well but I know that
the same pattern<br>
>> is used for<br>
>> everything so I don't have a firm objection
although the "is"<br>
>> seems likeit<br>
>> is could be removed in most cases. Some may
need a "do" adding<br>
>><br>
>><br>
>> Fixed using Jim's proposal.<br>
>><br>
>><br>
>> You have deleted getCallerInfo so probably you
can close<br>
>> <a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8144530">https://bugs.openjdk.java.net/browse/JDK-8144530</a>
as a dup of<br>
>> this bug.<br>
>><br>
>><br>
>> Done.<br>
>><br>
>> Cheers,<br>
>> Laurent<br>
</p>
</blockquote>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic