RFR (L): JDK-6843347 Boundary values in some public GC options cause crashes

[Thomas Schatzl]

Hi all,

  please review another version of the changes for this CR after Bengt's
comments.

The difference to the previous is
- do not try to fix up a MarkSweepAlwaysCompactCount=0 setting on the
command line (but fail on argument verification later)
- changed various "unsigned int" to "uint" that are related to
MarkSweepAlwaysCompactCount handling (psMarkSweep.cpp, space.hpp,
markSweep.?pp, 
- remove redundant braces/fix spacing in psMarkSweep.cpp

v1 to v2:
- change the type of MarkSweepAlwaysCompactCount from intx to uintx,
with related changes.

initial to v1:
- minimum value for HeapSizePerGCThread is os::vm_page_size()

Bugs.sun
http://bugs.sun.com/view_bug.do?bug_id=6843347

JBS:
https://jbs.oracle.com/bugs/browse/JDK-6843347

Webrev:
http://cr.openjdk.java.net/~tschatzl/6843347/webrev.3/

Testing:
jprt

Thanks,
  Thomas

On Tue, 2013-05-14 at 09:48 +0200, Thomas Schatzl wrote:
> Hi,
> 
> On Tue, 2013-05-14 at 08:47 +0200, Bengt Rutisson wrote:
> > 
> > Hi Thomas,
> > 
> > In arguments.cpp you check for MarkSweepAlwaysCompactCount == 0 and in
> > that case silently set MarkSweepAlwaysCompactCount = 1. I think I
> > would prefer not to reset it and let there be an error message when we
> > hit "verify_min_value(MarkSweepAlwaysCompactCount, 1,
> > "MarkSweepAlwaysCompactCount");" further down. Otherwise people won't
> > know that they have made a mistake on the command line if they run
> > with -XX:MarkSweepAlwaysCompactCount=0.
> 
> That's fine with me - I added that for backwards compatibility as the
> use e.g. in psMarkSweepDecorator.cpp also silently set it to this value.
> 
> > One minor nit:
> > 
> > In psMarkSweep.cpp you updated one line to be:
> > 
> > unsigned int count = (maximum_heap_compaction)? 1 :
> > MarkSweepAlwaysCompactCount;
> > 
> > I don't think the parenthesis increase readability here, but it would
> > help with a space before the ?. Also, I am more used to seeing uint
> > instead of unsigned int, but HotSpot has a lot of occurrences of both
> > variants, so I leave it up to you to choose. So, to summarize I would
> > prefer:
> > 
> > uint count = maximum_heap_compaction ? 1 :
> > MarkSweepAlwaysCompactCount;
> > 
> Will fix both issues. I changed the type to uint as I also prefer that.
> Thanks for your comments.
> 
> Thomas
> 
> 
>