RFR: 8196405: [REDO] NMT: add_committed_regions doesn't merge succeeding regions
Stefan Karlsson
stefan.karlsson at oracle.com
Wed Feb 21 17:33:32 UTC 2018
Thanks Coleen, Zhengyu, and Erik for reviewing! This has now been pushed.
Thanks,
StefanK
On 2018-02-15 15:05, Stefan Karlsson wrote:
> Hi all,
>
> Please review this patch to fix the merging of committed regions in
> NMT's VirtualMemoryTracker.
>
> http://cr.openjdk.java.net/~stefank/8196405/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8196405
>
> The previous attempt to fix this didn't deal with overlapping
> committed regions correctly. This patch solves that case by first
> removing regions, and parts of regions, that intersect with the newly
> committed region. After that, the new region can be added and/or
> merged with any adjacent regions.
>
> There used to be a special-case for code paths that both reserve and
> commit memory at the same time. ReservedMemoryRegion had an
> all_committed boolean to track that state, and subsequent commits in
> the reserved region were ignored. There were a number of bugs cause by
> this special-handeling, when regions were committed or uncommitted
> inside an all_committed region. This patch removes the all_committed
> property and instead queries the _committed_regions list where necessary.
>
> The patch removes special case handling for mtThreadStacks. The
> intention is that the add_committed_region and
> remove_uncomitted_region functions should be correct, so that we don't
> need that kind of special-case handling.
>
> The overlap_region function uses 'sz - 1', which would be broken if sz
> ever was 0. I added asserts to check for this.
>
> I've added a small jtreg test for the initial problem that this bug
> report was intending to fix. But I realized that the WhiteBox API
> didn't support setting up and testing different call stacks for the
> memory reservations and commits, so I introduced a C++ gtest. This
> test is both faster to execute, easier to debug, and more flexible, so
> I hope this is appreciated. :)
>
> I've tested this with the added unit tests, short run with
> Kitchensink, and mach5 hs-tier1,hs-tier2. Anything else that I should
> be running?
>
> While working on this I found two other bugs:
>
> https://bugs.openjdk.java.net/browse/JDK-8198225
> os::attempt_reserve_memory_at records memory as committed
>
> https://bugs.openjdk.java.net/browse/JDK-8198226
> os::attempt_reserve_memory_at records reserved memory twice on windows
>
> Thanks,
> StefanK
More information about the hotspot-runtime-dev
mailing list