RFR(M): 8195103: Refactor ReduceInitialCardMarks to not assume all GCs use card marks

[Erik Osterlund]

Hi Kim,

On 6 Feb 2018, at 05:15, Kim Barrett <kim.barrett at oracle.com> wrote:

>> On Jan 30, 2018, at 8:25 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
>> 
>> Hi Thomas,
>> 
>> Thanks for the review. :)
>> 
>>> On 2018-01-30 11:44, Thomas Schatzl wrote:
>>> Hi,
>>> 
>>> On Tue, 2018-01-16 at 10:42 +0100, Erik Österlund wrote:
>>> 
>>>  ^^ sorry for being a bit late...
>>> 
>>>> Hi,
>>>> 
>>>> The current interface between the compilers and GC regarding the
>>>> ReduceInitialCardMarks optimization lives in the CollectedHeap.
>>>> However, the optimization is relevant only for collectors with a card
>>>> mark barrier set (CardTableModRefBS). Therefore, this interface ought
>>>> to be moved into CardTableModRef so that code gets less messy when a
>>>> collector does not use card marking. In the process, the
>>>> CollectedHeap::pre_initialize member function was removed (as it was
>>>> only used for initializing ReduceInitialCardMarks).
>>>> 
>>>> The optimization needs to check if an object is in young or not.
>>>> This question is now asked to the barrier set rather than the heap.
>>>> For all collectors except G1, this has been implemented by forwarding
>>>> the question to the corresponding heap (inlined member function),
>>>> which is  what was done before. For G1, I chose to instead look at
>>>> the card value  and see if it is a young card, which should give the
>>>> same answer.
>>> Marking the cards young is done concurrently to the application. So you
>>> could get false answers here. However it seems that this is benign,
>>> i.e. at most too many objects are pushed into the deferred card mark
>>> from what I can see.
>>> 
>>> However the assert in
>>> CardTableModRefBs::flush_deferred_card_mark_barrier() may complain...
>>> i.e. at the time when the object is deferred, the result of is_young()
>>> may be false, but at the time the deferred card mark is flushed,
>>> is_young() will return true.
>>> 
>>> Note that while this occurrence is not very common, it does happen.
>>> 
>>> I think this needs to be fixed. Either the mentioned assert, or the
>>> is_young() check. The region type is always good btw.
>> 
>> We discussed this off-list. There is in fact no such race.
>> The compiler slow-path first allocates new memory (TLAB or not). Then it writes young to all of the cards. Then it contemplates whether performing a card mark is necessary for non-young objects to comply with ReduceInitialCardMarks.
>> So by the time the is_young() question is asked, Thread::current() has written the young value, which is always observable to itself. It might be that a concurrent thread over-writes this value with a monotonic card transition to the very same young value, due to crossing the same card boundary with another allocation. In either case, the young value will always be observed by the thread that performed the allocation if and only if the object then resides in young.
> 
> This threw me a bit too.
> 
> I thought there was a post-pause fill cards for young regions phase.
> It's alluded to in a comment in G1RemSet::refine_card_concurrently:
> "The region could be young. ..." But I can't find any such code now.
> Maybe I'm misremembering?  Or maybe it got refactored out of existance?

I had a vague memory about that too but after inspecting the code concluded this is currently not the case. I think we used to shade the cards young on region level after dropping a mutex but changed it at some time as it was a bad idea.

> Looks good.

Thanks for the review.

/Erik

> 
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8195103
>>>> 
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~eosterlund/8195103/webrev.00/
>>>> 
>>>> Testing: mach5 hs-tier1-5
>>>  looks good to me otherwise.
>> 
>> Thanks Thomas!
>> 
>> /Erik
>> 
>>> Thanks,
>>>  Thomas
> 
>