On Mon, 28 Oct 2024 16:12:39 GMT, Robert Toyonaga <d...@openjdk.org> wrote:

>> ### Summary
>> This PR just replaces `ThreadCritical` with a lock specific to NMT.  
>> `ThreadCritical` is a big lock and is unnecessary for the purposes of NMT. 
>> I've implemented the new lock with a semaphore so that it can be used early 
>> before VM init.  There is also the possibility of adding assertions in 
>> places we expect NMT to have synchronization. I haven't added assertions yet 
>> in many places because some code paths such as the (NMT tests)  don't lock 
>> yet. I think it makes sense to close any gaps in locking in another PR in 
>> which I can also add more assertions. 
>> 
>> Testing:
>> - hotspot_nmt
>> - gtest:VirtualSpace
>> - tier1
>
> Robert Toyonaga has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - add a comment explaining lock rank
>  - remove unnecessary dropping of tracking level

> About locking around os::free - not sure if we can do this. Are all paths 
> inside os::free lock free? It does call NMT. I think all cases inside NMT 
> that touches are lock free, but am not sure. Have to think about this a bit.

Ah okay should be fine. Both mallocs and frees should be lock free in NMT. In 
summary mode we just modify counters. In detail mode, we add to the malloc site 
table, but that one is lock free as well.

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

PR Comment: https://git.openjdk.org/jdk/pull/20852#issuecomment-2447593990

Reply via email to