RFR (M) 8210182: Remove macros for C compilation from vmTestBase but non jvmti

[JC Beyler]

Hi all,

Thanks for the reviews, I'll fix the issue that Serguei pointed out and I
ran the vmTestBase and all tests pass now with the fix that Alex pointed
out.

Thanks again,
Jc

On Fri, Aug 31, 2018 at 4:02 PM Chris Plummer <chris.plummer at oracle.com>
wrote:

> Hi JC,
>
> Other than what Serguei pointed out, it looks good.
>
> thanks,
>
> Chris
>
> On 8/30/18 11:31 PM, serguei.spitsyn at oracle.com wrote:
>
> Hi Jc,
>
> The fix looks great as it looks much more simple now!
>
> One minor suggestion about this file update:
>
>
> http://cr.openjdk.java.net/%7Ejcbeyler/8210182/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/share/jpda/libNativeMethodsTestThread.cpp.udiff.html
>
>   An example:
>
>   +        env->CallVoidMethod(thisObject, env->GetMethodID(klass, "log", "(Ljava/lang/String;)V"),  +                            message);
>
>   I'd suggest either to place one call at one line, or place each argument
> on a different line like this:
>
>   +        env->CallVoidMethod(thisObject,
>   +                            env->GetMethodID(klass, "log", "(Ljava/lang/String;)V"),  +                            message);
>
>   There are several similar cases in this file.
>
>   No need in another webrev if you fix this.
>
>
> Thanks,
> Serguei
>
>
> On 8/30/18 12:03, JC Beyler wrote:
>
> Hi Alex,
>
> I've fixed those cases and found an extra one. I'll revise my scripts for
> the next folder so these no longer happen (bad regexp that eagerly finds a
> ')'.
>
> I had launched in parallel a test for vmTestBase, it finished with a few
> failures that might be linked to this so I have relaunched the whole
> vmTestBase and will post the results here.
>
> The new webrev with the fixes is here:
> http://cr.openjdk.java.net/~jcbeyler/8210182/webrev.02/
>
> Thanks again for the review,
> Jc
>
> On Thu, Aug 30, 2018 at 10:48 AM Alex Menkov <alexey.menkov at oracle.com>
> wrote:
>
>> Hi Jc,
>>
>> It looks much better now.
>> BTW did you run all the tests?
>>
>> test/hotspot/jtreg/vmTestbase/nsk/share/jni/JNIreferences.cpp
>> "(V" should be  "()V":
>>
>>           // notify another thread that JNI local reference has been
>> created
>> -        JNI_ENV_PTR(env)->CallVoidMethod(JNI_ENV_ARG_3(env,
>> createWicket, JNI_ENV_PTR(env)->GetMethodID(JNI_ENV_ARG_4(env, klass,
>> "unlock", "()V"))));
>> +        env->CallVoidMethod(createWicket, env->GetMethodID(klass,
>> "unlock", "(V"));
>>
>>           // wait till JNI local reference can be released (it will
>> heppen then we will leave the method)
>> -        JNI_ENV_PTR(env)->CallVoidMethod(JNI_ENV_ARG_3(env,
>> deleteWicket, JNI_ENV_PTR(env)->GetMethodID(JNI_ENV_ARG_4(env, klass,
>> "waitFor", "()V"))));
>> +        env->CallVoidMethod(deleteWicket, env->GetMethodID(klass,
>> "waitFor", "(V"));
>>   }
>>
>> --alex
>>
>> On 08/29/2018 22:01, JC Beyler wrote:
>> > Hi all,
>> >
>> > A follow-up to Igor's work on getting tests in C++, I am working on
>> > simplifying the macros in the tests from the vmTestBase. The full
>> change
>> > being a bit too large, I'm cutting it up in pieces to be easier to
>> > review and integrate.
>> >
>> > Here is the first part, it changes all vmTestbase tests outside the
>> > vmTestbase/jvmti subfolder:
>> >
>> > Webrev: http://cr.openjdk.java.net/~jcbeyler/8210182/webrev.01/
>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8210182/webrev.01/>
>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8210182
>> >
>> > Thanks!
>> > Jc
>>
>
>
> --
>
> Thanks,
> Jc
>
>
>
>

-- 

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