Request for review - 8013184 CMS: Call reset_after_compaction() only if a compaction has been done

[Jon Masamitsu]

On 4/29/13 8:05 AM, Mikael Gerdin wrote:
> Jon,
>
> On 2013-04-25 21:32, Jon Masamitsu wrote:
>> 8013184: CMS: Call reset_after_compaction() only if a compaction has
>> been done
>>
>> When a System.gc() is done with -XX:-UseCMSCompactAtFullCollection, a
>> mark-sweep
>> without compaction is done.  In that instance the call to
>> reset_after_compaction() is
>> not appropriate.
>>
>> This changeset is built on top of the changeset for 8013032
>>
>> http://cr.openjdk.java.net/~jmasa/8013184/webrev.00/
>
> Could you change the name of the test to say something about using the 
> "foreground" GC?
> Maybe "SystemGCOnForegroundCollector"?
> I don't think the "three times" is relevant enough to be included in 
> the test name.

Yes to "SystemGCOnForegroundCollector"
>
> *_did_compact is only relevant for Full GCs, what would you say about 
> changing it to "full_gc_did_compact" to make it obvious that the 
> variable is only relevant for Full GCs?
Would you say that CMS does anything other than a full GC?   At least in
the general sense of "full GC".  I qualify that because CMS usually
only does a collection of the tenured generation but it's often
referred to as a "full GC".   If you also think of a CMS collection
as a full GC, then I think the "full_gc" in "full_gc_did_compaction"
is implied.  If you don't think that CMS usually does a full GC and
only the compacting GC is a full GC, then "_did_compact" implies
a full GC because that's the only time a compaction is done.

So I'm not keen on changing the name to "_full_gc_did_compact"
because it sounds redundant.  But l'll do it if you think it is
more clear.

Thanks.

Jon


>
> /Mikael