RFR (L) 8213501 : Deploy ExceptionJniWrapper for a few tests

Jean Christophe Beyler jcbeyler at google.com
Thu Apr 11 01:18:26 UTC 2019


Hi Paul,

Exactly :). Actually you are right, when trying to figure out build errors
in windows/solaris, I moved declarations around.

I've moved them back and the incremental is thus:
http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06_08/

And the full new version is:
http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.08/

This has passed the submit repo (thanks serguei ;-)).

Could I get another LGTM for this and then we can move forward ? :)

Thanks for your help,
Jc


On Mon, Apr 8, 2019 at 9:11 AM Hohensee, Paul <hohensee at amazon.com> wrote:

> Almost all copyright changes, plus a bit of code reorg, but no real
> implementation changes except in ExceptionCheckingJniEnv.cpp, which are
> fine. A quibble: I’d have left declarations located at first use instead of
> moving them up to the beginning of their methods, but I’m not doctrinaire,
> so they’re fine too.
>
>
>
> Paul
>
>
>
> *From: *Jean Christophe Beyler <jcbeyler at google.com>
> *Date: *Tuesday, April 2, 2019 at 10:00 AM
> *To: *Alex Menkov <alexey.menkov at oracle.com>
> *Cc: *"Hohensee, Paul" <hohensee at amazon.com>, hotspot-dev Source
> Developers <hotspot-dev at openjdk.java.net>
> *Subject: *Re: RFR (L) 8213501 : Deploy ExceptionJniWrapper for a few
> tests
>
>
>
> Hi all,
>
>
>
> Friendly ping on this one, I know I've used up quite a bit of time on it;
> my apologies again :)
>
> Jc
>
>
>
> On Tue, Mar 26, 2019 at 12:37 PM Jean Christophe Beyler <
> jcbeyler at google.com> wrote:
>
> Hi all,
>
>
>
> My apologies again for this one. This has been a bit tricky to get in and
> has fell off my priority list due to other issues/commitments but I'm back
> to finishing this up and the next webrevs regarding this work :)
>
>
>
> Problem is that we change years so the copyrights changed on some of these
> and there were a few problems with various architectures/build systems that
> made the testing fail on the submit repo.
>
>
>
> So I offer two webrevs:
>
>
>
> - The incremental from the last LGTM stamped one:
>
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06_07/
>
>
>
> - The full webrev that cleaned up a few problems for windows and solaris
> and now passes the submit repo:
>
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.07/
>
>
>
> And again, my sincere apologies that it took me SO long to get back to
> this but I had to work through the random submit repo failures and some of
> them took time for me to debug (thanks Serguei for your help :-)),
>
> Jc
>
>
>
> On Tue, Jan 22, 2019 at 6:03 PM Alex Menkov <alexey.menkov at oracle.com>
> wrote:
>
> +1
>
> --alex
>
> On 01/22/2019 10:29, JC Beyler wrote:
> > Thanks Paul!
> >
> > Anybody else for the review for version 6?
> >
> > Webrev: http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
> > <http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/>
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
> > <https://bugs.openjdk.java.net/browse/JDK-8213501>
> >
> > Thanks,
> > Jc
> >
> > On Tue, Jan 22, 2019 at 6:10 AM Hohensee, Paul <hohensee at amazon.com
> > <mailto:hohensee at amazon.com>> wrote:
> >
> >     Lgtm :)
> >
> >     Paul
> >
> >     On 1/14/19, 7:46 AM, "hotspot-dev on behalf of JC Beyler"
> >     <hotspot-dev-bounces at openjdk.java.net
> >     <mailto:hotspot-dev-bounces at openjdk.java.net> on behalf of
> >     jcbeyler at google.com <mailto:jcbeyler at google.com>> wrote:
> >
> >          Hi all,
> >
> >          Friendly ping on this one, I know that it has been a long
> >     process with back
> >          and forths, to which I apologize...
> >
> >          But is there any way I could get a final LGTM for version 6?
> >
> >          Webrev: http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
> >          Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
> >
> >          Thanks!
> >          Jc
> >
> >          On Tue, Jan 8, 2019 at 10:05 AM JC Beyler <jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>> wrote:
> >
> >          > Happy new year all!
> >          >
> >          > Could I get a final LGTM for version 6?
> >          >
> >          > Webrev:
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
> >          > Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >
> >          > Thanks!
> >          > Jc
> >          >
> >          > On Mon, Dec 17, 2018 at 8:43 AM JC Beyler
> >     <jcbeyler at google.com <mailto:jcbeyler at google.com>> wrote:
> >          >
> >          >> Hi all,
> >          >>
> >          >> I don't believe I got actual LGTM for this version:
> >          >>
> >          >>
> >          >> Webrev:
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
> >          >> Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >>
> >          >>
> >          >> It removed the namespaces and uses explicit static instead :)
> >          >>
> >          >> Thanks!
> >          >> Jc
> >          >>
> >          >> On Wed, Dec 12, 2018 at 8:06 PM JC Beyler
> >     <jcbeyler at google.com <mailto:jcbeyler at google.com>> wrote:
> >          >>
> >          >>> So did I Alexey but with David & Serguei preferring static,
> >     it seems
> >          >>> more reasonable to go down their route :-)
> >          >>>
> >          >>> So here is the latest webrev with static instead of an
> >     anonymous
> >          >>> namespace:
> >          >>>
> >          >>> Webrev:
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.06/
> >          >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >>>
> >          >>> Let me know what you think, can I get a webrev 06 review?
> >          >>>
> >          >>> Thanks!
> >          >>> Jc
> >          >>>
> >          >>> On Wed, Dec 12, 2018 at 3:10 PM Alex Menkov
> >     <alexey.menkov at oracle.com <mailto:alexey.menkov at oracle.com>>
> >          >>> wrote:
> >          >>>
> >          >>>> Hm..
> >          >>>> I considered unnamed namespaces "C++ style" (and static
> >     globals as "C
> >          >>>> style").
> >          >>>> Static globals were deprecated in C++ (but some time ago
> the
> >          >>>> deprecation
> >          >>>> was reverted).
> >          >>>>
> >          >>>> --alex
> >          >>>>
> >          >>>> On 12/12/2018 13:55, serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com> wrote:
> >          >>>> > Agreed.
> >          >>>> >
> >          >>>> > Thanks,
> >          >>>> > Serguei
> >          >>>> >
> >          >>>> >
> >          >>>> > On 12/12/18 13:52, David Holmes wrote:
> >          >>>> >> FWIW I think namespaces are overkill in all of this
> >     test code and
> >          >>>> just
> >          >>>> >> obfuscates things - the declaration is easily missed. A
> >     static
> >          >>>> >> variable in a .cpp is clearly a global variable to the
> >     file.
> >          >>>> >>
> >          >>>> >> Cheers,
> >          >>>> >> David
> >          >>>> >>
> >          >>>> >>
> >          >>>> >>
> >          >>>> >> On 13/12/2018 5:37 am, serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com> wrote:
> >          >>>> >>> Hi Jc,
> >          >>>> >>>
> >          >>>> >>>
> >          >>>> >>> On 12/11/18 21:16, JC Beyler wrote:
> >          >>>> >>>> Hi all,
> >          >>>> >>>>
> >          >>>> >>>> Here is the new webrev with the TEST.groups change.
> >     Serguei, let
> >          >>>> me
> >          >>>> >>>> know if I convinced you with the static vs anonymous
> >     namespaces or
> >          >>>> >>>> if you'd still rather have a "static" for now :-)
> >          >>>> >>>
> >          >>>> >>>
> >          >>>> >>> What do you think about this post? :
> >          >>>> >>>
> >          >>>>
> >
> https://stackoverflow.com/questions/11623451/static-vs-non-static-variables-in-namespace
> >          >>>> >>>
> >          >>>> >>>
> >          >>>> >>>>
> >          >>>> >>>> Webrev:
> >     http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.05/
> >          >>>> >>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.05/>
> >          >>>> >>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >>>> >>>
> >          >>>> >>> The update looks fine.
> >          >>>> >>>
> >          >>>> >>> Thanks,
> >          >>>> >>> Serguei
> >          >>>> >>>
> >          >>>> >>>
> >          >>>> >>> Thanks,
> >          >>>> >>> Serguei
> >          >>>> >>>
> >          >>>> >>>>
> >          >>>> >>>> Thanks again for the reviews!
> >          >>>> >>>> Jc
> >          >>>> >>>>
> >          >>>> >>>> On Mon, Dec 10, 2018 at 3:10 PM JC Beyler
> >     <jcbeyler at google.com <mailto:jcbeyler at google.com>
> >          >>>> >>>> <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>>> wrote:
> >          >>>> >>>>
> >          >>>> >>>>     Hi Serguei,
> >          >>>> >>>>
> >          >>>> >>>>     Yes basically it is equivalent :) I can put them
> >     in but they
> >          >>>> are
> >          >>>> >>>>     not required. The norm actually wanted to
> >     deprecate it but then
> >          >>>> >>>>     remembered that C compatibility would require the
> >     static
> >          >>>> key-word
> >          >>>> >>>>     for this case [1]
> >          >>>> >>>>
> >          >>>> >>>>     So, really, they are not required here and will
> >     amount to the
> >          >>>> same
> >          >>>> >>>>     thing: only that file can refer to them and you
> >     cannot get to
> >          >>>> them
> >          >>>> >>>>     without a globally available method to return a
> >     pointer to them
> >          >>>> >>>>     (ie same as a static variable in C).
> >          >>>> >>>>
> >          >>>> >>>>     I can put static if it makes it easier to see
> >     but, by being in
> >          >>>> an
> >          >>>> >>>>     anonymous namespace they are only available for
> >     the file's
> >          >>>> >>>>     translation unit. For example:
> >          >>>> >>>>
> >          >>>> >>>>     $ cat main.cpp
> >          >>>> >>>>
> >          >>>> >>>>     int totally_global;
> >          >>>> >>>>     static int explictly_static;
> >          >>>> >>>>
> >          >>>> >>>>     namespace {
> >          >>>> >>>>     int implicitly_static;
> >          >>>> >>>>     }
> >          >>>> >>>>
> >          >>>> >>>>     void foo();
> >          >>>> >>>>     int main() {
> >          >>>> >>>>       foo();
> >          >>>> >>>>     }
> >          >>>> >>>>
> >          >>>> >>>>     $ g++ -O3 main.cpp -c
> >          >>>> >>>>     $ nm main.o
> >          >>>> >>>>                      U _GLOBAL_OFFSET_TABLE_
> >          >>>> >>>>     0000000000000000 T main
> >          >>>> >>>>     0000000000000000 B totally_global
> >          >>>> >>>>                      U _Z3foov
> >          >>>> >>>>
> >          >>>> >>>>     As you can see, the static and anonymous
> >     namespace variables
> >          >>>> are
> >          >>>> >>>>     not in the file due to not being used. If you
> >     were to use them,
> >          >>>> >>>>     you'd see them show up as something like:
> >          >>>> >>>>     0000000000000008 b _ZL17explicitly_static
> >          >>>> >>>>     0000000000000004 b
> >     _ZN12_GLOBAL__N_117implicitly_staticE
> >          >>>> >>>>
> >          >>>> >>>>     Where again, it shows that it is mangling the
> >     names so that no
> >          >>>> >>>>     external usage can happen without tinkering.
> >          >>>> >>>>
> >          >>>> >>>>     Hopefully that helps :-),
> >          >>>> >>>>     Jc
> >          >>>> >>>>
> >          >>>> >>>>     [1]
> >          >>>> >>>>
> >     http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1012
> >          >>>> >>>>
> >          >>>> >>>>
> >          >>>> >>>>     On Mon, Dec 10, 2018 at 2:04 PM
> >     serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>
> >          >>>> >>>>     <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>> <
> >          >>>> serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>     <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>> wrote:
> >          >>>> >>>>
> >          >>>> >>>>         Hi Jc,
> >          >>>> >>>>
> >          >>>> >>>>         I had little experience with the C++
> namespaces.
> >          >>>> >>>>         My understanding is that static in this
> >     context should mean
> >          >>>> >>>>         internal linkage.
> >          >>>> >>>>
> >          >>>> >>>>         Thanks,
> >          >>>> >>>>         Serguei
> >          >>>> >>>>
> >          >>>> >>>>
> >          >>>> >>>>         On 12/10/18 13:57, JC Beyler wrote:
> >          >>>> >>>>>         Hi Serguei,
> >          >>>> >>>>>
> >          >>>> >>>>>         The variables and functions are in a
> >     anonymous namespace;
> >          >>>> my
> >          >>>> >>>>>         understanding of C++ is that this is
> >     equivalent to
> >          >>>> putting it
> >          >>>> >>>>>         as static.Hence, I didn't add them there.
> >     Does that make
> >          >>>> >>>>> sense?
> >          >>>> >>>>>
> >          >>>> >>>>>         Thanks!
> >          >>>> >>>>>         Jc
> >          >>>> >>>>>
> >          >>>> >>>>>         On Mon, Dec 10, 2018 at 1:33 PM
> >          >>>> serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>         <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>
> >          >>>> >>>>>         <serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>
> >          >>>> >>>>>         <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>> wrote:
> >          >>>> >>>>>
> >          >>>> >>>>>             Hi Jc,
> >          >>>> >>>>>
> >          >>>> >>>>>             It looks good in general.
> >          >>>> >>>>>             One question though.
> >          >>>> >>>>>
> >          >>>> >>>>>
> >          >>>>
> >
> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03a_04/test/hotspot/jtreg/vmTestbase/nsk/share/ExceptionCheckingJniEnv/exceptionjni001/exceptionjni001.cpp.html
> >          >>>> >>>>>
> >          >>>> >>>>>
> >          >>>> >>>>>             I wonder if the variables and functions
> >     have to be
> >          >>>> static.
> >          >>>> >>>>>
> >          >>>> >>>>>             Thanks,
> >          >>>> >>>>>             Serguei
> >          >>>> >>>>>
> >          >>>> >>>>>
> >          >>>> >>>>>             On 12/5/18 11:36, JC Beyler wrote:
> >          >>>> >>>>>>             Hi all,
> >          >>>> >>>>>>
> >          >>>> >>>>>>             My apologies to having to come back for
> >     another
> >          >>>> review
> >          >>>> >>>>>>             for this change: I ran into a snag when
> >     trying to
> >          >>>> pull
> >          >>>> >>>>>>             the latest changes compared to the base
> >     I was working
> >          >>>> >>>>>>             on. I basically forgot that there was
> >     an issue with
> >          >>>> >>>>>>             snprintf and that I had solved it via
> >     JDK-8213622.
> >          >>>> >>>>>>
> >          >>>> >>>>>>             Could I have a new review of this
> webrev:
> >          >>>> >>>>>>             Webrev:
> >          >>>> >>>>>>
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.04/
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.04/>
> >          >>>> >>>>>>             Bug:
> >          >>>> https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >>>> >>>>>>             Incremental from the port of webrev.03
> >     that got
> >          >>>> LGTMs:
> >          >>>> >>>>>>
> >     http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.03a_04/
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03a_04/>
> >          >>>> >>>>>>
> >          >>>> >>>>>>             A few comments on this because it took
> >     me a while to
> >          >>>> get
> >          >>>> >>>>>>             things in a state I thought was good:
> >          >>>> >>>>>>               - I had to implement an itoa method,
> >     do we have
> >          >>>> >>>>>>             something like that in the test base
> >     (remember that
> >          >>>> >>>>>>             JDK-8213622 could not use sprintf due
> >     to being in the
> >          >>>> >>>>>>             test code)?
> >          >>>> >>>>>>
> >          >>>> >>>>>>               - The differences here compared to
> >     the one you all
> >          >>>> >>>>>>             reviewed are:
> >          >>>> >>>>>>                   - I found that adding to the
> >     strlen/memcpy
> >          >>>> error
> >          >>>> >>>>>>             prone and thought that I would try to
> >     make it less
> >          >>>> so.
> >          >>>> >>>>>>             If you want to compare, I extended the
> >     strlen/memcpy
> >          >>>> >>>>>>             with the new format to show you if you
> >     prefer [1]
> >          >>>> >>>>>>                         - Note that the diff
> >     between the "old
> >          >>>> >>>>>>             extended way from [1]" to the webrev.04
> >     can be found
> >          >>>> >>>>>> in [2]
> >          >>>> >>>>>>
> >          >>>> >>>>>>                  - I added a test to test the
> >     exception wrapper
> >          >>>> in
> >          >>>> >>>>>>             tests :); I'm not sure it is deemed
> >     useful or not but
> >          >>>> >>>>>>             helped me assure myself that I was not
> >     doing things
> >          >>>> >>>>>>             wrong; you can find the base test file
> >     here [3];
> >          >>>> should
> >          >>>> >>>>>>             we have this or not? (I know that
> >     normally we don't
> >          >>>> add
> >          >>>> >>>>>>             tests to vmTestbase but thought this
> >     might be an
> >          >>>> >>>>>> exception)
> >          >>>> >>>>>>
> >          >>>> >>>>>>             Thanks for your help and my apologies
> >     for the snag,
> >          >>>> >>>>>>             Jc
> >          >>>> >>>>>>
> >          >>>> >>>>>>             [1]:
> >          >>>> >>>>>>
> >          >>>>
> >
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.03a/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.cpp.udiff.html
> >          >>>> >>>>>>
> >          >>>> >>>>>> <
> >          >>>>
> >
> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03a/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.cpp.udiff.html
> >
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>             [2]:
> >          >>>> >>>>>>
> >     http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.03a_04
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03a_04>
> >          >>>> >>>>>>             [3]
> >          >>>> >>>>>>
> >          >>>>
> >
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.04/test/hotspot/jtreg/vmTestbase/nsk/share/ExceptionCheckingJniEnv/exceptionjni001/exceptionjni001.cpp.html
> >          >>>> >>>>>>
> >          >>>> >>>>>> <
> >          >>>>
> >
> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.04/test/hotspot/jtreg/vmTestbase/nsk/share/ExceptionCheckingJniEnv/exceptionjni001/exceptionjni001.cpp.html
> >
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>             On Mon, Dec 3, 2018 at 11:29 PM David
> >     Holmes
> >          >>>> >>>>>>             <david.holmes at oracle.com
> >     <mailto:david.holmes at oracle.com>
> >          >>>> >>>>>>             <mailto:david.holmes at oracle.com
> >     <mailto:david.holmes at oracle.com>>> wrote:
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 Looks fine to me.
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 Thanks,
> >          >>>> >>>>>>                 David
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 On 4/12/2018 4:04 pm, JC Beyler
> wrote:
> >          >>>> >>>>>>                 > Hi both,
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > Thanks for the reviews! Since
> >     Serguei did not
> >          >>>> >>>>>>                 insist on get_basename, I
> >          >>>> >>>>>>                 > went for get_dirname since the
> >     method is a
> >          >>>> local
> >          >>>> >>>>>>                 static method and won't
> >          >>>> >>>>>>                 > have its name start spreading, I
> >     think it's ok
> >          >>>> too.
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > For the naming of the local
> >     variable, the idea
> >          >>>> >>>>>>                 initially was to use the
> >          >>>> >>>>>>                 > same name as the local variable
> >     for JNIEnv
> >          >>>> already
> >          >>>> >>>>>>                 used to reduce the
> >          >>>> >>>>>>                 > code change. Since I'm now adding
> >     the line
> >          >>>> macro
> >          >>>> >>>>>>                 at the end anyway, this
> >          >>>> >>>>>>                 > does not matter anymore so I
> >     converged all
> >          >>>> local
> >          >>>> >>>>>>                 variables to "jni".
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > So, without further ado, here is
> >     the new
> >          >>>> version:
> >          >>>> >>>>>>                 > Webrev:
> >          >>>> >>>>>>
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.03/
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.03/>
> >          >>>> >>>>>>                 > Bug:
> >          >>>> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > This passes the various tests
> >     changed by the
> >          >>>> >>>>>>                 webrev on my dev machine.
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > Let me know what you think,
> >          >>>> >>>>>>                 > Jc
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > On Mon, Dec 3, 2018 at 8:40 PM
> >          >>>> >>>>>> serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>
> >          >>>> >>>>>>                 <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>
> >          >>>> >>>>>>                 >
> >     <mailto:serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>>
> >          >>>> >>>>>>                 <serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>
> >          >>>> >>>>>>                 <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>
> >          >>>> >>>>>>                 >
> >     <mailto:serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>>> wrote:
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >     On 12/3/18 20:15, Chris
> >     Plummer wrote:
> >          >>>> >>>>>>                 >      > Hi JC,
> >          >>>> >>>>>>                 >      >
> >          >>>> >>>>>>                 >      > Overall it looks good. A
> >     few naming nits
> >          >>>> >>>>>>                 thought:
> >          >>>> >>>>>>                 >      >
> >          >>>> >>>>>>                 >      > In bi01t001.cpp, why have
> >     you declared
> >          >>>> the
> >          >>>> >>>>>>                 >     ExceptionCheckingJniEnvPtr
> >          >>>> >>>>>>                 >      > using jni_env(jni).
> >     Elsewhere you use
> >          >>>> >>>>>>                 jni(jni_env) and rename the
> >          >>>> >>>>>>                 >      > method argument passed in
> >     from jni to
> >          >>>> >>>>>> jni_env.
> >          >>>> >>>>>>                 >      >
> >          >>>> >>>>>>                 >      > Related to this, I also
> >     noticed in some
> >          >>>> >>>>>>                 files that already are using
> >          >>>> >>>>>>                 >      >
> >     ExceptionCheckingJniEnvPtr, such as
> >          >>>> >>>>>>                 CharArrayCriticalLocker.cpp, you
> >          >>>> >>>>>>                 >      > delcared it as
> >     env(jni_env). So that
> >          >>>> means
> >          >>>> >>>>>>                 there are 3 different
> >          >>>> >>>>>>                 >     names
> >          >>>> >>>>>>                 >      > you have used for the
> >          >>>> >>>>>>                 ExceptionCheckingJniEnvPtr local
> >     variable.
> >          >>>> >>>>>>                 >     They
> >          >>>> >>>>>>                 >      > should be consistent.
> >          >>>> >>>>>>                 >      >
> >          >>>> >>>>>>                 >      > Also, can you rename
> >     get_basename() to
> >          >>>> >>>>>>                 get_dirname()? I know Serguei
> >          >>>> >>>>>>                 >      > suggested get_basename() a
> >     while back,
> >          >>>> but
> >          >>>> >>>>>>                 unless "basename" is
> >          >>>> >>>>>>                 >      > commonly used for this
> >     purpose, I think
> >          >>>> >>>>>>                 "dirname" is more self
> >          >>>> >>>>>>                 >      > explanatory.
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >     In general, I'm Okay with
> >     get_dirname().
> >          >>>> >>>>>>                 >     Just to mention dirname can
> >     be both short
> >          >>>> or
> >          >>>> >>>>>>                 full, so it is a little
> >          >>>> >>>>>>                 >     confusing as well.
> >          >>>> >>>>>>                 >     It is the reason why the
> >     get_basename() was
> >          >>>> >>>>>>                 suggested.
> >          >>>> >>>>>>                 >     However, I do not insist on
> >     get_basename()
> >          >>>> nor
> >          >>>> >>>>>>                 get_full_dirname(). :)
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >     Thanks,
> >          >>>> >>>>>>                 >     Serguei
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      > thanks,
> >          >>>> >>>>>>                 >      >
> >          >>>> >>>>>>                 >      > Chris
> >          >>>> >>>>>>                 >      >
> >          >>>> >>>>>>                 >      > On 12/2/18 10:29 PM, David
> >     Holmes wrote:
> >          >>>> >>>>>>                 >      >> Hi Jc,
> >          >>>> >>>>>>                 >      >>
> >          >>>> >>>>>>                 >      >> I've been lurking on this
> >     one and have
> >          >>>> had
> >          >>>> >>>>>>                 a look through. I'm okay
> >          >>>> >>>>>>                 >      >> with the FatalError
> >     approach for the
> >          >>>> tests
> >          >>>> >>>>>>                 - we don't expect
> >          >>>> >>>>>>                 >     anything
> >          >>>> >>>>>>                 >      >> to go wrong in a well
> >     written test in a
> >          >>>> >>>>>>                 correctly functioning VM.
> >          >>>> >>>>>>                 >      >>
> >          >>>> >>>>>>                 >      >> Thanks,
> >          >>>> >>>>>>                 >      >> David
> >          >>>> >>>>>>                 >      >>
> >          >>>> >>>>>>                 >      >>
> >          >>>> >>>>>>                 >      >>
> >          >>>> >>>>>>                 >      >> On 3/12/2018 3:24 pm, JC
> >     Beyler wrote:
> >          >>>> >>>>>>                 >      >>> Hi all,
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>> Would someone on the GC
> >     or runtime
> >          >>>> team
> >          >>>> >>>>>>                 be motivated to give
> >          >>>> >>>>>>                 >     this a
> >          >>>> >>>>>>                 >      >>> review? :)
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>> It would be much
> >     appreciated!
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>> Webrev:
> >          >>>> >>>>>>
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.02/
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/>
> >          >>>> >>>>>>                 >      >>> Bug:
> >          >>>> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>> Thanks for your help,
> >          >>>> >>>>>>                 >      >>> Jc
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>> On Tue, Nov 27, 2018 at
> >     4:36 PM JC
> >          >>>> Beyler
> >          >>>> >>>>>>                 <jcbeyler at google.com
> >     <mailto:jcbeyler at google.com> <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>
> >          >>>> >
> >          >>>> >>>>>>                 >     <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>
> >          >>>> >>>>>>                 <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>>>
> >          >>>> >>>>>>                 >      >>>
> >     <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>
> >          >>>> >>>>>>                 <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>>
> >          >>>> >>>>>>                 <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>
> >          >>>> >>>>>> <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>>>>> wrote:
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>     Hi Chris,
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>     Yes I was waiting
> >     for another
> >          >>>> review
> >          >>>> >>>>>>                 since you had explicitly
> >          >>>> >>>>>>                 >      >>> asked :)
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>     And sounds good that
> >     when someone
> >          >>>> >>>>>>                 from GC or runtime gives a
> >          >>>> >>>>>>                 >      >>> review,
> >          >>>> >>>>>>                 >      >>>     I'll wait for your
> >     full review on
> >          >>>> the
> >          >>>> >>>>>>                 webrev.02!
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>     Thanks again for
> >     your help,
> >          >>>> >>>>>>                 >      >>>     Jc
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>     On Tue, Nov 27, 2018
> >     at 12:48 PM
> >          >>>> >>>>>>                 Chris Plummer
> >          >>>> >>>>>>                 >      >>>
> >     <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>> <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>>
> >          >>>> >>>>>>                 >  <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>> <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>>>>
> >          >>>> >>>>>>                 >     wrote:
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>         Hi JC,
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>         I think it would
> >     be good to
> >          >>>> get a
> >          >>>> >>>>>>                 review from the gc or
> >          >>>> >>>>>>                 >     runtime
> >          >>>> >>>>>>                 >      >>>         teams, since
> >     this also affects
> >          >>>> >>>>>>                 their tests.
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>         Also, once we
> >     are settled on
> >          >>>> this
> >          >>>> >>>>>>                 FatalError approach,
> >          >>>> >>>>>>                 >     I still
> >          >>>> >>>>>>                 >      >>>         need to give
> >     your webrev-02 a
> >          >>>> >>>>>>                 full review. I only
> >          >>>> >>>>>>                 >     skimmed over
> >          >>>> >>>>>>                 >      >>>         parts of it (I
> >     did look at all
> >          >>>> >>>>>>                 the changes in webrevo-01).
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>         thanks,
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>         Chris
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>         On 11/27/18 8:58
> AM,
> >          >>>> >>>>>> serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>
> >          >>>> >>>>>>                 <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>
> >          >>>> >>>>>>                 >
> >     <mailto:serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>>
> >          >>>> >>>>>>                 >      >>>
> >     <mailto:serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>>                 <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>
> >          >>>> >>>>>>                 >
> >     <mailto:serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>>> wrote:
> >          >>>> >>>>>>                 >      >>>>         Hi Jc,
> >          >>>> >>>>>>                 >      >>>>
> >          >>>> >>>>>>                 >      >>>>         I've already
> >     reviewed this
> >          >>>> too.
> >          >>>> >>>>>>                 >      >>>>
> >          >>>> >>>>>>                 >      >>>>         Thanks,
> >          >>>> >>>>>>                 >      >>>>         Serguei
> >          >>>> >>>>>>                 >      >>>>
> >          >>>> >>>>>>                 >      >>>>
> >          >>>> >>>>>>                 >      >>>>         On 11/27/18
> >     06:56, JC Beyler
> >          >>>> >>>>>> wrote:
> >          >>>> >>>>>>                 >      >>>>> Thanks Chris,
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>> Anybody else motivated
> >     to look at
> >          >>>> this
> >          >>>> >>>>>>                 and review it? :)
> >          >>>> >>>>>>                 >      >>>>>         Jc
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>>         On Mon, Nov
> >     26, 2018 at
> >          >>>> 1:26 PM
> >          >>>> >>>>>>                 Chris Plummer
> >          >>>> >>>>>>                 >      >>>>>
> >     <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>
> >          >>>> >>>>>>                 >  <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>> <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>
> >          >>>> >>>>>>                 >  <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>> <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>>>>
> >          >>>> >>>>>>                 >      >>>>> wrote:
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>> Hi JC,
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>> I'm ok with the
> >     FatalError approach,
> >          >>>> >>>>>>                 but would
> >          >>>> >>>>>>                 >     like to
> >          >>>> >>>>>>                 >      >>>>> hear opinions from
> >     others also.
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>> thanks,
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>> Chris
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>> On 11/21/18 8:19 AM,
> >     JC Beyler
> >          >>>> wrote:
> >          >>>> >>>>>>                 >      >>>>>>             Hi Chris,
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             Thanks
> >     for taking the
> >          >>>> time
> >          >>>> >>>>>>                 to look at it and yes you
> >          >>>> >>>>>>                 >      >>>>>>             have
> >     raised exactly why
> >          >>>> >>>>>>                 the webrev is between two
> >          >>>> >>>>>>                 >      >>>>>>             worlds:
> >     in cases where
> >          >>>> a
> >          >>>> >>>>>>                 fatal error on failure is
> >          >>>> >>>>>>                 >      >>>>>>             wanted,
> >     should we
> >          >>>> simplify
> >          >>>> >>>>>>                 the code to remove
> >          >>>> >>>>>>                 >     the return
> >          >>>> >>>>>>                 >      >>>>>>             tests
> >     since we do them
> >          >>>> >>>>>>                 internally? Now that I've
> >          >>>> >>>>>>                 >     looked
> >          >>>> >>>>>>                 >      >>>>>>             around
> >     for non-fatal
> >          >>>> >>>>>>                 cases, I think the answer
> >          >>>> >>>>>>                 >     is yes,
> >          >>>> >>>>>>                 >      >>>>>>             it
> >     simplifies the code
> >          >>>> >>>>>>                 while maintaining the checks.
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             I looked
> >     a bit and it
> >          >>>> >>>>>>                 seems that I can't find
> >          >>>> >>>>>>                 >     easily a
> >          >>>> >>>>>>                 >      >>>>>>             case
> >     where the test
> >          >>>> >>>>>>                 accepts a JNI failure to
> >          >>>> >>>>>>                 >     then move
> >          >>>> >>>>>>                 >      >>>>>>             on.
> >     Therefore, perhaps,
> >          >>>> >>>>>>                 for now, the fail with a
> >          >>>> >>>>>>                 >     Fatal
> >          >>>> >>>>>>                 >      >>>>>>             is enough
> >     and we can
> >          >>>> work
> >          >>>> >>>>>>                 on the tests to clean
> >          >>>> >>>>>>                 >     them up?
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             That
> >     means that this is
> >          >>>> >>>>>>                 the new webrev with only
> >          >>>> >>>>>>                 >     Fatal
> >          >>>> >>>>>>                 >      >>>>>>             and
> >     cleans up the
> >          >>>> tests so
> >          >>>> >>>>>>                 that it is no longer in
> >          >>>> >>>>>>                 >      >>>>>>             between
> >     two worlds:
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             Webrev:
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.02/
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/>
> >          >>>> >>>>>>                 >      >>>>>>             Bug:
> >          >>>> >>>>>>                 >
> >          >>>> https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             (This
> >     passes testing
> >          >>>> on my
> >          >>>> >>>>>>                 dev machine for all the
> >          >>>> >>>>>>                 >      >>>>>>             modified
> >     tests)
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             with the
> >     example you
> >          >>>> >>>>>>                 provided, it now looks like:
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
> >          >>>> >>>>>>
> >          >>>> >>>>>> <
> >          >>>>
> >
> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
> >
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>  <
> >          >>>>
> >
> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
> >
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             Where it
> >     does, to me at
> >          >>>> >>>>>>                 least, seem cleaner and less
> >          >>>> >>>>>>                 >      >>>>>>             "noisy".
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             Let me
> >     know what you
> >          >>>> think,
> >          >>>> >>>>>>                 >      >>>>>>             Jc
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             On Tue,
> >     Nov 20, 2018 at
> >          >>>> >>>>>>                 9:33 PM Chris Plummer
> >          >>>> >>>>>>                 >      >>>>>>             <
> >          >>>> chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>
> >          >>>> >>>>>>                 >  <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>> <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>>
> >          >>>> >>>>>>                 > >>>>>>
> >     <mailto:chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>
> >          >>>> >>>>>>                 >  <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>> <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>>>> wrote:
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>                 Hi JC,
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>                 Sorry
> >     about the
> >          >>>> delay.
> >          >>>> >>>>>>                 I had to go back an
> >          >>>> >>>>>>                 >     look at
> >          >>>> >>>>>>                 >      >>>>>>                 the
> >     initial 8210842
> >          >>>> >>>>>>                 webrev and RFR thread to see
> >          >>>> >>>>>>                 >      >>>>>>                 what
> >     this was
> >          >>>> >>>>>>                 initially all about.
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>                 In
> >     general the
> >          >>>> changes
> >          >>>> >>>>>>                 look good.
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>                 I
> >     don't have a good
> >          >>>> >>>>>>                 answer to your
> >          >>>> >>>>>>                 > >>>>>> FatalError/NonFatalError
> >     question. It
> >          >>>> makes
> >          >>>> >>>>>>                 >     the code
> >          >>>> >>>>>>                 >      >>>>>>                 a lot
> >     cleaner to
> >          >>>> use
> >          >>>> >>>>>>                 FatalError, but then it
> >          >>>> >>>>>>                 >     is a
> >          >>>> >>>>>>                 >      >>>>>>
> >       behavior change,
> >          >>>> and
> >          >>>> >>>>>>                 you also need to deal with
> >          >>>> >>>>>>                 >      >>>>>>                 tests
> >     that
> >          >>>> >>>>>>                 intentionally induce errors (do
> >          >>>> >>>>>>                 >     you have
> >          >>>> >>>>>>                 >      >>>>>>                 an
> >     example of
> >          >>>> that).
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>                 In
> >     any case, right
> >          >>>> now
> >          >>>> >>>>>>                 your webrev seems to be
> >          >>>> >>>>>>                 >      >>>>>>
> >       between two worlds.
> >          >>>> >>>>>>                 You are producing
> >          >>>> >>>>>>                 >     FatalError,
> >          >>>> >>>>>>                 >      >>>>>>                 but
> >     still checking
> >          >>>> >>>>>>                 results. Here's a good
> >          >>>> >>>>>>                 >     example:
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
> >          >>>> >>>>>>
> >          >>>> >>>>>> <
> >          >>>>
> >
> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
> >
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>  <
> >          >>>>
> >
> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t003/ap04t003.cpp.frames.html
> >
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>                 I'm
> >     not sure if
> >          >>>> this
> >          >>>> >>>>>>                 is just a temporary
> >          >>>> >>>>>>                 >     state until
> >          >>>> >>>>>>                 >      >>>>>>                 it
> >     was decided
> >          >>>> which
> >          >>>> >>>>>>                 approach to take.
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
>  thanks,
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>                 Chris
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>                 On
> >     11/20/18 2:14
> >          >>>> PM,
> >          >>>> >>>>>>                 JC Beyler wrote:
> >          >>>> >>>>>>                 >      >>>>>>>                 Hi
> all,
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >       Chris thought it
> >          >>>> made
> >          >>>> >>>>>>                 sense to have more
> >          >>>> >>>>>>                 >     eyes on
> >          >>>> >>>>>>                 >      >>>>>>>                 this
> >     change than
> >          >>>> just
> >          >>>> >>>>>>                 serviceability as it will
> >          >>>> >>>>>>                 >      >>>>>>>
> >       modify to tests
> >          >>>> that
> >          >>>> >>>>>>                 are not only
> >          >>>> >>>>>>                 >     serviceability
> >          >>>> >>>>>>                 >      >>>>>>>
> >       tests so I've
> >          >>>> moved
> >          >>>> >>>>>>                 this to conversation
> >          >>>> >>>>>>                 >     here :)
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>                 For
> >     convenience,
> >          >>>> I've
> >          >>>> >>>>>>                 copy-pasted the
> >          >>>> >>>>>>                 >     initial RFR:
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >       Could I have a
> >          >>>> review
> >          >>>> >>>>>>                 for the extension and
> >          >>>> >>>>>>                 >     usage
> >          >>>> >>>>>>                 >      >>>>>>>                 of
> the
> >          >>>> >>>>>>                 ExceptionJniWrapper. This adds
> >     lines and
> >          >>>> >>>>>>                 >      >>>>>>>
> >       filenames to the
> >          >>>> end
> >          >>>> >>>>>>                 of the wrapper JNI
> >          >>>> >>>>>>                 >     methods,
> >          >>>> >>>>>>                 >      >>>>>>>                 adds
> >     tracing, and
> >          >>>> >>>>>>                 throws an error if need
> >          >>>> >>>>>>                 >     be. I've
> >          >>>> >>>>>>                 >      >>>>>>>
> >       ported the gc/lock
> >          >>>> >>>>>>                 files to use the new
> >          >>>> >>>>>>                 >      >>>>>>>
> >       TRACE_JNI_CALL
> >          >>>> add-on
> >          >>>> >>>>>>                 and I've ported a few
> >          >>>> >>>>>>                 >     of the
> >          >>>> >>>>>>                 >      >>>>>>>
> >       tests that were
> >          >>>> >>>>>>                 already changed for the
> >          >>>> >>>>>>                 >     assignment
> >          >>>> >>>>>>                 >      >>>>>>>
> >       webrev for
> >          >>>> >>>>>> JDK-8212884.
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
>  Webrev:
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.01
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.01>
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.01>
> >          >>>> >>>>>>                 >      >>>>>>>                 Bug:
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>                 For
> >     illustration,
> >          >>>> if
> >          >>>> >>>>>>                 I force an error to the
> >          >>>> >>>>>>                 >      >>>>>>>
> >       AP04/ap04t03 test
> >          >>>> and
> >          >>>> >>>>>>                 set the verbosity on,
> >          >>>> >>>>>>                 >     I get
> >          >>>> >>>>>>                 >      >>>>>>>
> >       something like:
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>                 >>
> >     Calling JNI
> >          >>>> method
> >          >>>> >>>>>>                 FindClass from
> >          >>>> >>>>>>                 >      >>>>>>> ap04t003.cpp:343
> >          >>>> >>>>>>                 >      >>>>>>>                 >>
> >     Calling with
> >          >>>> these
> >          >>>> >>>>>>                 parameter(s):
> >          >>>> >>>>>>                 >      >>>>>>> java/lang/Threadd
> >          >>>> >>>>>>                 >      >>>>>>>                 Wait
> >     for thread
> >          >>>> to
> >          >>>> >>>>>> finish
> >          >>>> >>>>>>                 >      >>>>>>>                 <<
> >     Called JNI
> >          >>>> method
> >          >>>> >>>>>>                 FindClass from
> >          >>>> >>>>>>                 >      >>>>>>> ap04t003.cpp:343
> >          >>>> >>>>>>                 >      >>>>>>>
> >       Exception in
> >          >>>> thread
> >          >>>> >>>>>>                 "Thread-0"
> >          >>>> >>>>>>                 >      >>>>>>>
> >     java.lang.NoClassDefFoundError:
> >          >>>> >>>>>>                 >     java/lang/Threadd
> >          >>>> >>>>>>                 >      >>>>>>>
> >         at
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
>  Method)
> >          >>>> >>>>>>                 >      >>>>>>>
> >         at
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >         at
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >       Caused by:
> >          >>>> >>>>>>                 java.lang.ClassNotFoundException:
> >          >>>> >>>>>>                 >      >>>>>>>
> >       java.lang.Threadd
> >          >>>> >>>>>>                 >      >>>>>>>
> >         at
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:583)
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >         at
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >         at
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>
> >     java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
> >          >>>> >>>>>>                 >      >>>>>>>
> >         ... 3 more
> >          >>>> >>>>>>                 >      >>>>>>>
> >       FATAL ERROR in
> >          >>>> native
> >          >>>> >>>>>>                 method: JNI method
> >          >>>> >>>>>>                 >     FindClass
> >          >>>> >>>>>>                 >      >>>>>>>                 :
> >     internal error
> >          >>>> from
> >          >>>> >>>>>>                 ap04t003.cpp:343
> >          >>>> >>>>>>                 >      >>>>>>>
> >         at
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
>  Method)
> >          >>>> >>>>>>                 >      >>>>>>>
> >         at
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >         at
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> Questions/comments I
> >          >>>> >>>>>>                 have about this are:
> >          >>>> >>>>>>                 >      >>>>>>>                   -
> >     Do we want to
> >          >>>> >>>>>>                 force fatal errors when a JNI
> >          >>>> >>>>>>                 >      >>>>>>>                 call
> >     fails in
> >          >>>> >>>>>>                 general? Most of these tests
> >          >>>> >>>>>>                 >     do the
> >          >>>> >>>>>>                 >      >>>>>>>
> >       right thing and
> >          >>>> test
> >          >>>> >>>>>>                 the return of the JNI
> >          >>>> >>>>>>                 >     calls,
> >          >>>> >>>>>>                 >      >>>>>>>                 for
> >     example:
> >          >>>> >>>>>>                 >      >>>>>>>
> >       thrClass =
> >          >>>> >>>>>>                 >
> jni->FindClass("java/lang/Threadd",
> >          >>>> >>>>>>                 >      >>>>>>>
> >       TRACE_JNI_CALL);
> >          >>>> >>>>>>                 >      >>>>>>>
> >       if (thrClass
> >          >>>> ==
> >          >>>> >>>>>>                 NULL) {
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>                 but
> >     now the
> >          >>>> wrapper
> >          >>>> >>>>>>                 actually would do a
> >          >>>> >>>>>>                 >     fatal if
> >          >>>> >>>>>>                 >      >>>>>>>                 the
> >     FindClass call
> >          >>>> >>>>>>                 would return a nullptr,
> >          >>>> >>>>>>                 >     so we
> >          >>>> >>>>>>                 >      >>>>>>>
> >       could remove that
> >          >>>> >>>>>>                 test altogether. What do you
> >          >>>> >>>>>>                 >      >>>>>>> think?
> >          >>>> >>>>>>                 >      >>>>>>>
> >       - I prefer to
> >          >>>> >>>>>>                 leave them as the tests then
> >          >>>> >>>>>>                 >      >>>>>>>
> >       become closer to
> >          >>>> what
> >          >>>> >>>>>>                 real users would have in
> >          >>>> >>>>>>                 >      >>>>>>>
> >       their code and is
> >          >>>> the
> >          >>>> >>>>>>                 "recommended" way of
> >          >>>> >>>>>>                 >     doing it
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>                    -
> The
> >          >>>> alternative
> >          >>>> >>>>>>                 is to use the
> >          >>>> >>>>>>                 >     NonFatalError I
> >          >>>> >>>>>>                 >      >>>>>>>
> >       added which then
> >          >>>> just
> >          >>>> >>>>>>                 prints out that something
> >          >>>> >>>>>>                 >      >>>>>>>                 went
> >     wrong,
> >          >>>> letting
> >          >>>> >>>>>>                 the test continue. Question
> >          >>>> >>>>>>                 >      >>>>>>>                 will
> >     be what
> >          >>>> should
> >          >>>> >>>>>>                 be the default? The
> >          >>>> >>>>>>                 >     fatal or
> >          >>>> >>>>>>                 >      >>>>>>>                 the
> >     non-fatal
> >          >>>> error
> >          >>>> >>>>>>                 handling?
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>                 On a
> >     different
> >          >>>> >>>>>> subject:
> >          >>>> >>>>>>                 >      >>>>>>>                   -
> >     On the new
> >          >>>> tests,
> >          >>>> >>>>>>                 I've removed the
> >          >>>> >>>>>>                 >      >>>>>>>
> >       NSK_JNI_VERIFY
> >          >>>> since
> >          >>>> >>>>>>                 the JNI wrapper
> >          >>>> >>>>>>                 >     handles the
> >          >>>> >>>>>>                 >      >>>>>>>
> >       tracing and the
> >          >>>> >>>>>>                 verify in almost the same
> >          >>>> >>>>>>                 >     way; only
> >          >>>> >>>>>>                 >      >>>>>>>
> >       difference I can
> >          >>>> >>>>>>                 really tell is that the
> >          >>>> >>>>>>                 >     complain
> >          >>>> >>>>>>                 >      >>>>>>>
> >       method from NSK
> >          >>>> has a
> >          >>>> >>>>>>                 max complain before
> >          >>>> >>>>>>                 >     stopping
> >          >>>> >>>>>>                 >      >>>>>>>                 to
> >     "complain"; I
> >          >>>> have
> >          >>>> >>>>>>                 not added that part
> >          >>>> >>>>>>                 >     of the
> >          >>>> >>>>>>                 >      >>>>>>>                 code
> >     in this
> >          >>>> webrev
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>                 Once
> >     we decide on
> >          >>>> >>>>>>                 these, I can continue on the
> >          >>>> >>>>>>                 >      >>>>>>>
> >       files from
> >          >>>> >>>>>>                 JDK-8212884 and then do both the
> >          >>>> >>>>>>                 >      >>>>>>>
> >       assignment in an
> >          >>>> if
> >          >>>> >>>>>>                 extraction followed-by this
> >          >>>> >>>>>>                 >      >>>>>>>                 type
> >     of webrev in
> >          >>>> an
> >          >>>> >>>>>>                 easier fashion.
> >          >>>> >>>>>>                 >     Depending on
> >          >>>> >>>>>>                 >      >>>>>>>
> >       decisions here,
> >          >>>> >>>>>>                 NSK*VERIFY can be deprecated as
> >          >>>> >>>>>>                 >      >>>>>>>                 well
> >     as we go
> >          >>>> forward.
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
>  Thanks!
> >          >>>> >>>>>>                 >      >>>>>>>                 Jc
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>                 On
> >     Mon, Nov 19,
> >          >>>> 2018
> >          >>>> >>>>>>                 at 11:34 AM Chris Plummer
> >          >>>> >>>>>>                 > >>>>>>>                 <
> >          >>>> chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>
> >          >>>> >>>>>>                 >  <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>> <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >     <mailto:chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>>                 <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>
> >          >>>> >>>>>>                 >  <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>
> >          >>>> >>>>>> <mailto:chris.plummer at oracle.com
> >     <mailto:chris.plummer at oracle.com>>>>> wrote:
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >       On 11/19/18
> >          >>>> 10:07
> >          >>>> >>>>>>                 AM, JC Beyler wrote:
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       Hi all,
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       @David/Chris:
> >          >>>> >>>>>>                 should I then push this
> >          >>>> >>>>>>                 >     RFR to
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       the hotspot
> >          >>>> >>>>>>                 mailing or the runtime
> >          >>>> >>>>>>                 >     one? For
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       what it's
> >          >>>> worth,
> >          >>>> >>>>>>                 a lot of the tests
> >          >>>> >>>>>>                 >     under the
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       vmTestbase
> >          >>>> are
> >          >>>> >>>>>>                 jvmti so the review also
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       affects
> >          >>>> >>>>>>                 serviceability; it just turns
> >          >>>> >>>>>>                 >     out I
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       started with
> >          >>>> the
> >          >>>> >>>>>>                 GC originally and
> >          >>>> >>>>>>                 >     then hit
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       some other
> >          >>>> tests
> >          >>>> >>>>>>                 I had touched via the
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       assignment
> >          >>>> >>>>>>                 extraction.
> >          >>>> >>>>>>                 >      >>>>>>>
> >       I think
> >          >>>> hotspot
> >          >>>> >>>>>>                 would be best.
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >       Chris
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       @Serguei:
> >          >>>> Done
> >          >>>> >>>>>>                 for the method
> >          >>>> >>>>>>                 >     renaming, for
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       the indent,
> >          >>>> are
> >          >>>> >>>>>>                 you talking about
> >          >>>> >>>>>>                 >     going from
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       the 8-indent
> >          >>>> to
> >          >>>> >>>>>>                 4-indent? If so, would
> >          >>>> >>>>>>                 >     it not
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       just be
> >          >>>> better
> >          >>>> >>>>>>                 to do a new JBS bug and
> >          >>>> >>>>>>                 >     do the
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       whole files
> >          >>>> in
> >          >>>> >>>>>>                 one go? I ask because
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       otherwise, it
> >          >>>> >>>>>>                 will look a bit weird to
> >          >>>> >>>>>>                 >     have
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       parts of the
> >          >>>> >>>>>>                 file as 8-indent and others
> >          >>>> >>>>>>                 >      >>>>>>>> 4-indent?
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       Thanks for
> >          >>>> >>>>>>                 looking at it!
> >          >>>> >>>>>>                 >      >>>>>>>>
>  Jc
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       On Mon, Nov
> >          >>>> 19,
> >          >>>> >>>>>>                 2018 at 1:25 AM
> >          >>>> >>>>>>                 >      >>>>>>>>
> >     serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>
> >          >>>> >>>>>>                 <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>
> >          >>>> >>>>>>                 <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>
> >          >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>>
> >          >>>> >>>>>>                 >      >>>>>>>> <mailto:
> >          >>>> serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>>                 <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>
> >          >>>> >>>>>>                 >
> >     <mailto:serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>>>
> >          >>>> >>>>>>                 > >>>>>>>>
> >     <serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>
> >          >>>> >>>>>>                 <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>
> >          >>>> >>>>>>                 >
> >     <mailto:serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>>
> >          >>>> >>>>>>                 >      >>>>>>>> <mailto:
> >          >>>> serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>>                 <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>
> >          >>>> >>>>>>                 >
> >     <mailto:serguei.spitsyn at oracle.com <mailto:
> serguei.spitsyn at oracle.com>
> >          >>>> >>>>>> <mailto:serguei.spitsyn at oracle.com
> >     <mailto:serguei.spitsyn at oracle.com>>>>> wrote:
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           Hi Jc,
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           We have
> >          >>>> to
> >          >>>> >>>>>>                 start this review
> >          >>>> >>>>>>                 >     anyway. :)
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           It looks
> >          >>>> >>>>>>                 good to me in general.
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           Thank you
> >          >>>> >>>>>>                 for your consistency in this
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> refactoring!
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           Some
> >          >>>> minor
> >          >>>> >>>>>>                 comments.
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
> http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.cpp.udiff.html
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           +static
> >          >>>> >>>>>>                 const char*
> >          >>>> >>>>>>                 >     remove_folders(const
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           char*
> >          >>>> >>>>>>                 fullname) { I'd suggest to
> >          >>>> >>>>>>                 >     rename
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           the
> >          >>>> function
> >          >>>> >>>>>>                 name to something
> >          >>>> >>>>>>                 >     traditional
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           like
> >          >>>> >>>>>>                 get_basename. Otherwise, it
> >          >>>> >>>>>>                 >     sounds
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           like this
> >          >>>> >>>>>>                 function has to really
> >          >>>> >>>>>>                 >     remove
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           folders.
> >          >>>> :)
> >          >>>> >>>>>>                 Also, all *Locker.cpp have
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           wrong
> >          >>>> indent
> >          >>>> >>>>>>                 in the bodies of if
> >          >>>> >>>>>>                 >     and while
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> statements.
> >          >>>> >>>>>>                 Could this be fixed
> >          >>>> >>>>>>                 >     with the
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> refactoring?
> >          >>>> >>>>>>                 I did not look on how
> >          >>>> >>>>>>                 >     this
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           impacts
> >          >>>> the
> >          >>>> >>>>>>                 tests other than
> >          >>>> >>>>>>                 > >>>>>>>>
> >       serviceability.
> >          >>>> Thanks,
> >          >>>> >>>>>> Serguei
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >           On
> >          >>>> 11/16/18
> >          >>>> >>>>>>                 19:43, JC Beyler wrote:
> >          >>>> >>>>>>                 > >>>>>>>>>
> >       Hi all,
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >       Anybody
> >          >>>> >>>>>>                 motivated to review this? :)
> >          >>>> >>>>>>                 > >>>>>>>>>
>  Jc
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >       On Wed, Nov
> >          >>>> 7,
> >          >>>> >>>>>>                 2018 at 9:53 PM JC
> >          >>>> >>>>>>                 >     Beyler
> >          >>>> >>>>>>                 > >>>>>>>>>
> >       <jcbeyler at google.com <mailto:jcbeyler at google.com>
> >          >>>> >>>>>> <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>>
> >          >>>> >>>>>>                 >     <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>
> >          >>>> >>>>>>                 <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >     <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>
> >          >>>> >>>>>>                 <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>>
> >          >>>> >>>>>>                 >     <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>
> >          >>>> >>>>>> <mailto:jcbeyler at google.com
> >     <mailto:jcbeyler at google.com>>>>> wrote:
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Hi all,
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Could I
> >          >>>> have
> >          >>>> >>>>>>                 a review for the
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           extension
> >          >>>> >>>>>>                 and usage of the
> >          >>>> >>>>>>                 > >>>>>>>>> ExceptionJniWrapper.
> This
> >          >>>> >>>>>>                 >     adds lines
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           and
> >          >>>> >>>>>>                 filenames to the end of the
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           wrapper
> >          >>>> JNI
> >          >>>> >>>>>>                 methods, adds
> >          >>>> >>>>>>                 >     tracing,
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           and
> >          >>>> throws
> >          >>>> >>>>>>                 an error if need
> >          >>>> >>>>>>                 >     be. I've
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           ported
> >          >>>> the
> >          >>>> >>>>>>                 gc/lock files to
> >          >>>> >>>>>>                 >     use the
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           new
> >          >>>> >>>>>>                 TRACE_JNI_CALL add-on and
> >          >>>> >>>>>>                 >     I've
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           ported a
> >          >>>> few
> >          >>>> >>>>>>                 of the tests
> >          >>>> >>>>>>                 >     that were
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           already
> >          >>>> >>>>>>                 changed for the
> >          >>>> >>>>>>                 >     assignment
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           webrev
> >          >>>> for
> >          >>>> >>>>>>                 JDK-8212884.
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Webrev:
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>
> http://cr.openjdk.java.net/~jcbeyler/8213501/webrev.00/
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.00/>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>
> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8213501/webrev.00/>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Bug:
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8213501
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           For
> >          >>>> >>>>>>                 illustration, if I force
> >          >>>> >>>>>>                 >     an error
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           to the
> >          >>>> >>>>>>                 AP04/ap04t03 test and
> >          >>>> >>>>>>                 >     set the
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           verbosity
> >          >>>> >>>>>>                 on, I get something
> >          >>>> >>>>>>                 >     like:
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           >>
> >          >>>> Calling
> >          >>>> >>>>>>                 JNI method
> >          >>>> >>>>>>                 >     FindClass from
> >          >>>> >>>>>>                 > >>>>>>>>> ap04t003.cpp:343
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           >>
> >          >>>> Calling
> >          >>>> >>>>>>                 with these
> >          >>>> >>>>>>                 >     parameter(s):
> >          >>>> >>>>>>                 > >>>>>>>>> java/lang/Threadd
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Wait for
> >          >>>> >>>>>>                 thread to finish
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           << Called
> >          >>>> >>>>>>                 JNI method
> >          >>>> >>>>>>                 >     FindClass from
> >          >>>> >>>>>>                 > >>>>>>>>> ap04t003.cpp:343
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> Exception in
> >          >>>> >>>>>>                 thread "Thread-0"
> >          >>>> >>>>>>                 > >>>>>>>>>
> >     java.lang.NoClassDefFoundError:
> >          >>>> >>>>>>                 > >>>>>>>>> java/lang/Threadd
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           at
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Method)
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           at
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           at
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Caused
> >          >>>> by:
> >          >>>> >>>>>>                 > >>>>>>>>>
> >     java.lang.ClassNotFoundException:
> >          >>>> >>>>>>                 > >>>>>>>>> java.lang.Threadd
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           at
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:583)
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           at
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           at
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >     java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           ... 3
> >          >>>> more
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           FATAL
> >          >>>> ERROR
> >          >>>> >>>>>>                 in native method: JNI
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           method
> >          >>>> >>>>>>                 FindClass : internal error
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           from
> >          >>>> >>>>>>                 ap04t003.cpp:343
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           at
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003.runIterateOverHeap(Native
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Method)
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           at
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003HeapIterator.runIteration(ap04t003.java:140)
> >          >>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           at
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>
> >          >>>>
> >
>  nsk.jvmti.scenarios.allocation.AP04.ap04t003Thread.run(ap04t003.java:201)
> >          >>>> >>>>>>
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>> Questions/comments I
> >     have about
> >          >>>> >>>>>>                 > >>>>>>>>> this are:
> >          >>>> >>>>>>                 > >>>>>>>>>
> >             - Do we
> >          >>>> >>>>>>                 want to force fatal
> >          >>>> >>>>>>                 >     errors
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           when a
> >          >>>> JNI
> >          >>>> >>>>>>                 call fails in general?
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Most of
> >          >>>> >>>>>>                 these tests do the right
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           thing and
> >          >>>> >>>>>>                 test the return of
> >          >>>> >>>>>>                 >     the JNI
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           calls,
> >          >>>> for
> >          >>>> >>>>>>                 example:
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           thrClass
> >          >>>> =
> >          >>>> >>>>>>                 > >>>>>>>>>
> >     jni->FindClass("java/lang/Threadd",
> >          >>>> >>>>>>                 > >>>>>>>>> TRACE_JNI_CALL);
> >          >>>> >>>>>>                 > >>>>>>>>>
> >               if
> >          >>>> >>>>>>                 (thrClass == NULL) {
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           but now
> >          >>>> the
> >          >>>> >>>>>>                 wrapper actually
> >          >>>> >>>>>>                 >     would do
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           a fatal
> >          >>>> if
> >          >>>> >>>>>>                 the FindClass call
> >          >>>> >>>>>>                 >     would
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           return a
> >          >>>> >>>>>>                 nullptr, so we could
> >          >>>> >>>>>>                 >     remove
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           that test
> >          >>>> >>>>>>                 altogether. What do
> >          >>>> >>>>>>                 >     you
> >          >>>> >>>>>>                 > >>>>>>>>> think?
> >          >>>> >>>>>>                 > >>>>>>>>>
> >               - I
> >          >>>> >>>>>>                 prefer to leave them
> >          >>>> >>>>>>                 >     as the
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           tests
> >          >>>> then
> >          >>>> >>>>>>                 become closer to
> >          >>>> >>>>>>                 >     what real
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           users
> >          >>>> would
> >          >>>> >>>>>>                 have in their
> >          >>>> >>>>>>                 >     code and is
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           the
> >          >>>> >>>>>>                 "recommended" way of doing it
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >              - The
> >          >>>> >>>>>>                 alternative is to
> >          >>>> >>>>>>                 >     use the
> >          >>>> >>>>>>                 > >>>>>>>>>
> >       NonFatalError I
> >          >>>> added
> >          >>>> >>>>>> which
> >          >>>> >>>>>>                 >     then just
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           prints
> >          >>>> out
> >          >>>> >>>>>>                 that something
> >          >>>> >>>>>>                 >     went wrong,
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           letting
> >          >>>> the
> >          >>>> >>>>>>                 test continue.
> >          >>>> >>>>>>                 >     Question
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           will be
> >          >>>> what
> >          >>>> >>>>>>                 should be the
> >          >>>> >>>>>>                 >     default?
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           The
> >          >>>> fatal or
> >          >>>> >>>>>>                 the non-fatal error
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           handling?
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           On a
> >          >>>> >>>>>>                 different subject:
> >          >>>> >>>>>>                 > >>>>>>>>>
> >             - On
> >          >>>> the
> >          >>>> >>>>>>                 new tests, I've
> >          >>>> >>>>>>                 >     removed
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           the
> >          >>>> >>>>>>                 NSK_JNI_VERIFY since the JNI
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           wrapper
> >          >>>> >>>>>>                 handles the tracing
> >          >>>> >>>>>>                 >     and the
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           verify in
> >          >>>> >>>>>>                 almost the same
> >          >>>> >>>>>>                 >     way; only
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> difference I
> >          >>>> >>>>>>                 can really tell
> >          >>>> >>>>>>                 >     is that
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           the
> >          >>>> complain
> >          >>>> >>>>>>                 method from NSK
> >          >>>> >>>>>>                 >     has a
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           max
> >          >>>> complain
> >          >>>> >>>>>>                 before stopping to
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> "complain";
> >          >>>> >>>>>>                 I have not added that
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           part of
> >          >>>> the
> >          >>>> >>>>>>                 code in this webrev
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Once we
> >          >>>> >>>>>>                 decide on these, I can
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           continue
> >          >>>> on
> >          >>>> >>>>>>                 the files from
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> JDK-8212884
> >          >>>> >>>>>>                 and then do both the
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> assignment
> >          >>>> >>>>>>                 in an if extraction
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> followed-by
> >          >>>> >>>>>>                 this type of
> >          >>>> >>>>>>                 >     webrev in an
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           easier
> >          >>>> >>>>>>                 fashion. Depending on
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           decisions
> >          >>>> >>>>>>                 here, NSK*VERIFY can be
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> deprecated
> >          >>>> >>>>>>                 as well as we go
> >          >>>> >>>>>>                 >     forward.
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Thank you
> >          >>>> >>>>>>                 for the
> >          >>>> >>>>>>                 >     reviews/comments :)
> >          >>>> >>>>>>                 > >>>>>>>>>
> >           Jc
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
> >          >>>> >>>>>>                 > >>>>>>>>>
>  --
> >          >>>> >>>>>>                 > >>>>>>>>>
> >       Thanks,
> >          >>>> >>>>>>                 > >>>>>>>>>
>  Jc
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>>
>  --
> >          >>>> >>>>>>                 >      >>>>>>>>
> >       Thanks,
> >          >>>> >>>>>>                 >      >>>>>>>>
>  Jc
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>
> >          >>>> >>>>>>                 >      >>>>>>>                 --
> >          >>>> >>>>>>                 >      >>>>>>>
>  Thanks,
> >          >>>> >>>>>>                 >      >>>>>>>                 Jc
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>
> >          >>>> >>>>>>                 >      >>>>>>             --
> >          >>>> >>>>>>                 >      >>>>>>             Thanks,
> >          >>>> >>>>>>                 >      >>>>>>             Jc
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>>
> >          >>>> >>>>>>                 >      >>>>>         --
> >          >>>> >>>>>>                 >      >>>>> Thanks,
> >          >>>> >>>>>>                 >      >>>>>         Jc
> >          >>>> >>>>>>                 >      >>>>
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>     --
> >          >>>> >>>>>>                 >      >>>     Thanks,
> >          >>>> >>>>>>                 >      >>>     Jc
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>> --
> >          >>>> >>>>>>                 >      >>>
> >          >>>> >>>>>>                 >      >>> Thanks,
> >          >>>> >>>>>>                 >      >>> Jc
> >          >>>> >>>>>>                 >      >
> >          >>>> >>>>>>                 >      >
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > --
> >          >>>> >>>>>>                 >
> >          >>>> >>>>>>                 > Thanks,
> >          >>>> >>>>>>                 > Jc
> >          >>>> >>>>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>
> >          >>>> >>>>>>             --
> >          >>>> >>>>>>             Thanks,
> >          >>>> >>>>>>             Jc
> >          >>>> >>>>>
> >          >>>> >>>>>
> >          >>>> >>>>>
> >          >>>> >>>>>         --
> >          >>>> >>>>>         Thanks,
> >          >>>> >>>>>         Jc
> >          >>>> >>>>
> >          >>>> >>>>
> >          >>>> >>>>
> >          >>>> >>>>     --
> >          >>>> >>>>     Thanks,
> >          >>>> >>>>     Jc
> >          >>>> >>>>
> >          >>>> >>>>
> >          >>>> >>>>
> >          >>>> >>>> --
> >          >>>> >>>>
> >          >>>> >>>> Thanks,
> >          >>>> >>>> Jc
> >          >>>> >>>
> >          >>>> >
> >          >>>>
> >          >>>
> >          >>>
> >          >>> --
> >          >>>
> >          >>> Thanks,
> >          >>> Jc
> >          >>>
> >          >>
> >          >>
> >          >> --
> >          >>
> >          >> Thanks,
> >          >> Jc
> >          >>
> >          >
> >          >
> >          > --
> >          >
> >          > Thanks,
> >          > Jc
> >          >
> >
> >
> >          --
> >
> >          Thanks,
> >          Jc
> >
> >
> >
> >
> > --
> >
> > Thanks,
> > Jc
>
>
>
>
> --
>
>
>
> Thanks,
>
> Jc
>
>
>
>
> --
>
>
>
> Thanks,
>
> Jc
>


-- 

Thanks,
Jc


More information about the hotspot-dev mailing list