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

[Bengt Rutisson]

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.

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;

Otherwise looks good!
Bengt

On 5/13/13 9:42 PM, Thomas Schatzl wrote:
> Hi all,
>
>    after an internal review, please have a look at the current version.
>
> The difference to the previous is
> - change the type of MarkSweepAlwaysCompactCount from intx to uintx,
> with related changes.
>
> Difference to the first version
> - minimum value for HeapSizePerGCThread is os::vm_page_size() (suggested
> by Jon M.)
>
> 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.2/
>
> Testing:
> jprt
>
> Thanks,
>    Thomas
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130514/d79c7513/attachment.htm>