RFR (S): 8005989: The default value for MaxNewSize should be less than or equal to MaxHeapSize

[Jon Masamitsu]

Erik,

Change looks good.  Do you think it would
fit better in TwoGenCollectorPolicy::initialize_flags()
or maybe GenCollectorPolicy::initialize_flags()?  You'd have to
add it also to  G1CollectorPolicy::initialize_flags()
or do some refactoring of the various initialize_flags().

Jon


On 01/12/13 00:27, Erik Helin wrote:
> Hi all,
>
> this change sets MaxNewSize to always be less than or equal to 
> MaxHeapSize. The current default value is max_uintx.
>
> Webrev:
> http://cr.openjdk.java.net/~ehelin/8005989/webrev.00/
>
> RFE:
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005989
>
> Analysis:
> The change should not affect the existing code. The case that has to 
> be analyzed is when MaxNewSize is not set on the command line, since 
> the update of MaxNewSize is guarded by "if 
> (FLAG_IS_DEFAULT(MaxNewSize))".
>
> MaxNewSize is only used in the following files:
>
> - g1CollectorPolicy.cpp:
>   Guards all the usage of MaxNewSize in if statements:
>
>      if (FLAG_IS_CMDLINE(MaxNewSize)) {
>         ...
>      }
>
>   Since this change uses FLAG_SET_DEFAULT, this change won't affect
>   this code path.
>
> - arguments.cpp:
>   Only reads MaxNewSize in FLAG_IS_DEFAULT(MaxNewSize) statements.
>   Otherwise, only writes new values to MaxNewSize that are not based on
>   the value of MaxNewSize. This code is not affected since the value of
>   FLAG_IS_DEFAULT(MaxNewSize) is not affected.
>
> - collectorPolicy.cpp:
>   MaxNewSize is used in a couple of places:
>
>     if (NewSize > MaxNewSize) {
>       MaxNewSize = NewSize;
>     }
>
>   If both NewSize and MaxNewSize have their default values, then
>   NewSize is ScaleForWordSize(1*M) which will always be less than
>   MaxNewSize which now is MaxHeapSize. The semantics of the if
>   statement is only changed if NewSize is set on the command line to a
>   value larger than MaxHeapSize, which is not a valid case.
>
>   if (FLAG_IS_CMDLINE(MaxNewSize) || FLAG_IS_ERGO(MaxNewSize) {
>     ...
>   } else {
>     max_new_size = scale_by_NewRatio_aligned(max_heap_byte_size());
>     max_new_size = MIN2(MAX2(max_new_size, NewSize), MaxNewSize);
>   }
>
>   This "if" check will still follow the "else" branch in the default
>   case, just as before, since FLAG_SET_DEFAULT is used in this change.
>   The semantics of the calculation in the else branch is also preserved,
>   since MAX2(max_new_size, NewSize) will still always be less than
>   MaxNewSize for all valid values of NewSize.
>
> Testing:
> Before the change:
>   java -XX:+PrintFlagsFinal -version | grep 'MaxHeapSize \| MaxNewSize'
>       uintx MaxHeapSize := 2017460224 {product}
>       uintx MaxNewSize   = 18446744073709486080 {product}
>
> After the change:
>   java -XX:+PrintFlagsFinal -version | grep 'MaxHeapSize \| MaxNewSize'
>       uintx MaxHeapSize := 2017460224 {product}
>       uintx MaxNewSize   = 2015428608 {product}
>
> JPRT
>
> Thanks,
> Erik