RFR: Remove 5 develop options from GC code.

[David Lindholm]

Stefan,

Thanks for the review!

/David

On 2015-10-16 14:05, Stefan Johansson wrote:
> Hi David,
>
> Changes looks good.
>
> Thanks,
> Stefan
>
> On 2015-10-16 10:31, Thomas Schatzl wrote:
>> Hi david,
>>
>> On Thu, 2015-10-15 at 11:29 +0200, David Lindholm wrote:
>>> Thomas,
>>>
>>> Thanks for looking at this.
>>>
>>> On 2015-10-15 10:48, Thomas Schatzl wrote:
>>>> Hi David,
>>>>
>>>> On Fri, 2015-10-09 at 15:47 +0200, David Lindholm wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this patch that removes the following 5 develop options
>>>>> from the GC code:
>>>>>
>>>>> ScavengeWithObjectsInToSpace
>>>>> ParallelOldGCSplitALot
>>>>> ParallelOldGCSplitInterval
>>>>> PSAdjustTenuredGenForMinorPause
>>>>> PSAdjustYoungGenForMajorPause
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8139277 which is a
>>>>> subtask to
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8139276
>>>>> Webrev: http://cr.openjdk.java.net/~david/JDK-8139277/webrev.00/
>>>>>
>>>>> Testing: Passed JPRT
>>>>>
>>>>    - psAdaptiveSizePolicy.cpp:532: the change removes the if-clause
>>>> completely, potentially changing behavior. I would prefer if that were
>>>> not the case, as I cannot immediately see that there is no difference
>>>> here.
>>> I know, I choose between doing this or just leave an empty block (which
>>> would not change behaviour). If you want I can do the empty block 
>>> instead.
>>>
>> Looking at the value of max_gc_minor_pause_sec it seems that this branch
>> has never been taken anyway. Also MaxGCMinorPauseMillis has been
>> deprecated in 8, so we actually might be able to remove it as well in 9.
>>
>> I have not found any use of MaxGCMinorPauseMillis with Parallel GC doing
>> some quick searches (for whatever reason I found a few places where it
>> is used with CMS, but CMS ignores that option).
>>
>> So it seems okay to just remove everything.
>>
>>>>    - not sure how the change in psScavenge.cpp:300, 351, 680, 
>>>> theremoval
>>>> of code triggered by ZapUnusedArea is related to this change. Unless I
>>>> overlooked something, it seems to be something interesting to keep
>>> Since ScavengeWithObjectsInToSpace will always be false now (the 
>>> default
>>> value of the flag when it existed), we will never take the old "else 
>>> if"
>>> branch on line 351, so the value of ZapUnusedHeapArea will not matter
>>> any more here. All mangling will be done by
>>> young_gen->to_space()->clear(SpaceDecorator::Mangle); on line 344. 
>>> Since
>>> the whole space will be mangled always now (if ZapUnusedHeapArea is
>>> true), I assumed that we did not need the block on line 300. The calls
>>> on lines 681-683 seems to be debugcode under #ifdef DEBUG_MANGLING, and
>>> besides it is also called at the end of
>>> SpaceMangler::mangle_unused_area(). And mangling is not 
>>> asynchronous, as
>>> one can me led to believe by the name of that function,
>>> check_mangled_unused_area_complete().
>>>
>>    okay, thanks for the explanations. Looks good.
>>
>> Thomas
>>
>>
>