RFR: 8161208: Unable to run jtreg tests with MinimalVM

[Dmitry Fazunenko]

Michail,

the change looks good to me.
Thank you for taking my notes.

-- Dima

On 20.07.2016 15:10, Michail Chernov wrote:
>
> Hi Dima,
>
> I took into account your comments.
>
> http://cr.openjdk.java.net/~mchernov/8161208/webrev.03/
>
> JFR is enabled only with FlightRecorder or StartFlightRecording 
> options. Please look at closed src (jfrActivator.cpp - 
> JfrActivator::initialize()).
>
> I would keep checking of UnlockCommercialFeatures because it makes 
> this checking more clear and this is similar to code from closed part.
>
>
> Thanks,
> Michail
>
> On 07/20/2016 11:12 AM, Dmitry Fazunenko wrote:
>> Michail,
>>
>> The new version looks much better, thanks for finding existing WB API 
>> to access VM flags.
>> So, the fix looks good, but some rather cosmetic comments:
>>
>> 1) 45 private WhiteBox whiteBox = WhiteBox.getWhiteBox(); --> 45 
>> private *static final* WhiteBox WB = WhiteBox.getWhiteBox();
>>
>> 2) startFR is not boolean: 137 String isStartFR = --> 137 String 
>> startFR =
>>
>> 3) For the sake of code simplicity you can skip check for 
>> UnlockedCommercialFatures But it's up to you. 4) another way to turn 
>> JFR on: |-XX:FlightRecorderOptions| (https://docs.oracle.com/javacomponents/jmc-5-5/jfr-runtime-guide/comline.htm#JFRRT184)
>> I'm not sure if this option has an effect standalone, but it's better to check
>> for its setting as well.
>>
>> Thanks,
>> Dima
>>
>>
>> On 19.07.2016 16:35, Michail Chernov wrote:
>>> Hi David,
>>> Thanks for your answer. I revised code that checks FlightRecorder 
>>> status. At my point of view better way is to use WhiteBox to get VM 
>>> flag value. This allows not to use additional dependencies for VMProps.
>>>
>>> Updated webrev:
>>> http://cr.openjdk.java.net/~mchernov/8161208/webrev.01/
>>>
>>> Tested locally on Open JDK build, and Oracle JDK (Linux x64 , Linux 
>>> x86 - server and minimal VM).
>>>
>>> Thanks,
>>> Michail
>>>
>>> On 15.07.2016 08:31, David Holmes wrote:
>>>> Hi Michail,
>>>>
>>>> On 15/07/2016 2:21 AM, Michail Chernov wrote:
>>>>> Hi,
>>>>>
>>>>> Could I have a reviews for this change, please?
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8161208
>>>>> http://cr.openjdk.java.net/~mchernov/8161208/webrev.00/
>>>>> http://cr.openjdk.java.net/~mchernov/8161208/webrev.hotspot.00/
>>>>>
>>>>> This issue is started since this fix:
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-June/018319.html 
>>>>>
>>>>
>>>> I think I would have objected to the form of that fix - VMProps was 
>>>> only looking at information available via system properties.
>>>>
>>>>> This change fixes two issues:
>>>>> 1. Avoiding of java.management usage with minimal VM (JTreg was 
>>>>> started
>>>>> with '-vmoptions:"-minimal"' and worked as expected).
>>>>
>>>> Okay.
>>>>
>>>>> 2. Adding appropriate modules for proper VMProps execution on 
>>>>> non-full
>>>>> JDK images (JTreg was tested with '-vmoptions:"-limitmods 
>>>>> java.base"',
>>>>> java.compact1, java.compact2 and so on - JTreg is started and 
>>>>> works as
>>>>> expected).
>>>>
>>>> Was that testing done with a full VM or the Minimal VM? On a full 
>>>> VM limited to compact1, compact2 or java.base, java.lang.management 
>>>> will not be present, but we will execute the query code in VMProps 
>>>> as it isn't the minimal VM.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Tested locally and via RBT/JPRT.
>>>>>
>>>>> Thanks
>>>>> Michail
>>>
>>
>