<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Everyone,<br>
<br>
A new version of the changes can be found at:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~johnc/7145569/webrev.2/">http://cr.openjdk.java.net/~johnc/7145569/webrev.2/</a><br>
<br>
Changes in this version include:<br>
<br>
* The verification code blob closure in g1CollectedHeap.cpp now
inherits directly from CodeBlobClosure rather than
CodeBlobToOopClosure to address Mikael's final comment.<br>
* The list of nmethods has been moved from the HeapRegion class to
the HeapRegionRemSet class. The routines in HeapRegion are now, as
Thomas suggested, wrappers and invoke the associated method from the
RSet owned by the heap region.<br>
* Added a routine to return the number of bytes currently occupied
by the list of nmethods and used that in the G1SummarizeRSetStats
output and the G1PrintRegionLivenessInfo output. I've been back and
forth about whether to include the size of the memory occupied by
the code roots into the value returned by
HeapRegionRemSet::mem_size(). In this implementation I've included
it but also included separate output for it in RSet stats and region
liveness output.<br>
<br>
In the webrev I have folded Mikael's and Thomas' first set comments
into the main patch:<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.1.optimize-nmethod-scanning/">http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.1.optimize-nmethod-scanning/</a><br>
<br>
Thomas' second set of comments are given in:<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.2.thomas-comments-2/">http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.2.thomas-comments-2/</a><br>
<br>
And, as always, the whole tamale is in:<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.all/">http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.all/</a><br>
<br>
An example of the new RSet stats output is:<br>
<br>
<tt> Concurrent RS processed 154 cards</tt><tt><br>
</tt><tt> Of 21 completed buffers:</tt><tt><br>
</tt><tt> 21 (100.0%) by concurrent RS threads.</tt><tt><br>
</tt><tt> 0 ( 0.0%) by mutator threads.</tt><tt><br>
</tt><tt> Concurrent RS threads times (s)</tt><tt><br>
</tt><tt> 0.00 0.00 0.00 0.00</tt><tt><br>
</tt><tt> Concurrent sampling threads times (s)</tt><tt><br>
</tt><tt> 0.00</tt><tt><br>
</tt><tt> Total heap region rem set sizes = 231K. Max = 1K.</tt><tt><br>
</tt><tt> Static structures = 32K, free_lists = 0K.</tt><tt><br>
</tt><tt> 0 occupied cards represented.</tt><tt><br>
</tt><tt> Max size region =
0:(O)[0xac400000,0xac446590,0xac500000], size = 2K, occupied = 0K.</tt><tt><br>
</tt><tt> Did 0 coarsenings.</tt><tt><br>
</tt><tt><i> Total heap region code-root set sizes = 10K. Max =
0K.</i></tt><tt><i><br>
</i></tt><tt><i> Max size region =
0:(O)[0xac400000,0xac446590,0xac500000], size = 1K, num_elems =
32.</i></tt><tt><br>
</tt><br>
<br>
The new G1PrintRegionLivenessInfo output is:<br>
<br>
<tt>### PHASE Post-Marking @ 1.673</tt><tt><br>
</tt><tt>### HEAP committed: 0xac400000-0xb5b00000 reserved:
0xac400000-0xec400000 region-size: 1048576</tt><tt><br>
</tt><tt>###</tt><tt><br>
</tt><tt>### type address-range used prev-live
next-live gc-eff remset code-roots</tt><tt><br>
</tt><tt>### (bytes) (bytes)
(bytes) (bytes/ms) (bytes) (bytes)</tt><tt><br>
</tt><tt>### OLD 0xac400000-0xac500000 288144 284688
284688 295956.8 1852 196</tt><tt><br>
</tt><tt>### FREE 0xac500000-0xac600000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>### FREE 0xac600000-0xac700000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>### FREE 0xac700000-0xac800000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>### FREE 0xac800000-0xac900000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>### FREE 0xac900000-0xaca00000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>### FREE 0xaca00000-0xacb00000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>[snip]</tt><tt><br>
</tt><tt>### FREE 0xb5700000-0xb5800000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>### FREE 0xb5800000-0xb5900000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>### FREE 0xb5900000-0xb5a00000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>### FREE 0xb5a00000-0xb5b00000 0
0 0 0.0 1732 76</tt><tt><br>
</tt><tt>###</tt><tt><br>
</tt><tt>### SUMMARY capacity: 151.00 MB used: 12.40 MB / 8.21 %
prev-live: 12.40 MB / 8.21 % next-live: 2.67 MB / 1.77 % remset:
0.28 MB code-roots: 0.01 MB</tt><tt><br>
</tt><br>
Testing:<br>
specjvm98, client/server with -XX:+VerifyBeforeGC -XX:+VerifyAfterGC
-XX:+VerifyDuringGC -XX:+G1VerifyHeapRegionCodeRoots
-XX:+G1SummarizeRSetStats -XX:G1SummarizeRSetStatsPeriod=5
-XX:+G1PrintRegionLivenessInfo<br>
Kitchensink (4h runs), client/server with -XX:+VerifyBeforeGC
-XX:+VerifyAfterGC -XX:+VerifyDuringGC
-XX:+G1VerifyHeapRegionCodeRoots <br>
<br>
Thanks,<br>
<br>
JohnC<br>
<br>
<div class="moz-cite-prefix">On 6/21/2013 2:59 AM, Thomas Schatzl
wrote:<br>
</div>
<blockquote cite="mid:1371808776.2716.27.camel@cirrus" type="cite">
<pre wrap="">Hi,
On Thu, 2013-06-20 at 14:00 -0700, John Cuthbertson wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Thomas,
On 6/19/2013 1:33 PM, Thomas Schatzl wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi,
one issue I forgot to mention: currently when printing remembered set
sizes, the new nmethod remembered set is not taken into account.
I am asking myselves, isn't the _strong_code_root_list not a type of
remembered set, and shouldn't it be added to this size? What do you
think?
</pre>
</blockquote>
<pre wrap="">
I think you're right. In fact I've been calling it a sort of remembered set.
</pre>
</blockquote>
<pre wrap="">
Great.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">Additionally it would be really nice to make this information available
in some form so that further (size related) issues in that area could be
diagnosed.
</pre>
</blockquote>
<pre wrap="">
In the short term we could certainly add a strong_code_root_mem_size()
routine to HeapRegion and call that from the appropriate places. It
should be straight forward - GrowableArrays keep track of their capacity.
</pre>
</blockquote>
<pre wrap="">
This seems suboptimal as this would require all places that use
G1RemSet::mem_size() to add this call extra call (but possible); my
intention is to include this value in regular printouts of remembered
set sizes.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">Unfortunately at the moment the _strong_roots_code_list is located in
HeapRegion, not in the remembered set, where the method to get its size
is.
</pre>
</blockquote>
<pre wrap="">
Again calling a routine from the HeapRegion that "own" the RSet should
be OK. I'll send out a new webrev when I've done this.
Longer term moving the code roots in to the actual Rset structure (not
OtherRegionsTable though) is desirable. I'll investigate how much effort
it will be to this.
</pre>
</blockquote>
<pre wrap="">
That's exactly what I had in mind. Thanks for looking into this.
Thanks,
Thomas
</pre>
</blockquote>
<br>
</body>
</html>