RFR (S): 8013791: G1: G1CollectorPolicy::initialize_flags() may set min_alignment > max_alignment

[Thomas Schatzl]

Hi again,

  sorry, the email was sent accidentally too early...

On Thu, 2013-05-02 at 21:04 +0200, Thomas Schatzl wrote:
> Hi,
> 
> On Thu, 2013-05-02 at 20:16 +0200, Bengt Rutisson wrote:
> > Hi everyone,
> > 
> > Could I have a couple of reviews for this small patch?
> > 
> > http://cr.openjdk.java.net/~brutisso/8013791/webrev.00/
> > 
> > For G1 it is important that the heap size is aligned by the region size. 
> > The ergonomics will pick CollectorPolicy::max_alignment() for aligning 
> > the heap, but this alignment is based on the card table rather than the 
> > region size.
> > 
> > This fix makes sure that we pick the larger alignment for the 
> > max_alignment value and it adds the same assert for the min and max 
> > alignments that is available in GenCollectorPolicy::initialize_flags().
> 
> Is it possible to move the min/max alignment constraint checking into
> CollectorPolicy::initialize_flags() instead of duplicating the code?
> It seems that this is a general requirement i.e. all other collectors
> use a descendant of GenCollectorPolicy.
> 
> I also think a better reproducer sets MaxRAM and MaxRAMFraction to odd

not "odd" but "specific

> values as this will hit the "right" code paths. E.g.

java -XX:MaxRAM=155M -XX:MaxRAMFraction=1 -XX:MinRAMFraction=1
-XX:G1HeapRegionSize=32M -XX:+UseG1GC -version

This should hit the error always, see Arguments::set_heap_size().

The fix itself looks good.

Hth,
Thomas