<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<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>
<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>
<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>
<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>
<br>
Thanks,<br>
Bengt<br>
<br>
<br>
<br>
On 5/7/13 12:23 AM, John Cuthbertson wrote:<br>
</div>
<blockquote cite="mid:51882D7B.2090703@oracle.com" type="cite">Hi
Everyone,
<br>
<br>
Can I get a couple of volunteers review these fairly straight
forward changes? The webrev can be found at:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~johnc/8010738/webrev.0/">http://cr.openjdk.java.net/~johnc/8010738/webrev.0/</a>.
<br>
<br>
Summary:
<br>
At sustaining engineering's request I've added the change
information about the metadata space to the detailed heap output
for full GCs:
<br>
<br>
<blockquote type="cite">4.896: [Full GC (Allocation Failure)
512M->343M(512M), 4.3640117 secs]
<br>
[Eden: 0.0B(25.0M)->0.0B(97.0M) Survivors: 0.0B->0.0B
Heap: 512.0M(512.0M)->343.3M(512.0M)], [Metaspace:
1611K->2433K(6448K)]
<br>
[Times: user=5.13 sys=0.15, real=4.36 secs]
<br>
</blockquote>
<br>
The information that gets printed mirrors that of the other
collectors. Here's parallel GC:
<br>
<br>
<blockquote type="cite">33.245: [Full GC (Ergonomics) [PSYoungGen:
133632K->3423K(144640K)] [ParOldGen:
348158K->348158K(348160K)] 481790K->351581K(492800K),
[Metaspace: 1612K->2433K(6448K)], 2.5788089 secs] [Times:
user=9.96 sys=0.00, real=2.58 secs]
<br>
</blockquote>
<br>
The information that's printed is the amount of metaspace used
before the GC, the capacity, and the amount of reserved space.
<br>
<br>
The changes should be straight forward when this is backported to
hs24 - namely replacing the call to
MetaspaceAux::print_metaspace_change() to
perm_gen()->print_heap_change().
<br>
<br>
Most of the changes are the result of renaming some of the fields
in G1CollectorPolicy and the local variables in
G1CollectorPolicy::print_detailed_heap_transition().
<br>
<br>
The included regression test just spawns a child java process to
run a very simple system GC test program with G1 and
+PrintGCDetails. The output of the spawned process is checked for
"Metaspace".
<br>
<br>
Thanks,
<br>
<br>
JohnC
<br>
</blockquote>
<br>
</body>
</html>