<!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 bgcolor="#ffffff" text="#000000">
Hiroshi,<br>
<br>
Looks fine. I only looked at the files where you indicated a change in
your<br>
mail below. If there is more for me to look at, please let me know
which files.<br>
I'll get another one of the GC guys here to also review<br>
the changes. <br>
<br>
I've create a CR for this.<br>
<br>
6952079: With UseParallelScavenge use
MinHeapFreeRation/MaxHeapFreeRatio if UseAdaptiveSivePolicy is off.<br>
<br>
What testing have you done? I would expect that UseParallelGC with<br>
UseAdaptiveSizePolicy turned off to grow and shrink the generations<br>
in a way similar to UseSerialGC. Can you check that is the case?<br>
I think if you set SurvivorRatio explicitly the survivor spaces will<br>
be close. You might have to set InitialSurvivorRatio. If you<br>
set InitialTenuringThreshold and MaxTenuringThreshold to the<br>
same value, I would expect the promotion rate to be close.<br>
Promotion rates don't have to be the same but it would remove<br>
one variable from a comparison. If there are surprises let talk about <br>
them. Different isn't necessarily wrong.<br>
<br>
Jon<br>
<br>
On 05/10/10 16:50, Hiroshi Yamauchi wrote:
<blockquote
cite="mid:AANLkTikV57O2KLdF69X7_42IrrNLxDMTFqsOi_xfat2y@mail.gmail.com"
type="cite">
<pre wrap="">Jon,
Thanks for your comments. My new revision is here
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~hiroshi/webrevs/heapshrink/">http://cr.openjdk.java.net/~hiroshi/webrevs/heapshrink/</a>
My comments are inlined below:
</pre>
<blockquote type="cite">
<pre wrap="">In parallelScavengeHeap.cpp
In the UseParallelGC collector the initial size (init_size) can be
significantly larger than the minimum size. If you want to shrink down
the heap to minimize footprint, do you really want to limit by the
initial size instead of the minimum size?
1001 maximum_desired_capacity = MAX2(maximum_desired_capacity, init_size);
Similarly here the amount of used data could be very small and
setting the floor at the initial size may not be what you
want.
1034 minimum_desired_capacity = MAX2(minimum_desired_capacity, init_size);
</pre>
</blockquote>
<pre wrap=""><!---->
I changed the code so that it uses the min size instead of the init size.
</pre>
<blockquote type="cite">
<pre wrap="">In psYoungGen.cpp you guard output with PrintGC only. In similar
cases this type of output is guarded with Verbose also. Does this
output as is get printed in the middle of the usual
-verbosegc (also known as PrintGC) line?
1000 if (PrintGC) {
1001 gclog_or_tty->print_cr(" Resizing young gen. expand_bytes=%d,%d",
1002 eden_expand_bytes, survivor_expand_bytes);
1003 gclog_or_tty->print("BEFORE: Young Gen: ");
1004 gclog_or_tty->print("eden capacity : " SIZE_FORMAT ", ",
1005 eden_space()->capacity_in_bytes());
1006 gclog_or_tty->print("eden used : " SIZE_FORMAT ", " ,
1007 eden_space()->used_in_bytes());
1008 gclog_or_tty->print("survivor capacity : " SIZE_FORMAT ", ",
1009 from_space()->capacity_in_bytes());
1010 gclog_or_tty->print_cr("survivor used : " SIZE_FORMAT ", " ,
1011 from_space()->used_in_bytes());
1012 }
</pre>
</blockquote>
<pre wrap=""><!---->
I added Verbose to the guard. And yes, the output show up in the
middle of PrintGC line.
</pre>
<blockquote type="cite">
<pre wrap="">
In psOldGen.cpp you could try to expand up to max_gen_size
instead of returning?
501 void PSOldGen::try_to_expand_by(size_t expand_bytes) {
502 if (expand_bytes < MinHeapDeltaBytes ||
503 capacity_in_bytes() + expand_bytes > max_gen_size()) {
504 return;
Additionally PSOldGen::expand() chooses to use MinHeapDeltaBytes
as the minimum expansion size (in the sense that sizes for expansion
are round up to MinHeapDeltaBytes). You would rather not expand
for less than MinHeapDeltaBytes? I'll admit that there maybe some
inconsistencies in the way MinHeapDeltaBytes is used. Just checking
that this is what you want to do.
Again, seems to me that you want to shrink down to the minimum
generation size as opposed to the initial size.
523 void PSOldGen::try_to_shrink_by(size_t shrink_bytes) {
524 if (shrink_bytes < MinHeapDeltaBytes ||
525 capacity_in_bytes() - shrink_bytes < init_gen_size()) {
526 return;
527 }
</pre>
</blockquote>
<pre wrap=""><!---->
Now I have:
502 if (capacity_in_bytes() + expand_bytes > max_gen_size()) {
503 expand_bytes = max_gen_size() - capacity_in_bytes();
504 }
and likewise for try_to_shrink_by().
1. It allows the subsequent code (eg expand()) to round up the size <
MinHeapDeltaBytes.
2. It rounds down the size that goes past the max/min gen size so that
the heap is expanded/shrunk to the max/min.
</pre>
<blockquote type="cite">
<pre wrap="">In globals.hpp. We may want this feature implemented in other
collectors at some point - G1 comes to mind. I'd drop the leading
PS on the flag so that it is ResizeByFreeRatioWithSystemGC.
3079 product(bool, PSResizeByFreeRatioWithSystemGC, false, \
3080 "Resize the heap by free ratio in System.gc() " \
3081 "under UseParallelGC")
</pre>
</blockquote>
<pre wrap=""><!---->
Done.
Hiroshi
</pre>
</blockquote>
</body>
</html>