RFR: 8166607: G1 needs klass_or_null_acquire

[Erik Helin]

On 11/18/2016 03:03 PM, Kim Barrett wrote:
>> On Nov 15, 2016, at 6:58 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>
>>> On Nov 15, 2016, at 5:21 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>>
>>> Hi Kim,
>>>
>>> On Mon, 2016-11-07 at 14:38 -0500, Kim Barrett wrote:
>>>>>
>>>>> On Nov 7, 2016, at 5:53 AM, Thomas Schatzl <thomas.schatzl at oracle.c
>>>>> om> wrote:
>>>>> Maybe it would be cleaner to call a method in the barrier set
>>>>> instead of inlining the dirtying + enqueuing in lines 685 to 691?
>>>>> Maybe as an additional RFE.
>>>> We could use _ct_bs->invalidate(dirtyRegion).  That's rather
>>>> overgeneralized and inefficient for this situation, but this
>>>> situation should occur *very* rarely; it requires a stale card get
>>>> processed just as a humongous object is in the midst of being
>>>> allocated in the same region.
>>>
>>> I kind of think for these reasons we should use _ct_bs->invalidate() as
>>> it seems clearer to me. There is the mentioned drawback of having no
>>> other more efficient way, so I will let you decide about this.
>>
>> I've made the change to call invalidate, and also updated some comments.
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8166607
>>
>> Webrevs:
>> full: http://cr.openjdk.java.net/~kbarrett/8166607/webrev.03/
>> incr: http://cr.openjdk.java.net/~kbarrett/8166607/webrev.03.inc/
>>
>> Also, see RFR: 8166811, where I've included a webrev combining the
>> latest changes for 8166607 and 8166811, since they are rather
>> intertwined.  I think I'll do as Erik suggested and push the two
>> together.
>
> Sorry folks, but I want to revert this part and go back to the old
> code where it locked the shared queue and enqueued there.
>
> If the executing invocation of refine_card is from a Java thread,
> e.g. this is the "mutator helps with refinement" case, calling
> invalidate would enqueue to the current thread's buffer.  But that is
> effectively a reentrant call to enqueue, and the Java thread case of
> enqueue is not reentrant-safe.  Only enqueue to the shared queue is
> reentrant-safe.
>
> I think that scenario presently can't happen, since the mutator helps
> case is dealt with by the mutator processing it's own buffer.  In that
> situation, all the cards in the buffer came from writes by this thread
> to an object this thread either allocated or has access to, so the
> klass must be there.  But that's getting uncomfortably subtle in what
> is already difficult to analyze code.

Agree, lets revert to the old code. Thanks for being so careful about 
this change.

> Also, we've talked about changing the mutator helps case to not
> immediately process it's own buffer but instead add its buffer to the
> pending buffer list and process the next (FIFO ordered) buffer, in
> order to let its buffer age.  (I have a change for that in my post-JDK
> 9 collection of pending changes.  The mutator-invoked enqueue might be
> reentrant-safe in that change, but I don't think I want to make that
> guarantee.)

It is hard as it is to keep track of all the synchronization and 
guarantees spread out in the code to make the card refinement work, so I 
would prefer to keep it simple as just revert back to the existing code.