RFR [2/2] 8065258: JDK-8065358 Refactor G1s usage of save_marks and reduce related races
bengt.rutisson at oracle.com
Thu Nov 20 14:00:33 UTC 2014
On 11/20/14 2:24 PM, Mikael Gerdin wrote:
> (I just noticed that I mistyped the bug number in the subject, it
> should be 8065358 twice.)
> I removed the weird qualifier combination from "st" in
> Thomas also suggested to me that the guarantee in
> record_retained_region may in fact be incorrect in some corner cases,
> I removed it since no other part of the code actually depends on this
> being true.
> New webrev at:
> Incremental webrev at:
New webrev looks good to me. Reviewed.
> On 2014-11-20 09:46, Mikael Gerdin wrote:
>> Hi all,
>> This is the second patch of two to fix JDK-8065358
>> In this change I suggest that we introduce a new field _scan_top to
>> G1OffsetTableContigSpace to keep track of the maximum address where a GC
>> worker thread is allowed to scan objects in old regions which have been
>> used as allocation regions during the current evacuation cycle.
>> More background (from the bug):
>> G1 currently uses the _saved_mark_word field in Space to detemine a
>> point in a region above which a current GC cannot scan objects. This is
>> needed to prevent card scanning threads from scanning in memory regions
>> where other gc workers are concurrently allocating objects.
>> This re-use of the _saved_mark_word field causes confusion to readers of
>> the code since it's not used for the same purpose as the other Space
>> The setting and reading of this field, and the per region gc time stamp
>> which accompanies it is also unnecessarily racy. Previously the
>> _saved_mark_word field was set to the value of the _top pointer when a
>> region was selected as a GC allocation region and the time stamp was set
>> to indicate that the saved mark field should be used as a maximum
>> address. This code had some problems with races in JDK-8058209 and could
>> be redesigned in a less racy way.
>> Suggested fix is to introduce a new field in HeapRegions to keep track
>> of the maximum address where card scanning is allowed and set that field
>> at the point of retaining an OldGCAllocRegion instead of when
>> allocations are being started. That way we get rid of the store ordering
>> requirement in the timestamp record path.
>> There is still a race between the per region time stamps and the per
>> region top pointer, where we must ensure that the time stamp store must
>> be visible before any subsequent top stores.
>> This store ordering is enforced by the fact that all stores to top are
>> either ordered with #storestore (the initial allocation) or guarded by a
>> Mutex. To ensure that the reader path sees a consistent view it must be
>> exectued with the proper load ordering, where we must read top before
>> the time stamp in order to ensure that we don't see a top value which
>> has been updated by a concurrent gc worker if we see a time stamp from a
>> previous gc cycle.
>> Webrev: http://cr.openjdk.java.net/~mgerdin/8065358/webrev.0/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8065358
>> Testing: JPRT, Kitchensink (4hr+)
More information about the hotspot-gc-dev