RFR 8153783: UnlockDiagnosticVMOptions should not be trueInDebug

[Stefan Karlsson]

On 2018-01-26 22:41, Vladimir Ivanov wrote:
> 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.

I agree.

StefanK

>
> 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
>>>>
>>>
>>