RFR 8153783: UnlockDiagnosticVMOptions should not be trueInDebug

[jesper.wilhelmsson at oracle.com]

> On 26 Jan 2018, at 22:58, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> 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.

+1
/Jesper


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