RFR: 8146523: Incorrect memory tracker used by os::unmap_memory()
coleen.phillimore at oracle.com
Thu Jan 7 02:04:36 UTC 2016
On 1/6/16 7:54 PM, Jiangli Zhou wrote:
> Hi David,
>> On Jan 5, 2016, at 10:33 PM, David Holmes <david.holmes at oracle.com> wrote:
>> Hi Jiangli,
>> On 6/01/2016 8:30 AM, Jiangli Zhou wrote:
>>> Please review the small fix for JDK-8146523 <https://bugs.openjdk.java.net/browse/JDK-8146523>:
>>> The os::map_memory() and os:unmapp_memory() functions use the memory tracker asymmetrically. The os::map_memory() function calls MemTracker::record_virtual_memory_reserve_and_commit() to track the mapped memory, while os::unmap_memory() uses the 'virtual_memory_release_tracker' to track the memory being unmapped. In NMT, there are different types of tracker for releasing and uncommitting memory. The ‘virtual_memory_uncommit_tracker' is for 'uncommit’ and ‘virtual_memory_release_tracker’ is for 'release'. The os::unmap_memory() should use the ‘virtual_memory_uncommit_tracker' to be balanced with os::map_memory().
>> I find this very confusing. Is there some document that describes the relationship between mapping, reserving and committing virtual memory and the related NMT functionality?
> I agree. This is very confusing. I haven’t come across any document explaining the usage of the NMT APIs in different scenarios. My understand is based on reading the related code.
>> It is not at all clear to me why reserve_and_commit pairs with uncommit (unless committed memory must first be reserved, and uncommit both uncommits and "unreserves" ??)
> I dug deeper into it and found more issues. It appears MemTracker::record_virtual_memory_reserve_and_commit() should record the memory as both ‘reserved’ and ‘committed’. However, that only happens if the memory region is not known by the VirtualMemoryTracker. If the memory overlaps with a known region in the VirtualMemoryTracker, MemTracker::record_virtual_memory_reserve_and_commit() could be a ‘nop' in certain case. In the bug revealed by the specific test scenario, the memory region causing the issue is the CDS shared memory. Here are the call sequences related to the shared memory region:
> 1. ReservedSpace::ReservedSpace - reserve the memory for shared region. VirtualMemorySummary::record_reserved_memory is called and the memory is recored as ‘reserved’.
> 2.os::map_memory - each shared space within the shared memory region is mapped. MemTracker::record_virtual_memory_reserve_and_commit is called, the memory is not recounted. So the shared memory region is only counted as ‘reserved’ in NMT by previous VirtualMemorySummary::record_reserved_memory call.
> 3.os::unmap_memory - record each shared space within the shared memory region with virtual_memory_release_tracker
> 4 ReservedSpace::release (calls os::release_memory) - the virtual_memory_release_tracker is used again for the shared memory region.
> On linux and all other non-windows platform, os::unmap_memory only unmap the memory (by calling system munmap) without ‘release’ the memory. On windows, unmap_memory also does ‘release’.
Yes, this is the difference between windows and unix machines. I think
NMT was developed on windows.
> So, there are at lease two issues:
> The shared memory region is not recorded as ‘committed’.
File a P4 bug on this because I don't think it causes any crashes. I
think it may under count the amount of committed memory. We can
> The shared memory region is counted twice as ‘released’.
Your change conditionalized for Windows in os.cpp would be the best
thing. Making the NMT call at a lower level ie. pd_unmap_memory runs
into the problem that there are duplicates for the xnix platforms.
Also, there are multiple returns that would need NMT logic so it
wouldn't look good. Also, there is risk because some of the os
platforms call pd_unmap_memory by os::pd_attempt_reserve_memory_at()
which would get the tracking wrong (I think).
>>> Tested with test/runtime/NMT.
More information about the hotspot-runtime-dev