Request for review 7122219: Passed StringTableSize value not verified

[David Holmes]

On 12/11/2012 11:37 PM, harold seigel wrote:
> Hi David,
>
> Thanks for the comments. I used 'rollover' instead of 'overflow' because
> I think of 'overflow' as something that causes a SIGFPE.

Integer overflow never causes SIGFPE. If you grep for overflow in the 
source code you will see it is quite extensively used.

http://en.wikipedia.org/wiki/Integer_overflow

I think of "rollover" as something your bank does with a term deposit 
when you don't withdraw it at the end of the term. ;-)

David
-----

> Also, I thought that "StringTable size" was perhaps a bit more
> understandable because the user is specifying the size of the
> StringTable, but maybe it's not more understandable?
>
> Thanks again,
> Harold
>
> On 11/10/2012 7:43 PM, David Holmes wrote:
>> On 10/11/2012 5:11 AM, harold seigel wrote:
>>> Please review the following change to fix bug 7122219.
>>>
>>> Summary: This change prevents the SIGFPE and SIGSEGV problems that can
>>> occur when using this option. It checks that the specified value is >=
>>> the current default (1009) and <= the maximum requestable heap size, and
>>> issues an error if it is not.
>>>
>>> This webrev also deletes an unused create_table() method and adds a
>>> check for integer rollover to method os::malloc().
>>
>> Minor nit: we generally refer to overflow rather than rollover.
>>
>>> With this change, if users request an invalid value, they will get an
>>> error message like this:
>>>
>>> % $JAVA_HOME/bin/java -XX:StringTableSize=1g -cp .. <java-main-class>
>>> StringTable size of 1073741824 is invalid; must be between 1009 and
>>
>> Shouldn't that be "StringTableSize of ..."
>>
>> Was the option of renaming StringTableSize to something reflecting
>> that it is actually the bucket count rejected? I know there is a
>> follow on CR in this area but giving this variable a more accurate
>> name doesn't seem unreasonable.
>>
>> That said I'm okay if this is pushed as-is.
>>
>> David
>>
>>> 536870911
>>> Error: Could not create the Java Virtual Machine.
>>> Error: A fatal exception has occurred. Program will exit.
>>
>>
>>
>>> Open webrev at http://cr.openjdk.java.net/~hseigel/bug_7122219
>>> <http://cr.openjdk.java.net/%7Ehseigel/bug_7122219>
>>> Bug link at http://bugs.sun.com/view_bug.do?bug_id=7122219
>>>
>>> The changes were tested with JCK, JPRT, JTREG, and UTE tests, and with
>>> hand-run tests on 32-bit Linux and 64-bit Windows.