<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 Coleen,<br>
<br>
On 5/8/13 1:44 PM, Coleen Phillimore wrote:<br>
</div>
<blockquote cite="mid:518A3A97.6070107@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">On 5/7/2013 11: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>
The metaspace will only grow between young collections as new
classes are loaded. It will only shrink during full GC's or any
GC that does class unloading. <br>
</blockquote>
<br>
Right, but I thought it would actually be interesting to see how the
metaspace grows.<br>
<br>
<blockquote cite="mid:518A3A97.6070107@oracle.com" type="cite"> It
seems expensive to print it out for all GC's.<br>
</blockquote>
<br>
Is it really that expensive?<br>
<br>
Anyway, I'm fine with the way John did it now. It is consistent with
the other GCs. <br>
<br>
John, I just noticed one thing that I find a bit odd. The
G1CollectorPolicy::print_detailed_heap_transition() method is only
called from
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
G1CollectedHeap::do_collection(), so it really doesn't have to take
the full parameter. It could always print the metaspace information,
right?<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote cite="mid:518A3A97.6070107@oracle.com" type="cite"> <br>
Coleen<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>
<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
moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejohnc/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>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>