RFR (M): 8159422: Very high Concurrent Mark mark stack contention

[Mikael Gerdin]

Hi Thomas,

On 2016-09-15 10:57, Thomas Schatzl wrote:
> Hi Kim,
>
> On Wed, 2016-09-14 at 11:42 -0400, Kim Barrett wrote:
>>>
>>> On Sep 14, 2016, at 8:54 AM, Thomas Schatzl <thomas.schatzl at oracle.
>>> com> wrote:
>>>
>>> Hi Kim,
>>>
>>> On Tue, 2016-09-13 at 12:19 -0400, Kim Barrett wrote:
>>>>
>>>>>
>>>>>
>>>>> On Sep 13, 2016, at 6:15 AM, Thomas Schatzl <thomas.schatzl at ora
>>>>> cle.
>>>>> com> wrote:
>>>>> http://cr.openjdk.java.net/~tschatzl/8159422/webrev.2_to_3/
>>>>> (diff)
>>>>> http://cr.openjdk.java.net/~tschatzl/8159422/webrev.3/ (full)
>>>> ---------------------------------------------------------------
>>>> ----
>>>> -----------
>>>> src/share/vm/gc/g1/g1ConcurrentMark.cpp
>>>>  242 G1CMMarkStack::OopChunk*
>>>> G1CMMarkStack::remove_chunk_from_chunk_list() {
>>>>  243   MutexLockerEx x(MarkStackChunkList_lock,
>>>> Mutex::_no_safepoint_check_flag);
>>>>  244   _chunks_in_chunk_list--;
>>>>  245   return remove_chunk_from_list(&_chunk_list);
>>>>  246 }
>>>>
>>>> Decrement of _chunks_in_chunk_list needs to be conditional on
>>>> result
>>>> of remove_chunk_from_list, only decrementing if got a chunk.
>>>>
>>>   it did not fail with the latest jprt run (I am basically always
>>> doing
>>> jprt runs for all changes I send out) because this method (and all
>>> these new shorthand methods) have not actually been called.
>> I missed that too!
>>
>>>
>>>>
>>>> src/share/vm/gc/g1/g1ConcurrentMark.hpp
>>>> I think some of the padding in G1CMMarkStack isn't useful with
>>>> the
>>>> lock-based approach.  I'm assuming you've left it because you
>>>> plan to
>>>> get back to the lock-free approach later.  Is there a CR for
>>>> that?
>>>>
>>> I can only see that the padding for
>>> _chunks_in_chunk_list/_chunk_list
>>> might not be useful as they are always accessed together.
>>> I removed it.
>> OK
>>
>>>
>>> The new CR is JDK-8165848 (also referenced by this CR).
>> OK
>>
>>>
>>> Changes at
>>> http://cr.openjdk.java.net/~tschatzl/8159422/webrev.3_to_4/ (diff)
>>> http://cr.openjdk.java.net/~tschatzl/8159422/webrev.4/ (full)
>> Looks good.
>>
>
>   thanks a lot for your continued reviews.

I've looked through the updated webrevs and it still looks good :)

/Mikael

>
> Thanks,
>   Thomas
>