[16] RFR(T) 8248426: NMT: VirtualMemoryTracker::split_reserved_region() does not properly update summary counting

[Thomas Stüfe]

On Mon, Jun 29, 2020 at 4:29 PM Zhengyu Gu <zgu at redhat.com> wrote:

> Hi Thomas,
>
> On 6/29/20 2:30 AM, Thomas Stüfe wrote:
> >
> >     - splitting the reserved region
> >
> >     Could we not simply add the call
> >     to VirtualMemorySummary::record_released_memory(size,
> >     reserved_rgn->flag()); to
> >     VirtualMemoryTracker::split_reserved_region() instead?
>
> I assume you could. Then removing and adding reserved region calls
> are asymmetrical.
>
>
How so?


> For this particular case, remove_released_region() turns out to be
> exactly what you suggested, since reserved_rgn->same_region(addr, size)
> == true.
>
> ...
> VirtualMemorySummary::record_released_memory(size, reserved_rgn->flag());
>
> ...
>
> return _reserved_regions->remove(rgn);
>
>
>
One other disadvantage is that
using VirtualMemoryTracker::remove_released_region() will do the region
lookup again, so we pay twice for the lookup.

But if you are still unconvinced, I won't hold you up. The change certainly
works as it is now and is okay for me, it just would not be my preferred
solution.

Cheers, Thomas


> Thanks,
>
> -Zhengyu
>
>
> >
> >     Thanks, Thomas
> >
> >
> >
> >
> >
> >     On Fri, Jun 26, 2020 at 10:54 PM Zhengyu Gu <zgu at redhat.com
> >     <mailto:zgu at redhat.com>> wrote:
> >
> >         Hi,
> >
> >         Please review this trivial patch that fixes summary counting in
> >         VirtualMemoryTracker::split_reserved_region().
> >
> >         The method uses internal method to remove a reserved region,
> >         which does
> >         not update counting information. It should use high level
> tracking
> >         method VirtualMemoryTracker::remove_released_region() instead.
> >
> >         Without patch, NMT summary reports uncategorized memory, e.g.
> >
> >         -                   Unknown (reserved=1060416KB, committed=0KB)
> >                                       (mmap: reserved=1060416KB,
> >         committed=0KB)
> >
> >
> >
> >
> >         Bug: https://bugs.openjdk.java.net/browse/JDK-8248426
> >         Webrev: http://cr.openjdk.java.net/~zgu/JDK-8248426/webrev.00/
> >
> >         Test:
> >             hotspot_nmt
> >             Submit test in progress
> >
> >
> >         Thanks,
> >
> >         -Zhengyu
> >
>
>