RFR: JDK-8138717: TestGCEventMixedWithG1ConcurrentMark.java fails

[Bengt Rutisson]

Thanks, Jesper!

Bengt

On 2015-10-08 14:22, Jesper Wilhelmsson wrote:
> Looks good!
> Great that you found a way to fix GenCollectedHeap as well!
> /Jesper
>
> Den 8/10/15 kl. 12:53, skrev Bengt Rutisson:
>>
>> Hi all,
>>
>> Could I have a couple of reviews for this fix?
>>
>> http://cr.openjdk.java.net/~brutisso/8138717/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8138717
>>
>> The fix for JDK-8134953 tried to keep creation of new GC ids the same 
>> as before.
>> But there was one exception. When we create heap dumps or do heap 
>> inspection
>> before or after a GC we no longer create a new GC id for those 
>> events. Instead
>> they use the same GC id as the "enclosing" GC.
>>
>> This was an intentional change. But in retrospect it should probably 
>> not have
>> been done as part of the other changes since
>> it breaks some tests that expect the GC id for the heap inspections 
>> to be
>> different than the GC id for the actual GC. It should be a separate 
>> discussion
>> of whether or not we should have different GC ids for these activities.
>>
>> Here's the change to collectedHeap.cpp that removes the creation of 
>> new GC ids:
>> http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/diff/983c56341c80/src/share/vm/gc/shared/collectedHeap.cpp 
>>
>>
>>
>> The proposed patch adds the creation of new GC ids back at the same 
>> places. The
>> patch also introduces a GCIdMarkAndRestore class since the "GC" for a 
>> heap dump
>> or heap inspected will be nested inside of a normal GC. This is the same
>> behavior as before JDK-8134953, but it means that we can't use a 
>> GCIdMark since
>> that would reset the GC id to undefinied in its destructor.
>>
>> In fact there is one more place where we nest GC ids. It's in
>> GenCollectedHeap::do_collection(). This isn't a real problem (yet) 
>> since no one
>> is using the GC id after it has been set to undefinied. But now that 
>> we have the
>> GCIdMarkAndRestore anyway, we can use that here too to have the 
>> correct GC id
>> available at all times.
>>
>> Thanks,
>> Bengt