<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<tt>Thanks for the rewview Sangheon, <br>
<br>
I've updated the copy and added the braces as suggested and will
push this once the repos open again.<br>
<br>
Cheers,<br>
Stefan<br>
</tt><br>
<div class="moz-cite-prefix">On 2017-09-13 21:41, sangheon.kim
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:1d2e3ee8-fca3-d5ca-105f-13d06d8c73a1@oracle.com">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
Hi Stefan,<br>
<br>
<div class="moz-cite-prefix">On 09/08/2017 02:56 AM, Stefan
Johansson wrote:<br>
</div>
<blockquote type="cite"
cite="mid:10fcdaae-4456-5257-6856-0cfb319a8795@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<tt>Thanks for reviewing Erik,</tt><br>
<br>
<div class="moz-cite-prefix">On 2017-09-05 15:49, Erik Helin
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:a2a45fc0-5c7b-0fae-8e47-b921e364d1d7@oracle.com">Hi
Stefan, <br>
<br>
thanks for cleaning this up! <br>
<br>
On 09/01/2017 02:57 PM, Stefan Johansson wrote: <br>
<blockquote type="cite">Hi, <br>
<br>
Please review this RFE to convert two more flags to use UL:
<br>
<a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8184286"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8184286</a>
<br>
<br>
Webrev: <br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esjohanss/8184286/hotspot.00/"
moz-do-not-send="true">http://cr.openjdk.java.net/~sjohanss/8184286/hotspot.00/</a>
<br>
</blockquote>
<tt><br>
</tt><tt>For GenCollectedHeap::pri</tt>nt_tracing_info(), I
would prefer it to be renamed to
GenCollectedHeap::log_tracing_info(), since the method now
logs instead of prints :) <br>
<br>
I would have written GenCollectedHeap::print_tracing_info()
like: <br>
<br>
LogTarget(Debug, gc, heap, exit) lt; <br>
<br>
<br>
if (lt.is_enabled()) { <br>
LogStream ls(lt); <br>
<br>
<br>
_young_gen->print_summary_info(&ls); <br>
<br>
<br>
_old_gen->print_summary_info(&ls); <br>
<br>
<br>
} <br>
<br>
and then remove void Generation::print_summary_info(), only
keep Generation::print_summary_info_on(outputStream* st) (but
update the log message like you did). <br>
<br>
But, if you prefer the current approach, that is fine by me as
well. <br>
</blockquote>
<tt>I made a mix to keep us both happy :) Using your approach to
pass a LogStream to print_summary_info_on, but the if check
uses </tt><tt><span class="new">log_is_enabled(Debug, gc,
heap, exit)</span></tt><tt> because all other check use
that.<br>
</tt><tt><span class="new"></span></tt><tt><br>
Updated webrevs:<br>
Inc: <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esjohanss/8184286/hotspot.00-01"
moz-do-not-send="true">http://cr.openjdk.java.net/~sjohanss/8184286/hotspot.00-01</a><br>
</tt><tt><span class="new"></span></tt><tt>Full: <a
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esjohanss/8184286/hotspot.01"
moz-do-not-send="true">http://cr.openjdk.java.net/~sjohanss/8184286/hotspot.01</a><br>
</tt></blockquote>
01 version looks good as is.<br>
<br>
Just minor comments:<br>
1. Copyright update<br>
- parallelScavengeHeap.cpp<br>
- generation.cpp<br>
- generation.hpp<br>
<br>
2. Adding parenthesis at if-statement. This is pre-existing issue
but it would be better fixing it now as those parts are mostly
don't have parenthesis at if-statement. Interestingly
psParallelCompact.cpp line 1901 has it. :)<br>
1) psMarkSweep.cpp<br>
177 if (log_is_enabled(Debug, gc, heap, exit))
accumulated_time()->start();<br>
346 if (log_is_enabled(Debug, gc, heap, exit))
accumulated_time()->stop();<br>
<br>
2) psParallelCompact.cpp<br>
1782 if (log_is_enabled(Debug, gc, heap, exit))
accumulated_time()->start();<br>
<br>
3) psScavenge.cpp<br>
310 if (log_is_enabled(Debug, gc, heap, exit))
accumulated_time()->start();<br>
612 if (log_is_enabled(Debug, gc, heap, exit))
accumulated_time()->stop();<br>
<br>
If you are interested applying above, I don't need extra webrev
for this.<br>
<br>
Thanks,<br>
Sangheon<br>
<br>
<br>
<blockquote type="cite"
cite="mid:10fcdaae-4456-5257-6856-0cfb319a8795@oracle.com"><tt>
<br>
Thanks,<br>
Stefan</tt><br>
<blockquote type="cite"
cite="mid:a2a45fc0-5c7b-0fae-8e47-b921e364d1d7@oracle.com"> </blockquote>
<blockquote type="cite"
cite="mid:a2a45fc0-5c7b-0fae-8e47-b921e364d1d7@oracle.com"> <br>
Thanks, <br>
Erik <br>
<br>
<blockquote type="cite">Summary: <br>
Converting two logging/tracing flags that had not been yet
been changed to use unified logging. TraceYoungGenTime and
TraceOldGenTime prints information at VM exit and the same
information will now be available under tag-set "gc, heap,
exit" on debug level. <br>
<br>
Testing: <br>
* Manually verified that same information as before is
available. <br>
* JPRT for sanity <br>
<br>
Thanks, <br>
Stefan <br>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>