Request for review: JDK-8003581, , UseG1GC is not properly accounted for by INCLUDE_ALTERNATE_GCS

[David Holmes]

On 6/02/2013 3:12 AM, Joe Provino wrote:
> Here's the latest webrev after Chris and David's comments:
>
> http://cr.openjdk.java.net/~jprovino/8003581/webrev.02/

Almost there - this:

! #if INCLUDE_ALL_GCS
!   #if (defined JAVASE_EMBEDDED || defined ARM)
!     UNSUPPORTED_GC_OPTION(UseG1GC);
!   #endif
! #endif

should use:

UNSUPPORTED_OPTION(UseG1GC, "G1 GC");

David

> joe
>
> On 2/5/2013 10:31 AM, Chris Plummer wrote:
>> Hi Joe,
>>
>> David and I made quite a few comments over the past few days. Did you
>> not see them? I believe the conclusion is that this is what you need:
>>
>> #if INCLUDE_ALL_GCS
>> #if (defined JAVASE_EMBEDDED || defined ARM)
>> UNSUPPORTED_OPTION(UseG1GC, "G1 GC");
>> #endif
>> #endif
>>
>> #if !INCLUDE_ALL_GCS
>> UNSUPPORTED_GC_OPTION(UseG1GC);
>> UNSUPPORTED_GC_OPTION(UseParallelGC);
>> UNSUPPORTED_GC_OPTION(UseParallelOldGC);
>> UNSUPPORTED_GC_OPTION(UseConcMarkSweepGC);
>> UNSUPPORTED_GC_OPTION(UseParNewGC);
>> #endif // INCLUDE_ALL_GCS
>>
>> David also suggested that the 2nd part above be moved to
>> force_serial_gc().
>>
>> Chris
>>
>> On 2/5/13 6:22 AM, Joe Provino wrote:
>>> Would anyone have time to review this updated change. It's one file --
>>> arguments.cpp.
>>>
>>> thanks.
>>>
>>> joe
>>>
>>> On 1/25/2013 11:25 AM, Joe Provino wrote:
>>>> > With changes from Bob and David, here is a new webrev:
>>>> >
>>>> > http://cr.openjdk.java.net/~jprovino/8003581/webrev.01
>>>> >
>>>> > joe
>>>> >
>>>> > On 1/25/2013 12:44 AM, David Holmes wrote:
>>>>> >> On 25/01/2013 4:45 AM, Bob Vandette wrote:
>>>>>> >>> You claim to be defaulting to SerialGC but you don't set
>>>>>> UseG1GC to
>>>>>> >>> false?
>>>>> >>
>>>>> >> The UNSUPPORTED_OPTION macro does that.
>>>>> >>
>>>>> >> But I don't see anything forcing use of serialGC. And
>>>>> >> force_serial_gc() should not be altered.
>>>>> >>
>>>>>> >>> If you build with INCLUDE_ALL_GCS=1, the G1 code will be
>>>>>> available
>>>>>> >>> and be used.
>>>>> >>
>>>>> >> Only on those platforms that support it.
>>>>> >>
>>>>> >> David
>>>>> >>
>>>>>> >>> Bob.
>>>>>> >>>
>>>>>> >>> On Jan 24, 2013, at 1:38 PM, Joe Provino wrote:
>>>>>> >>>
>>>>>>> >>>> Webrev is here:
>>>>>>> >>>> http://cr.openjdk.java.net/~jprovino/8003581/webrev.00
>>>>>>> >>>>
>>>>>>> >>>> It's a change to one file: arguments.cpp.
>>>>>>> >>>>
>>>>>>> >>>> thanks.
>>>>>>> >>>>
>>>>>>> >>>> joe
>>>>>> >>>
>>