RFR (L) 8211899: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[E-M]

[JC Beyler]

Hi all,

How about on a Tuesday? :)

Sneak peak and motivational sentence: after this goes in, we have this one
<http://cr.openjdk.java.net/~jcbeyler/8212148/webrev.00/> which removes the
NSK_CPP_STUBs from the tests entirely! :)
Jc


On Fri, Oct 12, 2018 at 5:37 PM JC Beyler <jcbeyler at google.com> wrote:

> Hi all,
>
> Any chance for a second reviewer on a Friday? :)
> Jc
>
> On Thu, Oct 11, 2018 at 11:03 AM Alex Menkov <alexey.menkov at oracle.com>
> wrote:
>
>> got it.
>>
>> LGTM.
>>
>> --alex
>>
>> On 10/10/2018 19:03, JC Beyler wrote:
>> > Hi Alex,
>> >
>> > Thanks for the review! Yes I had seen that too before sending this
>> > review out and forked that fix into this:
>> > https://bugs.openjdk.java.net/browse/JDK-8211905
>> >
>> > Which now is merged and I've updated my webrev to reflect this of
>> course.
>> >
>> > My rationale for not doing it here directly is always the same: keeping
>> > the changes to the "spirit" of what the change is trying to do. Those
>> > extra casts were a bit out-of-scope and so I just forked the fix in
>> 8211905.
>> >
>> > Thanks!
>> > Jc
>> >
>> > On Wed, Oct 10, 2018 at 5:40 PM Alex Menkov <alexey.menkov at oracle.com
>> > <mailto:alexey.menkov at oracle.com>> wrote:
>> >
>> >     Hi Jc,
>> >
>> >     Overall looks good.
>> >
>> >     one minor note:
>> >
>> >
>>  test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/events/EM06/em06t001/em06t001.cpp:
>> >     -    jclassName = (jstring) (jstring) (jstring) (jstring) (jstring)
>> >     (jstring) (jstring) (jstring) (jstring)
>> NSK_CPP_STUB3(CallObjectMethod,
>> >     jni_env, klass,
>> >     -                        methodID);
>> >     +    jclassName = (jstring) (jstring) (jstring) (jstring) (jstring)
>> >     (jstring) (jstring) (jstring) (jstring)
>> >     jni_env->CallObjectMethod(klass,
>> >     methodID);
>> >
>> >     Please drop multi "(jstring)"
>> >
>> >     --alex
>> >
>> >     On 10/08/2018 21:21, JC Beyler wrote:
>> >      > Hi all,
>> >      >
>> >      > I am continuing the NSK_CPP_STUB removal with this next webrev.
>> >      > Webrev: http://cr.openjdk.java.net/~jcbeyler/8211899/webrev.00/
>> >     <http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/>
>> >      > <http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/>
>> >      > Bug: https://bugs.openjdk.java.net/browse/JDK-8211899
>> >      >
>> >      > The change is still straight-forward though, since it is just
>> >     doing the
>> >      > same NSK_CPP_STUB removal. However when I was looking at the
>> >     changes, a
>> >      > lot of these changes are touching lines with spaces before/after
>> >      > parenthesis. I've almost never touched the spaces except if I was
>> >      > refactoring by hand the line at the same time. The rationale
>> >     being that
>> >      > the lines will get fixed a few more times and are, at worse,
>> >     covered by
>> >      > the bug: https://bugs.openjdk.java.net/browse/JDK-8211335, which
>> >     I've
>> >      > commited to doing.
>> >      >
>> >      > Two exceptions are here where I pushed out the code into
>> >     assignments due
>> >      > to really long lines and complex if structures:
>> >      > - jvmti/scenarios/hotswap/HS204/hs204t003/hs204t003.cpp
>> >      >
>> >     <
>> http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS204/hs204t003/hs204t003.cpp.udiff.html
>> >
>> >      > - jvmti/scenarios/jni_interception/JI01/ji01t001/ji01t001.cpp
>> >      >
>> >     <
>> http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/jni_interception/JI01/ji01t001/ji01t001.cpp.udiff.html
>> >
>> >      >
>> >      > And one exception here where a commented line was doing the
>> >     out-of-if
>> >      > assignment so I just uncommented it and used the variable:
>> >      > - jvmti/scenarios/hotswap/HS301/hs301t001/hs301t001.cpp
>> >      >
>> >     <
>> http://cr.openjdk.java.net/%7Ejcbeyler/8211899/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS301/hs301t001/hs301t001.cpp.udiff.html
>> >
>> >      >
>> >      > I've tested the modified changes on my machine, all modified
>> >     tests pass.
>> >      >
>> >      > Let me know what you think,
>> >      > Jc
>> >      >
>> >      > Ps: 2 more of these and we can say good bye to NSK_CPP_STUB*
>> >      >
>> >
>> >
>> >
>> > --
>> >
>> > Thanks,
>> > Jc
>>
>
>
> --
>
> Thanks,
> Jc
>


-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181016/a0fb30b1/attachment-0001.html>