RFR[9u-dev]: 8150900: Implement diagnostic_pd

[Gerard Ziemski]

hi Cheleswer,

The change looks good.

My understanding is that the jdk9 repo is now closed to features, so you will need an exception to push this fix - jesper.wilhelmsson at oracle.com told me yesterday that the process for that is not ready/documented yet.


cheers 


> On May 24, 2016, at 6:49 AM, Cheleswer Sahu <cheleswer.sahu at oracle.com> wrote:
> 
> Hi,
>  
> I just wanted to let you know that since review there has been one new file added  “commandLineFlagWriteableList.cpp”,  and this files also needs to be modified/updated for implementing “diagnostic_pd”. This is just one additional change over what was reviewed before,  so I am  going ahead with this fix and need not a new review. I have tested this change and its working fine as before.
>  
> Webrev link: http://cr.openjdk.java.net/~csahu/8150900/webrev.01/
>  
> Regards, 
> Cheleswer
>  
> From: Cheleswer Sahu 
> Sent: Wednesday, May 11, 2016 2:29 PM
> To: Christian Thalinger
> Cc: Kevin Walls; Gerard Ziemski; serviceability-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
> Subject: RE: RFR[9u-dev]: 8150900: Implement diagnostic_pd
>  
> Thanks Christian for review. I will correct the alignment.
>  
> Regards,
> Cheleswer
>  
> From: Christian Thalinger 
> Sent: Wednesday, May 11, 2016 1:00 AM
> To: Cheleswer Sahu
> Cc: Kevin Walls; Gerard Ziemski; serviceability-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd
>  
> src/share/vm/runtime/globals.hpp
> 
> -  develop_pd(bool, ImplicitNullChecks,                                      \
> +  diagnostic_pd(bool, ImplicitNullChecks,                                      \
>            "Generate code for implicit null checks")                         \
> Align the \
>  
> On May 10, 2016, at 1:47 AM, Cheleswer Sahu <cheleswer.sahu at oracle.com> wrote:
>  
> Hi, 
> I need one reviewer (R) to review these changes before pushing in JDK9.  Can somebody please review the changes.
> 
> Regards,
> Cheleswer
> 
> 
> -----Original Message-----
> From: Kevin Walls
> Sent: Friday, May 06, 2016 3:53 PM
> To: Cheleswer Sahu; Gerard Ziemski
> Cc: serviceability-dev at openjdk.java.net; hotspot-runtime-
> dev at openjdk.java.net
> Subject: Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd
> 
> 
> Thanks Cheleswer, looks good to me too, have been over the macros as
> much as I can!
> 
> Thanks
> Kevin
> 
> 
> 
> On 03/05/2016 07:34, Cheleswer Sahu wrote:
> 
> Hi Gerard,
> 
> 
> 
> -----Original Message-----
> From: Gerard Ziemski
> Sent: Monday, May 02, 2016 9:07 PM
> To: Cheleswer Sahu
> Cc: hotspot-runtime-dev at openjdk.java.net; serviceability-
> dev at openjdk.java.net
> Subject: Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd
> 
> hi Cheleswer,
> 
> 
> #1 Shouldn’t the following files be modified as well? :
> 
> open:
> 
> src/cpu/sparc/vm/globals_sparc.hpp
> src/cpu/x86/vm/globals_x86.hpp
> src/cpu/zero/vm/globals_zero.hpp
> 
> closed:
> 
> cpu/arm/vm/globals_arm.hpp
> I have implemented  "diagnostic_pd" using "product_pd" as a reference
> implementation. "product_pd" is not implemented for " ARCH_FLAGS ",
> therefore I have also not implemented "diagnostic_pd" for "ARCH_FLAGS"
> type.
> 
>  
> 
> share/vm/runtime/globals_ext.hpp
> share/vm/runtime/os_ext.hpp
> These 2 files are under closed repository, so I have initiated a separate
> internal review request for those changes.
> 
>  
> 
> 
> #2 Bunch of header files need to be updated with 2016 for Copyright:
> 
> /*
> - * Copyright (c) 2011, 2015, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved.
>  * ORACLE PROPRIETARY/CONFIDENTIAL.  Use is subject to license terms.
>  */
> 
> I agree, I will update the copyright headers.
> 
> 
> #3 What tests have you run? Did you do:
> 
> a) JPRT hotspot
> b) RBT hs-nightly-runtime
> 
> I have run JPRT hostspot tests for this. It shows no error.
> 
> 
> Please email me if you need help with those.
> 
> 
> #4 Just heads up that I will be shortly asking for review of my fix
> (https://bugs.openjdk.java.net/browse/JDK-8073500), which touches
> many of the same file, so one of us will have a tricky merge
> 
> Thanks for informing about this.
> 
> 
> Regards,
> Cheleswer
> 
> 
> cheers
> 
> 
> On May 2, 2016, at 4:51 AM, Cheleswer Sahu
> <cheleswer.sahu at oracle.com> wrote:
> 
> Hi,
> 
> 
> 
> Please review the code changes for
> https://bugs.openjdk.java.net/browse/JDK-8150900.
> 
> 
> 
> Webrev Link: http://cr.openjdk.java.net/~csahu/8150900/webrev.00/
> 
> 
> 
> Enhancement Brief:  A new variant of flag "diagnostic_pd" is
> implemented.
> 
> All flags which are diagnostic in nature and platform dependent can
> be placed
> 
> under this variant. These flags can be enable using  "-
> XX:+UnlockDiagnosticVMOptions".
> 
> At present I have placed 4 flags under "diagnostic_pd"
> 
> 1.        1. InitArrayShortSize
> 
> 2.        2. ImplicitNullChecks
> 
> 3.        3. InlineFrequencyCount
> 
> 4.        4. PostLoopMultiversioning
> 
> 
> 
> 
> 
> Regards,
> 
> Cheleswer