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