RFR (L) 8211899: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[E-M]
JC Beyler
jcbeyler at google.com
Wed Oct 17 22:18:47 UTC 2018
Hi Paul,
Thanks for the review :)
I've fixed the cases you saw to at least keep them consistent. However, I'm
not too worried about these as my next set of webrevs is going to fix the
spaces for all ( and ) in the vmTestbase so we will fix everything once and
for all.
The JDK bug I'm referring to is :
https://bugs.openjdk.java.net/browse/JDK-8211335
I already have relatively the scripts to do it :-),
Jc
On Wed, Oct 17, 2018 at 1:01 PM Hohensee, Paul <hohensee at amazon.com> wrote:
> Re spaces :). There’s a bunch of places where “(jvmti_env” should be “(
> jvmti_env”, but of course not all of them. I found a bunch. Otherwise,
> lgtm., no need for another webrev.
>
>
>
> Paul
>
>
>
> In hs104t002.cpp
>
>
>
> - if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti,
>
> - &caps) )) {
>
> + if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
>
>
>
> =>
>
>
>
> + if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
>
>
>
> In hs203t003.cpp
>
>
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB4(GetClassSignature,
>
> - jvmti_env, klass, &className, &generic) ) ) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti_env->GetClassSignature(klass, &className, &generic) ) ) {
>
>
>
> =>
>
>
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti_env->GetClassSignature(klass, &className, &generic) ) ) {
>
>
>
> and
>
>
>
> - if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps) )) {
>
> + if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
>
>
>
> =>
>
>
>
> + if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
>
>
>
> and
>
>
>
> - if ( ! NSK_JVMTI_VERIFY( NSK_CPP_STUB3(GetThreadState, jvmti, thread, &state) ) ) {
>
> + if ( ! NSK_JVMTI_VERIFY(jvmti->GetThreadState(thread, &state) ) ) {
>
>
>
> =>
>
>
>
> + if ( ! NSK_JVMTI_VERIFY( jvmti->GetThreadState(thread, &state) ) ) {
>
>
>
> and
>
>
>
> - if ( ! NSK_JVMTI_VERIFY( NSK_CPP_STUB3(GetThreadState, jvmti, thread, &state) ) ) {
>
> + if ( ! NSK_JVMTI_VERIFY(jvmti->GetThreadState(thread, &state) ) ) {
>
> nsk_printf(" Agent :: Error while getting thread state.\n");
>
> nsk_jvmti_agentFailed();
>
> } else {
>
> if ( state & JVMTI_THREAD_STATE_SUSPENDED) {
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2( PopFrame, jvmti, thread) ) ) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti->PopFrame(thread) ) ) {
>
>
>
> =>
>
>
>
> + if ( ! NSK_JVMTI_VERIFY( jvmti->GetThreadState(thread, &state) ) ) {
>
> …
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti->PopFrame(thread) ) ) {
>
>
>
> and
>
>
>
> - if ( !NSK_JVMTI_VERIFY( NSK_CPP_STUB2 ( ResumeThread, jvmti, thread)) ) {
>
> + if ( !NSK_JVMTI_VERIFY(jvmti->ResumeThread(thread)) ) {
>
>
>
> =>
>
>
>
> + if ( !NSK_JVMTI_VERIFY( jvmti->ResumeThread(thread)) ) {
>
>
>
> In hs203t004.cpp
>
>
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(GenerateEvents, jvmti_env,
>
> - JVMTI_EVENT_COMPILED_METHOD_LOAD ) )) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti_env->GenerateEvents(JVMTI_EVENT_COMPILED_METHOD_LOAD) )) {
>
>
>
> =>
>
>
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti_env->GenerateEvents(JVMTI_EVENT_COMPILED_METHOD_LOAD) )) {
>
>
>
> and
>
>
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB3(GetMethodDeclaringClass,
>
> - jvmti_env, method, &threadClass) ) ) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti_env->GetMethodDeclaringClass(method, &threadClass) ) ) {
>
>
>
> =>
>
>
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti_env->GetMethodDeclaringClass(method, &threadClass) ) ) {
>
>
>
> and
>
>
>
> - if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps) )) {
>
> + if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
>
>
>
> =>
>
>
>
> + if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
>
>
>
> and
>
>
>
> - if ( NSK_JVMTI_VERIFY( NSK_CPP_STUB2(SuspendThread, jvmti, thread) ) ) {
>
> + if ( NSK_JVMTI_VERIFY(jvmti->SuspendThread(thread) ) ) {
>
>
>
> =>
>
>
>
> + if ( NSK_JVMTI_VERIFY( jvmti->SuspendThread(thread) ) ) {
>
>
>
> and
>
>
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB3(GetThreadState, jvmti,
>
> - thread, &state) ) ) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti->GetThreadState(thread, &state) ) ) {
>
> NSK_COMPLAIN0("#error Agent :: while getting thread's state.\n");
>
> nsk_jvmti_agentFailed();
>
> } else {
>
> if ( state & JVMTI_THREAD_STATE_SUSPENDED) {
>
> - if ( ! NSK_JVMTI_VERIFY( NSK_CPP_STUB2(PopFrame, jvmti, thread) ) ){
>
> + if ( ! NSK_JVMTI_VERIFY(jvmti->PopFrame(thread) ) ){
>
>
>
> =>
>
>
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti->GetThreadState(thread, &state) ) ) {
>
> …
>
> + if ( ! NSK_JVMTI_VERIFY( jvmti->PopFrame(thread) ) ){
>
>
>
> In hs204t001.cpp
>
>
>
> - /* if( (myTestClass =NSK_CPP_STUB2(NewGlobalRef,jni_env, klass)) == NULL) {
>
> + /* if( (myTestClass =jni_env->NewGlobalRef(klass)) == NULL) {
>
>
>
> =>
>
>
>
> + /* if( (myTestClass = jni_env->NewGlobalRef(klass)) == NULL) {
>
>
>
> and
>
>
>
> - if (!NSK_JNI_VERIFY(env, (testClass =(jclass) NSK_CPP_STUB2(NewGlobalRef, env, klass)) != NULL))
>
> + if (!NSK_JNI_VERIFY(env, (testClass =(jclass) env->NewGlobalRef(klass)) != NULL))
>
> nsk_jvmti_setFailStatus();
>
> - if (!NSK_JNI_VERIFY(env, (testedThread =NSK_CPP_STUB2(NewGlobalRef, env, thread)) != NULL))
>
> + if (!NSK_JNI_VERIFY(env, (testedThread =env->NewGlobalRef(thread)) != NULL))
>
>
>
> =>
>
>
>
> + if (!NSK_JNI_VERIFY(env, (testClass = (jclass) env->NewGlobalRef(klass)) != NULL))
>
> nsk_jvmti_setFailStatus();
>
> …
>
> + if (!NSK_JNI_VERIFY(env, (testedThread = env->NewGlobalRef(thread)) != NULL))
>
>
>
> and
>
>
>
> - if (!NSK_JVMTI_VERIFY( NSK_CPP_STUB2(SuspendThread, jvmti, thread))) {
>
> + if (!NSK_JVMTI_VERIFY( jvmti->SuspendThread(thread))) {
>
>
>
> =>
>
>
>
> + if (!NSK_JVMTI_VERIFY(jvmti->SuspendThread(thread))) {
>
>
>
> In hs204t003.cpp
>
>
>
> - if (! NSK_JVMTI_VERIFY( NSK_CPP_STUB3(GetThreadState, jvmti, thread, &state)) ){
>
> + if (! NSK_JVMTI_VERIFY(jvmti->GetThreadState(thread, &state)) ){
>
> NSK_DISPLAY0(" Agent :: Error getting thread state.\n");
>
> nsk_jvmti_agentFailed();
>
> } else {
>
> if ( state & JVMTI_THREAD_STATE_SUSPENDED) {
>
> NSK_DISPLAY0(" Agent :: Thread state = JVMTI_THREAD_STATE_SUSPENDED.\n");
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(PopFrame, jvmti, thread) ) ) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti->PopFrame(thread) ) ) {
>
> NSK_DISPLAY0("#error Agent :: Jvmti failed to do popFrame.\n");
>
> nsk_jvmti_agentFailed();
>
> } else {
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(ResumeThread, jvmti, thread)) ) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti->ResumeThread(thread)) ) {
>
>
>
> =>
>
>
>
> + if (! NSK_JVMTI_VERIFY( jvmti->GetThreadState(thread, &state)) ){
>
> …
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti->PopFrame(thread) ) ) {
>
> …
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti->ResumeThread(thread)) ) {
>
>
>
> In hs301t001.cpp
>
>
>
> - if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps) )) {
>
> + if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
>
>
>
> =>
>
>
>
> + if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
>
>
>
> In hs301t002.cpp
>
>
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps) ) ) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) ) ) {
>
>
>
> =>
>
>
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) ) ) {
>
>
>
> In hs301t003.cpp
>
>
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB4(GetClassSignature,
>
> - jvmti_env, klass, &className, &generic) ) ) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti_env->GetClassSignature(klass, &className, &generic) ) ) {
>
>
>
> =>
>
>
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti_env->GetClassSignature(klass, &className, &generic) ) ) {
>
>
>
> and
>
>
>
> - if (! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities, jvmti, &caps) )) {
>
> + if (! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
>
>
>
> =>
>
>
>
> + if (! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
>
>
>
> and
>
>
>
> - if ( ! NSK_JVMTI_VERIFY ( NSK_CPP_STUB2(AddCapabilities,
>
> - jvmti, &caps) )) {
>
> + if ( ! NSK_JVMTI_VERIFY (jvmti->AddCapabilities(&caps) )) {
>
>
>
> =>
>
>
>
> + if ( ! NSK_JVMTI_VERIFY ( jvmti->AddCapabilities(&caps) )) {
>
>
>
> *From: *serviceability-dev <serviceability-dev-bounces at openjdk.java.net>
> on behalf of JC Beyler <jcbeyler at google.com>
> *Date: *Tuesday, October 16, 2018 at 4:24 PM
> *To: *"alexey.menkov at oracle.com" <alexey.menkov at oracle.com>
> *Cc: *"serviceability-dev at openjdk.java.net" <
> serviceability-dev at openjdk.java.net>
> *Subject: *Re: RFR (L) 8211899: Remove the NSK_CPP_STUB macros from
> vmTestbase for jvmti/scenarios/[E-M]
>
>
>
> 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
>
--
Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181017/d692106f/attachment-0001.html>
More information about the serviceability-dev
mailing list