RFR: 8228991: Obsolete -XX:UseAdaptiveGCBoundary

[Stefan Karlsson]

On 2020-04-15 12:53, Kim Barrett wrote:
>> On Apr 15, 2020, at 5:40 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>
>> Hi Kim,
>>
>> On 2020-04-15 00:23, Kim Barrett wrote:
>>> Please review this change which obsoletes the boolean product flag
>>> -XX:UseAdaptiveGCBoundary and removed the supporting code.
>>> As discussed in the associated CSR, this feature has had a long bug
>>> tail and does not appear to have ever really worked, so we're going
>>> straight to obsoleting the option and removing the code, rather than
>>> first deprecating it.
>>> This mostly involves removal of a bunch of code.  In some places there
>>> are functions which previously needed to be virtual but no longer need
>>> that because the derived classes no longer exist.
>>> There is an assertion in ParallelScavengeHeap::initialize() that was
>>> changed by removing !UseAdaptiveGCBoundary from a disjunction, which
>>> is not what one would expect if making that option unconditionally
>>> false.  It looks like the presence of a test for that option never
>>> mattered, as except in the heterogeneous heap case the virtual spaces
>>> have always supposed to meet (it's using AdjoiningGenerations after
>>> all) and did so.
>>>   CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8228991
>>> https://bugs.openjdk.java.net/browse/JDK-8242164 (CSR)
>>> Webrev:
>>> https://cr.openjdk.java.net/~kbarrett/8228991/open.00/
>> This looks good to me. If you want to push this, that's fine by me.
>>
>> I found a lot of dead/unneeded code when following the trails of what you removed. It was too much to describe, so I created a patch for it instead:
>> https://cr.openjdk.java.net/~stefank/8228991/webrev.01/
> You are right, there’s more dead code there.  I hadn’t noticed that after my changes
> AdjoiningGeneration was only used to provide construction of the PS{Young,Old}Gen
> objects, which then get pulled out and cached and the  AdjoiningGeneration object
> is unused after that.
>
>> Main change in the patch:
>> - Removes AdjoiningGenerations, AdjoiningVirtualSpaces, PSVirtualSpaceHighToLow
>> - Cleans up PS{Young,Old}Gen
>> -- Removes most virtual declarations from Removes
>> -- Moves PS{Young,Old}Gen initialization to constructor and makes initilization functions private.
>> -- protected -> private
>>
>> Should I propose this as a follow-up cleanup?
> We could do that, or I could merge your changes into mine and do it all at once.
> Reviewing might be easier as two steps, esp. since you and Thomas have already
> looked the initial set of changes.  I’m fine with either, though if done as two steps
> I’m going to skip some of Thomas’s comments, since they apply to stuff that’s just
> going to get deleted by the followup.

Let's take it in two steps then, just to make future archeology easier.

StefanK
>