RFR 8153783: UnlockDiagnosticVMOptions should not be trueInDebug

[Vladimir Ivanov]

As for me, I'd prefer to leave it as is.

I use debug builds on daily basis and requiring to specify 
-XX:+UnlockDiagnosticVMOptions would add observable overhead
into my development workflow.

IMO missing -XX:+UnlockDiagnosticVMOptions in a test manifests more 
serious issue it is proposed to fix: the test was never run with product 
binaries and we shouldn't try to hide that by unifying flag's default 
value between product & debug binaries.

And it's not a theoretical possibility: I regularly run into slight 
differences in behavior between product & fastdebug binaries while 
writing (and testing externally contributed) JIT-compiler regression 
tests which cause failures.

Best regards,
Vladimir Ivanov

On 1/26/18 1:08 PM, coleen.phillimore at oracle.com wrote:
> 
> Thanks Dan, Lois and Harold for the code reviews.
> 
> We're still trying to answer the meta-question of whether we want to use 
> this option in day to day development, when using flags like 
> VerifyDuringGC, and PrintNMethods, vs. having diagnostic options added 
> to tests or sent to other parties missing the necessary 
> -XX:+UnlockDiagnosticVMOptions.
> 
> Thanks,
> Coleen
> 
> 
> On 1/26/18 3:19 PM, Daniel D. Daugherty wrote:
>> On 1/26/18 11:44 AM, coleen.phillimore at oracle.com wrote:
>>> Summary: Make default false in all builds like 
>>> UnlockExperimentalVMOptions and fix tests.
>>>
>>> Ran all jtreg hotspot tests and jdk/java/lang tests, and sun.misc. 
>>> There's some compiler test that I fixed. Please check if I did this 
>>> right:
>>> TestPrintPreciseRTMLockingStatisticsBase.java.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8153783.01/webrev
>>
>> src/hotspot/share/runtime/globals.hpp
>>     No comments.
>>
>> test/hotspot/jtreg/compiler/rtm/cli/TestPrintPreciseRTMLockingStatisticsBase.java 
>>
>>     No comments. Messy test, but I think it is correct.
>>
>> test/hotspot/jtreg/runtime/Unsafe/RangeCheck.java
>>     No comments.
>>
>> test/hotspot/jtreg/runtime/appcds/SharedArchiveFile.java
>>     No comments.
>>
>> test/hotspot/jtreg/runtime/handshake/HandshakeTransitionTest.java
>>     No comments.
>>
>> Thumbs up on the code!
>>
>> The meta question is, of course, that this changes things for folks that
>> use non-product builds on a daily basis. Personally, I like the idea of
>> removing one more annoying difference between 'release' and 'fastdebug'
>> bits.
>>
>> Yes, that means I have to remember to to include 
>> -XX:+UnlockDiagnosticVMOptions
>> but I typically run things from scripts so I've been training myself 
>> to always
>> include that option when I'm setting a script for bug hunt. So this 
>> change in
>> behavior isn't a big deal for me.
>>
>> Dan
>>
>>
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8153783
>>>
>>> Thanks,
>>> Coleen
>>>
>>
>