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

Hohensee, Paul hohensee at amazon.com
Mon Apr 8 16:08:29 UTC 2019


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


More information about the hotspot-dev mailing list