RFR: 8196405: [REDO] NMT: add_committed_regions doesn't merge succeeding regions

Zhengyu Gu zgu at redhat.com
Thu Feb 15 16:23:42 UTC 2018


Looks really good.

Thanks for fixing it.

-Zhengyu

On 02/15/2018 09:05 AM, 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