Request for review (S): 7110718 -XX:MarkSweepAlwaysCompactCount=0 crashes the JVM

[Bengt Rutisson]

Ramki,

On 2011-11-14 20:32, Srinivas Ramakrishna wrote:
>
> Thanks, Bengt, for the super-quick turnaround!! A comment below on the 
> choice of <= 0 for the option value....

Thanks for the review! See comments below.

> On Mon, Nov 14, 2011 at 1:25 AM, Bengt Rutisson 
> <bengt.rutisson at oracle.com <mailto:bengt.rutisson at oracle.com>> wrote:
>
>
>     Hi all,
>
>     Can I have a couple of reviews for this small change?
>     http://cr.openjdk.java.net/~brutisso/7110718/webrev.01/
>     <http://cr.openjdk.java.net/%7Ebrutisso/7110718/webrev.01/>
>
>     It is a fix for the issue that Ramki reported recently.
>     MarkSweepAlwaysCompactCount is used for division and Hotspot
>     crashes if it is set to 0.
>
>     I choose to log an error and exit the VM if someone tries to start
>     with -XX:MarkSweepAlwaysCompactCount=0. An alternative is to just
>     log a warning and set it to 1.
>
>     I prefer the error way since it is not really clear what one wants
>     to achieve with MarkSweepAlwaysCompactCount=0. Always do full
>     compactions or never do full compactions? So I am not convinced
>     that 1 is an appropriate value.
>
>     Also, since the VM, up until now, has crashed if someone tried
>     -XX:MarkSweepAlwaysCompactCount=0 I think we can be sure that
>     there are no customers that are running with that setting. It
>     should be safe to forbid it.
>
>
> I agree with that statement. However, given that the value 0 was 
> producing crashes, proving that no production code would have been 
> using that setting,
> and based on yr comment above that the 0 value could as well have been 
> used to denote "never force full compaction", it seems as though an 
> alternative to exiting with
> an error, is now to define all values <=0 to mean "never _force_ full 
> compaction"
>
> Especially since tolerating allowed input values and mapping them to 
> specific non-exiting behaviours allows us to modify production JVM's 
> on the fly
> without causing loss of availability. (Consider a future in which this 
> option becomes a "manageable"; you would then be faced with the same
> question, and it seems as though making this choice now would help 
> maintain consistency and robustness going forward -- we could of course
> always throw a "illegal value exception" or such at that point, but 
> allowing the specification of "never _force_ full compaction" (unless 
> the JVM
> otherwise chooses to) would appear to be a choice to allow users; 
> mapping negative and 0 values to that setting would avoid having to
> throw an error.) However, I understand that this is somewhat 
> subjective, so I am willing to go with whatever the majority consensus 
> here
> mght be. It just seemed more pleasant to:
> (1) allow the specification of reasonable behaviour (i.e. never 
> _force_ ...)
> (2) map the full domain of the option to a reasonable behaviour (i.e. 
> allow <= 0 to map to never _force_ ..)
>
> Comments?

I see your point, and I think this should be fairly straight forward to 
fix. However it will require some more testing etc. I can do that, but I 
don't think I know enough to say whether or not the extra work is worth 
it. How important is this option? Is it something that customers use a lot?

Thanks,
Bengt

>
> -- ramki
>
>
>
>     CR:
>
>     7110718 -XX:MarkSweepAlwaysCompactCount=0 crashes the JVM
>     http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7110718
>
>     Thanks,
>     Bengt
>
>

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