<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Bengt,<br>
<br>
Would a less specific name such as "update" be<br>
better? Something that says to the reader<br>
"you have to go look at what the method is<br>
doing because a name is not going to be <br>
descriptive enough".<br>
<br>
Jon<br>
<br>
On 03/04/13 22:56, Bengt Rutisson wrote:
<blockquote cite="mid:51359721.5030508@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
Hi Tao,<br>
<br>
The method PSAdaptiveSizePolicy::compute_generation_free_space()
is a monster method. It does all kinds of things. Its comment
kind of reveals this:<br>
<br>
339 // Calculates optimial free space sizes for both the old
and young<br>
340 // generations. Stores results in _eden_size and
_promo_size.<br>
341 // Takes current used space in all generations as input,
as well<br>
342 // as an indication if a full gc has just been performed,
for use<br>
343 // in deciding if an OOM error should be thrown.<br>
<br>
So, I think that if we should rename it we should give it a name
that reflects this. The current name is not good, but I don't
think compute_generations_free_space() is much better. I would
suggest to either leave it unchanged or change to a more
descriptive name.<br>
<br>
Maybe gether_adapitve_size_statistics() or
update_adaptive_size_values(). Not very happy with those names
either, but at least they indicate that ther eis more than just
free space calculations in there.<br>
<br>
<br>
In psGCAdaptivePolicyCounters.hpp you updated a method name that
has been commented out. I would suggest removing it rather than
updating it.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
On 3/5/13 1:46 AM, Tao Mao wrote:<br>
</div>
<blockquote cite="mid:51354057.1070308@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
Oops...sent to open list.<br>
<br>
Thanks.<br>
Tao<br>
<div class="moz-cite-prefix"><br>
On 3/4/2013 4:38 PM, Tao Mao wrote:<br>
</div>
<blockquote cite="mid:51353E78.1040700@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
This is the proposed final webrev, with copyright dated
appropriately.<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8007762/webrev.02/">http://cr.openjdk.java.net/~tamao/8007762/webrev.02/</a><br>
<br>
Jon,<br>
A patch is rendered below. Please take the patch and help push
the changeset.<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8007762/webrev.02/8007762RenameMethodsInSizePolicy.patch">http://cr.openjdk.java.net/~tamao/8007762/webrev.02/8007762RenameMethodsInSizePolicy.patch</a><br>
<br>
Thank you.<br>
Tao<br>
<br>
On 2/26/13 8:37 AM, Jon Masamitsu wrote:
<blockquote cite="mid:512CE4C5.6080101@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
Thanks. <br>
<br>
Looks good.<br>
<br>
Jon<br>
<br>
On 02/25/13 11:09, Tao Mao wrote:
<blockquote cite="mid:512BB6E6.6060503@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
A new webrev is updated. <br>
Thanks.<br>
Tao<br>
<br>
<div class="moz-cite-prefix">On 2/18/2013 7:46 AM, Jon
Masamitsu wrote:<br>
</div>
<blockquote cite="mid:51224CBF.8050705@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
Tao,<br>
<br>
Why this change from resize_old_gen() to<br>
resize_tenured_gen()? There are many<br>
variable names and comments referring to<br>
"old gen" still in ParallelScavengeHeap.<br>
</blockquote>
Reverted.<br>
<blockquote cite="mid:51224CBF.8050705@oracle.com"
type="cite"> <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8007762/webrev.00/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp.udiff.html">http://cr.openjdk.java.net/~tamao/8007762/webrev.00/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp.udiff.html</a><br>
<br>
Jon<br>
<br>
On 02/14/13 11:18, Tao Mao wrote:
<blockquote cite="mid:511D388A.4010700@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
8007762: Rename a bunch of methods in size policy
across collectors<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://jbs.oracle.com/bugs/browse/JDK-8007762">https://jbs.oracle.com/bugs/browse/JDK-8007762</a><br>
<br>
webrev: <br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8007762/webrev.00/">http://cr.openjdk.java.net/~tamao/8007762/webrev.00/</a><br>
<br>
changeset:<br>
The motivation of this change is associated with CR
7098155 (<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://jbs.oracle.com/bugs/browse/JDK-7098155">https://jbs.oracle.com/bugs/browse/JDK-7098155</a>).
For this, we need to split
compute_generations_free_space into two routine (one
for compute_eden_space_size and the other for
compute_tenured_generation_free_space) in order to
save some overhead when we want to compute only one of
the two. The next step is to split
PSAdaptiveSizePolicy::compute_generations_free_space(),
which will be resolved in CR 8007763.<br>
<br>
To unify the naming, a bunch of routines are renamed
here.<br>
(1) compute_*: <br>
compute_survivor_space_size_and_threshold() <br>
compute_generations_free_space() =
compute_eden_space_size() +
compute_tenured_generation_free_space()<br>
(2) resize_*_gen: <br>
resize_young_gen() <br>
resize_tenured_gen() <br>
(3) rename judging routine resize_geneneration() to
need_resize() to avoid ambiguity<span style="color:
rgb(0, 0, 0); font-family: Arial, FreeSans,
Helvetica, sans-serif; font-size: 13px; font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: 17px; orphans:
2; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows:
2; word-spacing: 0px; -webkit-text-size-adjust:
auto; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); display:
inline !important; float: none;"></span><br>
<br>
testing:<br>
purely renaming; passed JPRT<br>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
</body>
</html>