RFR (S) 8227123: Assertion failure when setting SymbolTableSize larger than 2^17 (131,072)

[coleen.phillimore at oracle.com]

On 7/23/19 9:45 AM, Daniel D. Daugherty wrote:
> On 7/23/19 7:03 AM, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 7/23/19 12:27 AM, David Holmes wrote:
>>> Hi Coleen,
>>>
>>> -  experimental(bool, UnlockExperimentalVMOptions, false, \
>>> +  experimental(bool, UnlockExperimentalVMOptions, trueInDebug,     \
>>>
>>> I can't quite convince myself this is harmless nor necessary.
>>
>> Well if it's added, then the option range test would test the 
>> option.  Otherwise, I think it's benign.  In debug mode, one would no 
>> longer have to specify -XX:+UnlockExperimental options, just like 
>> UnlockDiagnosticVMOptions.   The option is there either way.
>
> Mentioning 'UnlockDiagnosticVMOptions' reminds me that some folks think
> that 'UnlockDiagnosticVMOptions' being 'trueInDebug' can cause bugs in 
> tests
> that are runnable in all build configs: 'release', 'fastdebug' and 
> 'slowdebug'.
> Folks use an option in a test that requires 
> '-XX:+UnlockDiagnosticVMOptions',
> but forget to include it in the test's run statement and we end up with a
> test failure in 'release' bits.
>
> I would prefer that 'UnlockExperimentalVMOptions' did not introduce 
> the same
> path to failing tests.

I tried to change UnlockDiagnosticVMOptions to be false, and got a wall 
of opposition:

See: https://bugs.openjdk.java.net/browse/JDK-8153783

http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-January/029882.html

I think the same exact arguments should apply to 
UnlockExperimentalVMOptions.  I'd like to hear from someone that uses 
experimental options on ZGC or shenandoah, since those have the most 
experimental options.

The reason that I made it trueInDebug is so that the command line option 
range test would test these options.  Otherwise a more hacky solution 
could be done, including adding the parameter 
-XX:+UnlockExperimentalVMOptions to all the VM option range tests. I'd 
rather not do this.

Thanks,
Coleen

>
> Dan
>
>
>>>
>>> Functional change seems fine. Is it worth adding a clarifying 
>>> comment to:
>>>
>>> +          range(minimumSymbolTableSize, 16777216ul)     \
>>>
>>> with:
>>>
>>> +          range(minimumSymbolTableSize, 16777216ul /* 2^24 */) 
>>>                \
>>
>> Let me see if the X macro allows that and I could also add that to 
>> StringTableSize (which is not experimental option).
>> Thanks,
>> Coleen
>>>
>>> Thanks,
>>> David
>>>
>>> On 23/07/2019 4:45 am, coleen.phillimore at oracle.com wrote:
>>>> Summary: Increase max size for SymbolTable and fix experimental 
>>>> option range.  Make experimental options trueInDebug so they're 
>>>> tested by the command line option testing
>>>>
>>>> open webrev at 
>>>> http://cr.openjdk.java.net/~coleenp/2019/8227123.01/webrev
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8227123
>>>>
>>>> Tested locally with default and -XX:+UseZGC since ZGC has a lot of 
>>>> experimental options.  I didn't test with shenanodoah.
>>>>
>>>> I will test with hs-tier1-3 before checking in.
>>>>
>>>> Thanks,
>>>> Coleen
>>
>