<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Hi Poonam,</div><div><br></div>Not a reviewer but looks good. Does this little amount of dup’ed code matter since perm is gone so this code won’t show up in 8 and beyond? In fact wouldn’t keeping if unshared be best in this case?<div><br></div><div>Regards,</div><div>Kirk</div><div><br><div><div>On Apr 18, 2014, at 3:53 AM, Poonam Bajaj <<a href="mailto:poonam.bajaj@oracle.com">poonam.bajaj@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div bgcolor="#FFFFFF" text="#000000">
Hi Jon,<br>
<br>
On 4/17/2014 10:07 PM, Jon Masamitsu wrote:
<blockquote cite="mid:53500340.4050702@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
Poonam,<br>
<br>
In jdk7 I think there is a <br>
<br>
GenCollectedHeap::print_perm_heap_change()<br>
<br>
Your changes have added in G1CollectorPolicy<br>
<pre><span class="new">1168 void G1CollectorPolicy::print_perm_heap_change(size_t perm_prev_used) const {
</span><span class="new"></span>
</pre>
Is there still a GenCollectedHeap::print_perm_heap_change() in
the jdk7<br>
you're fixing and could you have used that instead of adding the<br>
print_perm_heap_change() to G1CollectorPolicy.<br>
</blockquote>
<br>
Yes, there is print_perm_heap_change() in GenCollectedHeap class.
But according to the following hierarchy of classes, I could not use
it in G1CollectedHeap<br>
<br>
//<br>
// CollectedHeap<br>
// SharedHeap<br>
// GenCollectedHeap<br>
// G1CollectedHeap<br>
// ParallelScavengeHeap<br>
//<br>
<br>
So I added the similar function in G1CollectorPolicy. I can move it
to G1CollectedHeap too. And if we don't want duplication, I can
explore the possibility of moving it to SharedHeap and then use it
in both the derived classes.<br>
<br>
Thanks,<br>
Poonam<br>
<br>
<blockquote cite="mid:53500340.4050702@oracle.com" type="cite"> <br>
<br>
<pre><span class="new"></span></pre>
Jon<br>
<br>
<br>
<br>
<div class="moz-cite-prefix">On 4/16/2014 6:03 PM, Poonam Bajaj
wrote:<br>
</div>
<blockquote cite="mid:534F2877.50303@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
Hi,<br>
<br>
Could I have reviews for the following fix:<br>
<br>
<a moz-do-not-send="true" href="https://bugs.openjdk.java.net/browse/JDK-8010738">JDK-8010738</a>:
G1: Output for full GCs with +PrintGCDetails should contain perm
gen/meta data size change info<br>
webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Epoonam/8010738/webrev.00/">http://cr.openjdk.java.net/~poonam/8010738/webrev.00/</a><br>
<br>
With JDK7 G1GC when we use -XX:+PrintGCDetails option, PermGen
information does not get printed in the Full GC logs as shown
below:<br>
[Full GC 16M->16M(20M), 0.7055536 secs]<br>
[Eden: 0.0B(1024.0K)->0.0B(1024.0K) Survivors:
0.0B->0.0B Heap: 16.7M(20.0M)->16.7M(20.0M)]<br>
<br>
PermGen info should also get printed with Full GC info similar
to what other garbage collectors report. These changes are
backport of the fix in jdk8 where MetaSpace info gets printed in
the Full GC logs.<br>
<br>
With the fix the output looks like:<br>
[Full GC 16M->16M(20M), 0.6763478 secs]<br>
[Eden: 0.0B(1024.0K)->0.0B(1024.0K) Survivors:
0.0B->0.0B Heap: 16.7M(20.0M)->16.7M(20.0M)], [Perm:
1725K->1725K(16384K)]<br>
<br>
<br>
Thanks,<br>
Poonam<br>
<br>
</blockquote>
<br>
</blockquote>
</div>
</blockquote></div><br></div></body></html>