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

[Daniel D. Daugherty]

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.

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
>