review request (S): 8057531: refactor gc arg processing code slightly

[John Coomes]

Mikael Gerdin (mikael.gerdin at oracle.com) wrote:
> Hi John,
> 
> On Thursday 04 September 2014 02.30.25 John Coomes wrote:
> > Please review this small refactoring of gc argument processing code:
> > 
> > http://cr.openjdk.java.net/~jcoomes/8u/8u40/8057531-refactor-gc-arg-proc/
> 
> I'm by no means an expert on the argument processing code but this ĺooks like 
> a good cleanup.

Thanks for looking at it!

> I noticed that the conditions for ending up in 
> 1537     if (should_auto_select_low_pause_collector()) {
> 
> from 
> 1541     if (!UseSerialGC &&
> 1542         !UseConcMarkSweepGC &&
> 1543         !UseG1GC &&
> 1544         !UseParNewGC &&
> 1545         FLAG_IS_DEFAULT(UseParallelGC)) {
> 
> to
>  611   return UseConcMarkSweepGC || UseG1GC || UseParallelGC || 
> UseParallelOldGC ||
>  612     UseParNewGC || UseSerialGC;
>  613 }
> 
> I think that's a good change.

Yes, there was a missing case before.  It rarely (never?) mattered
since should_auto_select_low_pause_collector() is off by default.

-John

> Before your change it seems like setting -XX:+UseParallelOldGC could still end 
> up in should_auto_select_low_pause_collector since set_parallel_gc_flags() was 
> called after set_ergonomics_flags() and the condition only looked at 
> UseParallelGC.
> 
> The change overall looks ok to me, but I'm not a 8u Reviewer.


> > Thanks for any feedback. 
> > 
> > -John
>