<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Bengt,<br>
<br>
Thanks for looking at the changes. Replies inline...<br>
<br>
<div class="moz-cite-prefix">On 5/7/2013 8:40 PM, Bengt Rutisson
wrote:<br>
</div>
<blockquote cite="mid:5189C935.5020107@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
Hi John,<br>
<br>
This looks good!<br>
<br>
One question. Why do we only print the metaspace information for
full GCs? I see that this is consistent with the other GCs, but
the metaspace information changes over time, so wouldn't it be
interesting to print it for every GC?<br>
</div>
</blockquote>
<br>
That's an option. I don't think it would be that expensive since
IIRC the print_metaspace_change() uses the previously cached values
for used(). There is a slow used routine that walks the metadata
spaces which would be expensive.<br>
<br>
<blockquote cite="mid:5189C935.5020107@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
Also a heads up, as you may know there are a couple of suggested
changes to how the capacity and used methods for metaspace
should work. This is probably fine with regards to your change
since it is similar to the other GCs, but it may mean that the
actual output will change.<br>
</div>
</blockquote>
<br>
OK. Thanks for point that out.<br>
<br>
<blockquote cite="mid:5189C935.5020107@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
A couple of minor comments: <br>
<br>
The JTreg test does not have an @run tag. What does that mean?
How is it executed?<br>
</div>
</blockquote>
<br>
That's a good point. I think Mikael explained it but I would be
lying if I knew it beforehand. Anyway here's what I saw in the .jtr
file (confirming Mikael's explanation):<br>
<br>
<blockquote type="cite">#Test Results (version 2)<br>
#Wed May 08 16:53:54 PDT 2013<br>
#checksum:7a223b495367b2dc<br>
#-----testdescription-----<br>
$file=/export/workspaces/8010738/test/gc/g1/TestPrintGCDetails.java<br>
$root=/export/workspaces/8010738/test<br>
keywords=regression bug8010738<br>
library=/testlibrary<br>
run=ASSUMED_ACTION main TestPrintGCDetails\n<br>
source=TestPrintGCDetails.java<br>
title=Ensure that the PrintGCDetails for a full GC with G1
includes Metaspace.<br>
<br>
</blockquote>
<br>
I think it should be safe to not run this test in separate JVM.<br>
<br>
<br>
<blockquote cite="mid:5189C935.5020107@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
What do you think about renaming _heap_bytes_before_gc to
_heap_used_before_gc? Seems more consistent with the other
variable names such as
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
_heap_capacity_before_gc.<br>
<br>
Similarly, what do you think about renaming the local varialbles
*_byte_* to *_used_* in
G1CollectorPolicy::print_detailed_heap_transition()?<br>
</div>
</blockquote>
<br>
See earlier reply to you and Jon about a compromise.<br>
<br>
Thanks,<br>
<br>
JohnC<br>
</body>
</html>