RFR: 8331539: [REDO] NMT: add/make a mandatory MEMFLAGS argument to family of os::reserve/commit/uncommit memory API [v2]

Afshin Zafari azafari at openjdk.org
Fri May 24 09:01:02 UTC 2024


On Thu, 23 May 2024 12:41:25 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixed the missing parts of shenandoahHeap.cpp
>
> src/hotspot/os/posix/os_posix.cpp line 387:
> 
>> 385:     if (os::release_memory(extra_base, begin_offset))
>> 386:     {
>> 387:       ThreadCritical tc;
> 
> In many of the functions we put the `ThreadCritical` inside the `MemTracker` after the `enabled()` check, but we don't do it here. Why is that? Shouldn't the `ThreadCritical` usage be hidden inside `MemTracker`?

I have already tried to move `ThreadCritical` into the `MemTracker` (in another PR), but it failed. AFAIR, the unmapping/releasing the memory should be in critical section too. The current implementation follows this order: 1) create critical section 2) unmap/release 3) if successful, call MemTracker. The step 2) should be in critical section.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19343#discussion_r1613124698


More information about the shenandoah-dev mailing list