[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd
From:       Kevin Walls <kevin.walls () oracle ! com>
Date:       2016-05-24 16:46:55
Message-ID: 3c84643c-c10a-7eaa-d28e-83fa0ee037de () oracle ! com
[Download RAW message or body]


...although we consider this a fix, more than a feature, and hope it 
isn't excluded from being pushed.

---
Kevin



On 24/05/2016 17:24, Gerard Ziemski wrote:
> 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@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@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@openjdk.java.net; \
> >                 hotspot-runtime-dev@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@openjdk.java.net; \
> >                 hotspot-runtime-dev@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@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@openjdk.java.net; hotspot-runtime-
> > dev@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@openjdk.java.net; serviceability-
> > dev@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@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


[prev in list] [next in list] [prev in thread] [next in thread]