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

[Igor Ignatyev]

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?
>> 
>> ------------------------------------------------------------------------------
>> 
>