RFR: 8154096: Extend WhiteBox API with methods which retrieve from VM information about available GC

[Dmitry Fazunenko]

Igor,

thanks a lot for review!

Kim,

Did I address your concerns?

Thanks,
Dima

On 17.05.2016 20:26, Igor Ignatyev wrote:
> Dima,
>
> the fix looks good to me, reviewed.
>
> Thanks,
> — Igor
>
>> On May 10, 2016, at 9:37 PM, Dmitry Fazunenko <dmitry.fazunenko at oracle.com> wrote:
>>
>> Kim,
>>
>> I've fixed all your comments:
>>   http://cr.openjdk.java.net/~dfazunen/8154096/webrev.03/
>>   http://cr.openjdk.java.net/~dfazunen/8154096/webrev.02vs03/
>>
>>
>> Answering your question I copy my previous answer:
>>
>>> ------------------------------------------------------------------------------
>>>
>>> Looking at the JDK-8154096 and the CRs it is blocking, I'm failing to
>>> understand the motivation for the "selected by ergonomics" predicate.
>>> Why is ergo-selection more interesting than command-line or default
>>> selection?  And for the described use-case (jtreg @requires
>>> filtering), why would any of those distinctions be interesting?
>>>
>>> ------------------------------------------------------------------------------
>>
>> This is needed to check the ability to set an alternative collector in the next VM start.
>> How the 'vm.gc' @requires properties is setting now:
>> jtreg parses given VM flags  for -XX:+Use(.*)GC  pattern. If no matches found, 'vm.gc' will be set to null, which means: all tests for any collector are applicable. If user specified -XX:+UseParallelGC, jtreg will set 'vm.gc' to Parallel, that means that only tests which set Parallel collector or do not set any collector are applicable.
>> This approach has disadvantages:
>> - on some VMs (like minimal), giving -XX:+UseParallelGC will not have any effect, Serial will be used anyway.
>>   but jtreg is now aware of this, so 'vm.gc' will be set to Parallel
>> - If a GC has a mode which is turned like: -XX:+UseG1GC -XX:+UseModeOfG1GC,
>>   jtreg will not be able to handle that correctly, so  'vm.gc' will be set to either G1 or ModeOfG1 (depending on the flag order)
>>
>> Incorrect setting of 'vm.gc' will cause situations when some tests are executed in inappropriate configurations and fail.
>>
>> To address the issues like that, jtreg now allows a custom code to be executed prior the tests.
>> This delegates the responsibility on @requires property setting to test suites.
>> So now we need to write a code to decide whether a collector X could be set or not.
>> The collector X could be set if:
>>   (currentGC == X) | (X isSupportedGC() && currentGC.isSelectedByErgo)
>> For that purpose we need to distinguish cases when the current collector is set by user or selected by ergo.
>>
>> Thanks,
>> Dima
>>
>> On 30.04.2016 3:03, Kim Barrett wrote:
>>>> On Apr 29, 2016, at 7:34 AM, Dmitry Fazunenko <dmitry.fazunenko at oracle.com> wrote:
>>>>
>>>> Hello,
>>>>
>>>> To address the offline comments from Igor I completely reimplemented the fix.
>>>> The main change is introduction a new class sun.hotspot.gc.GC which could be used by tests to:
>>>> http://cr.openjdk.java.net/~dfazunen/8154096/webrev.02/test/raw_files/new/test/lib/sun/hotspot/gc/GC.java
>>>>   - get the currently used GC
>>>>   - check if the current GC was selected by ergo or set explicitly
>>>>   - get list of supported GC
>>>>
>>>> The new version also minimize the changes to the WhiteBox API
>>>>
>>>> So reviewers are still very welcome!
>>>>
>>>> http://cr.openjdk.java.net/~dfazunen/8154096/webrev.02/
>>>> https://bugs.openjdk.java.net/browse/JDK-8154096
>>> ------------------------------------------------------------------------------
>>> hotspot/src/share/vm/prims/whitebox.cpp
>>>   289   } else if (UseParallelGC | UseParallelOldGC) {
>>>
>>> Use || rather than | with booleans.
>>>
>>> ------------------------------------------------------------------------------
>>> hotspot/src/share/vm/prims/whitebox.cpp
>>>   289   } else if (UseParallelGC | UseParallelOldGC) {
>>>   290     return parallel_code;
>>>   291   } else if (UseParallelOldGC) {
>>>   292     return FLAG_IS_ERGO(UseParallelOldGC);
>>>
>>> The test on line 291 will never be true, because that case is already
>>> handled by line 289.
>>>
>>> ------------------------------------------------------------------------------
>>> hotspot/src/share/vm/prims/whitebox.cpp
>>>   298   assert(false, "No GC selected yet");
>>> and
>>>   322   assert(false, "No GC selected yet");
>>>
>>> ------------------------------------------------------------------------------
>>> hotspot/src/share/vm/prims/whitebox.cpp
>>>   299   return false;
>>> and
>>>   323   return false;
>>>
>>> Don't use boolean false as an empty bit mask; return 0.
>>>
>>> ------------------------------------------------------------------------------
>>>
>>> Looking at the JDK-8154096 and the CRs it is blocking, I'm failing to
>>> understand the motivation for the "selected by ergonomics" predicate.
>>> Why is ergo-selection more interesting than command-line or default
>>> selection?  And for the described use-case (jtreg @requires
>>> filtering), why would any of those distinctions be interesting?
>>>
>>> ------------------------------------------------------------------------------
>>>