RFR: Remove 5 develop options from GC code.
Thomas Schatzl
thomas.schatzl at oracle.com
Fri Oct 16 08:31:59 UTC 2015
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
More information about the hotspot-gc-dev
mailing list