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 <[email protected]
<mailto:[email protected]>>:
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
