RFR: Remove 5 develop options from GC code.

[Stefan Johansson]

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