RFR (XS): 8021879: G1: G1HeapRegionSize flag value not updated correctly

[Thomas Schatzl]

Hi,

On Tue, 2013-07-30 at 14:11 +0200, Bengt Rutisson wrote:
> 
> Hi everyone,
> 
> Could I have a couple of reviews for this very small change?
> 
> http://cr.openjdk.java.net/~brutisso/8021879/webrev.00/
> 
> Background:
> 
> The G1HeapRegionSize flag is not updated with the actual value of the
> region size. This makes it impossible to figure out the heap region
> size using PrintFlagsFinal: 
> 
> $ java -XX:+UseG1GC -XX:+PrintFlagsFinal -version | grep
> G1HeapRegionSize 
>     uintx G1HeapRegionSize = 0 {product} 
> 
> Or when setting it to the wrong value (this will actually use a region
> size of 8m): 
> 
> $ java -XX:+UseG1GC -XX:G1HeapRegionSize=11m -XX:+PrintFlagsFinal
> -version | grep G1HeapRegionSize 
>     uintx G1HeapRegionSize := 11534336 {product} 
> 
> 
> With the proposed patch I get this output:
> 
> $ java -XX:+UseG1GC -XX:+PrintFlagsFinal -version | grep
> G1HeapRegionSize
>     uintx G1HeapRegionSize                         := 1048576
> {product}           
> 
> and:
> 
> $ java -XX:+UseG1GC -XX:G1HeapRegionSize=11m -XX:+PrintFlagsFinal
> -version | grep G1HeapRegionSize
>     uintx G1HeapRegionSize                         := 8388608
> {product} 

Looks good. What about a regression test that verifies that this value
is set to something reasonable after startup?

There are already some helper classes to parse PrintFlagsFinal output in
the gc/g1 test directory.
> 
> Here are also some interesting scenarios that are visible with my
> suggested patch:
> 
> $ java -XX:+UseG1GC -Xmx64g  -XX:+PrintFlagsFinal -version | grep
> G1HeapRegionSize
>     uintx G1HeapRegionSize                         := 1048576
> {product}           
> 
> $ java -XX:+UseG1GC -Xms64g -Xmx64g  -XX:+PrintFlagsFinal -version |
> grep G1HeapRegionSize
>     uintx G1HeapRegionSize                         := 33554432
> {product}           

I think 8019902 tries to rectify this to some degree.

Maybe part of the problem described in that CR is that G1 (and other
collectors) do not update the various sizes (OldSize, NewSize,
MaxNewSize and maybe others) after calculating them, but then use these
variables for that anyway.

Thomas