RFR: JDK-8251384: [TESTBUG] jvmti tests should not be executed with minimal VM

David Holmes david.holmes at oracle.com
Thu Aug 27 04:49:04 UTC 2020


Hi Alex,

On 21/08/2020 6:54 am, Alex Menkov wrote:
> Hi Igor,
> 
> On 08/20/2020 09:23, Igor Ignatyev wrote:
>> HI Alex,
>>
>> one minor nit: according to usual java coding conventions, 
>> isJVMTIIncluded should be spelled as isJvmtiIncluded. otherwise the 
>> fix looks good to me.
> 
> I tried to be consistent with other methods like
> isCDSIncludedInVmBuild, isJFRIncludedInVmBuild, isGCSupported, 
> isGCSelected, etc.

Yes - when a name includes an acronym the use of camel-case is a 
secondary consideration.

> Maybe this should be isJVMTIIncludedInVmBuild..

Yes that seems better and I would also prefer to see it implemented in 
the same style as:

WB_ENTRY(jboolean, WB_IsJFRIncludedInVmBuild(JNIEnv* env))
#if INCLUDE_JFR
   return true;
#else
   return false;
#endif // INCLUDE_JFR
WB_END

to avoid implicit booleans and avoid the runtime condition check.

Thanks,
David
-----

>>
>>> Other tests will be updated in the follow-ups.
>> have you already identified all the tests which need this @requires? 
>> filed bugs/RFEs for them?
> 
> Not yet.
> I had problem with running all hotspot tests with minimal build (for 
> some reason jtreg was not able to complete it), so I decided start from 
> the tests mentioned in the jira issue and then test area-by-area, file 
> and fix the tests in batches.
> 
> --alex
> 
>>
>> Cheers,
>> -- Igor
>>
>>
>>> On Aug 19, 2020, at 6:02 PM, Alex Menkov <alexey.menkov at oracle.com> 
>>> wrote:
>>>
>>> Hi all,
>>>
>>> please review the fix for
>>> https://bugs.openjdk.java.net/browse/JDK-8251384
>>> webrev:
>>> http://cr.openjdk.java.net/~amenkov/jdk16/minimal_jvmti/webrev/
>>>
>>> The fix introduces new @requires option "vm.jvmti":
>>> test/lib/sun/hotspot/WhiteBox.java
>>> test/jtreg-ext/requires/VMProps.java
>>> src/hotspot/share/prims/whitebox.cpp
>>> test/hotspot/jtreg/TEST.ROOT
>>>
>>> and updates tests in test/hotspot/jtreg/serviceability/jvmti (the 
>>> only change in all tests is added "@requires vm.jvmti")
>>> Other tests will be updated in the follow-ups.
>>>
>>> The
>>


More information about the serviceability-dev mailing list