RFR(S): 8010738: G1: Output for full GCs with +PrintGCDetails should contain perm gen size/meta data change info
John Cuthbertson
john.cuthbertson at oracle.com
Thu May 9 00:04:19 UTC 2013
Hi Bengt,
Thanks for looking at the changes. Replies inline...
On 5/7/2013 8:40 PM, Bengt Rutisson wrote:
>
> Hi John,
>
> This looks good!
>
> 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?
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.
>
> 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.
OK. Thanks for point that out.
>
> A couple of minor comments:
>
> The JTreg test does not have an @run tag. What does that mean? How is
> it executed?
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):
> #Test Results (version 2)
> #Wed May 08 16:53:54 PDT 2013
> #checksum:7a223b495367b2dc
> #-----testdescription-----
> $file=/export/workspaces/8010738/test/gc/g1/TestPrintGCDetails.java
> $root=/export/workspaces/8010738/test
> keywords=regression bug8010738
> library=/testlibrary
> run=ASSUMED_ACTION main TestPrintGCDetails\n
> source=TestPrintGCDetails.java
> title=Ensure that the PrintGCDetails for a full GC with G1 includes
> Metaspace.
>
I think it should be safe to not run this test in separate JVM.
>
> 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 _heap_capacity_before_gc.
>
> Similarly, what do you think about renaming the local varialbles
> *_byte_* to *_used_* in
> G1CollectorPolicy::print_detailed_heap_transition()?
See earlier reply to you and Jon about a compromise.
Thanks,
JohnC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130508/4df7fa42/attachment.htm>
More information about the hotspot-gc-dev
mailing list