<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#ffffff">
Hi all,<br>
<br>
I fixed a couple of minor issues in the changes below, here's the
updated webrev:<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tonyp/7084509/webrev.1/">http://cr.openjdk.java.net/~tonyp/7084509/webrev.1/</a><br>
<br>
If you've already started looking at the previous one, here are the
two (small) fixes:<br>
<br>
src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp<br>
< double _reserve_regions;<br>
---<br>
> size_t _reserve_regions;<br>
<br>
src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp<br>
< "it's been updated to %u", G1ReservePercent);<br>
---<br>
> "it's been updated to %u", reserve_perc);<br>
<br>
(with thanks to Bengt for pointing me to the first one)<br>
<br>
Tony<br>
<br>
On 08/30/2011 05:22 AM, Tony Printezis wrote:
<blockquote cite="mid:4E5CABE1.9050209@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
Hi all,<br>
<br>
I'd like one more review for this change (Bengt already reviewed
it; I accidentally "stepped on his toes" with this refactoring, so
we reviewed each other's changes to see how to move forward):<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etonyp/7084509/webrev.0/">http://cr.openjdk.java.net/~tonyp/7084509/webrev.0/</a><br>
<br>
This change fixed several issues in the young target length
calculations:<br>
<br>
- There are two entry points to the calculation:
calculate_young_list_target_length() and
calculate_young_list_target_length(size_t rs_lengths). The former
calls the latter, but in some cases the latter can be called by
itself too. But there are some extra calculations (max survivor
size, max GC locker expansion) that were not done when the latter
was called by itself. Additionally, when
calculate_young_list_target_length() was called it also required
for another method to also be called beforehand
(calculate_min_young_list_length()).<br>
<br>
Fix: replace the above with a single method,
update_young_list_target_length() which takes an optional
rs_lengths parameter. Everything is done inside it to ensure that
everything that needs to be calculated, it is calculated and no
other methods need to be called beforehand. This also ensures that
if we want to apply any min / max bounds to the young target
length, we can do so in a single place.<br>
<br>
- The max survivor size is done with an integer division. This
means that, if the resulting value is between 0.0 and 1.0, the max
survivor size will be 0 which effectively tenures everything
during the next GC. It'd be better if it was 1.<br>
<br>
Fix: use double division and ceiling in order for the max survivor
size to be 1 in the above case. Additionally, I now calculate the
survivor parameters at the beginning of a pause instead of when
the young target length is calculated / recalculated. Since those
parameters only affect the next GC it's pointless to calculate /
recalculate them earlier.<br>
<br>
- The code that calculates the optimal young target length (i.e.,
the max young length predicted to be within the required pause
time) is embarrassingly incorrect. It uses binary search to yield
the optimal length, but unfortunately exits early and in many
situations returns a young target length that is shorter than it
could be.<br>
<br>
Fix: updated the binary search algorithm to do the right thing. I
compared the before / after calculations and the after calculation
consistently yielded longer young target lengths which still fit
within the required pause time.<br>
<br>
Additional fixes:<br>
<br>
- I now calculate the heap reserve every time the heap is resized
(as it stays the same for a given heap size). There's no point in
recalculating it every time we do the young target length
calculations.<br>
<br>
- Refactoring and simplification to make the code easier to
follow. This should help make the changes for the following two
CRs easier:<br>
<br>
6929868: <font face="">G1: introduce min / max young gen size
bounds</font><br>
7084525: G1: Generate ergonomic decision log records for young gen
sizing and for pause prediction<br>
<br>
The bulk of the changes are in G1CollectorPolicy. It might be
easier if you looked at the new versions of the following methods:<br>
<br>
G1CollectorPolicy::predict_will_fit()<br>
G1CollectorPolicy::calculate_young_list_desired_min_length()<br>
G1CollectorPolicy::calculate_young_list_desired_max_length()<br>
G1CollectorPolicy::update_young_list_target_length()<br>
G1CollectorPolicy::calculate_young_list_target_length()<br>
<br>
and compared them to the previous versions instead of looking at
their diffs.<br>
<br>
Tony<br>
<br>
</blockquote>
</body>
</html>