RFR: 8212748: ZGC: Add reentrant locking functionality

Kim Barrett kim.barrett at oracle.com
Tue Nov 20 20:37:46 UTC 2018


> On Nov 20, 2018, at 2:45 AM, Per Liden <per.liden at oracle.com> wrote:
> 
> Hi,
> 
> On 10/26/18 2:00 AM, Kim Barrett wrote:
>>> On Oct 25, 2018, at 12:20 PM, Erik Österlund <erik.osterlund at oracle.com> wrote:
>>> 
>>> Hi Per,
>>> 
>>> Thanks for having a look at this.
>>> 
>>> On 2018-10-25 17:27, Per Liden wrote:
>>>> Hi Erik,
>>>> On 2018-10-25 15:44, Erik Österlund wrote:
>>>>> Hi,
>>>>> 
>>>>> ZGC needs a per-nmethod lock to be used for concurrent IC cleaning, protecting misaligned oops from concurrently being patched by nmethod entry barriers, and read using CompiledMethod::is_unloading(), with interactions crossing JavaThreads and non-Java threads. This patch adds that utility.
>>>>> 
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~eosterlund/8212748/webrev.00/
>>>> I'd like to suggest that ZReentrantLock doens't inherit from ZLock, but that it instead has a ZLock. And that ZLocker is adjusted to take a T* instead of a ZLock*.
>>> 
>>> Wouldn't it be annoying that you can't use a plain ZLocker then, but instead would have to use ZLocker<ZLock>? I'm not sure I see the advantage of that approach, only a usability disadvantage. I'm not opposed either though if you prefer it to be done in that way.
>> For what its worth, that’s exactly how the similar C++11 facility works, e.g. one says
>>     std::lock_guard<std::mutex> lk(m);
>> or
>>     std::lock_guard<std::recursive_mutex> lk(rm);
> 
> Erik and I discussed this some more and we ended up with the following patch:
> 
> http://cr.openjdk.java.net/~pliden/8212748/webrev.0/
> 
> cheers,
> Per

------------------------------------------------------------------------------

Is there a reason why this was not based on a pthread_mutex_t
configured with the PTHREAD_MUTEX_RECURSIVE attribute?  (Rather than
this "hand-rolled" recursion.)

------------------------------------------------------------------------------

What is RecursiveZLock::is_owned for?  That might be why the RECURSIVE
attribute wasn't used, but since I don't see the point, except perhaps
for assertion checks.  Part of my question is, if is_owned is
desirable for recursive case, why not for ordinary?  A similar
implementation would seem to suffice.

------------------------------------------------------------------------------ 



More information about the hotspot-gc-dev mailing list