RFR: 8138705: Kitchen sink stress test fails

[Max Ockner]

Zhengyu,
I have replied below:

On 5/23/2016 8:34 AM, Zhengyu Gu wrote:
> Hi Max,
>
>
> On 05/17/2016 03:53 PM, Max Ockner wrote:
>> Hello,
>> I have responded to suggestions from Coleen and Zhengyu.
>>
>> New webrev: http://cr.openjdk.java.net/~mockner/8138705.02/
>>  - Removed print statements
>>  - Added check to break out of loop once we reach the end of the 
>> region being committed
>>
>> Thanks,
>> Max
>>
>> On 5/17/2016 9:06 AM, Zhengyu Gu wrote:
>>> Hi Max,
>>>
>>> I have not done a complete review.
>>>
>>> virtualMemoryTracker.cpp:  63 - 88
>>>
>>> Is it possible that newly committed region does not overlap, but 
>>> covers an existing region?
>>>
> You did not answer above question. If incoming committed region covers 
> existing region(s), but does not overlap or adjacent to any the 
> existing region(s), it will be added as new region.

If a region covers an existing (but not identical) region, then it is an 
overlapping region. Two regions overlap if they intersect in any way. So 
this case can not happen.

   inline bool overlap_region(address addr, size_t sz) const {
     VirtualMemoryRegion rgn(addr, sz);
     return contain_address(addr) ||
            contain_address(addr + sz - 1) ||
            rgn.contain_address(base()) ||
            rgn.contain_address(end() - 1);
   }

Sorry that I missed this question.

Thanks,
Max

> As use cases get complicated, I think that it is simpler to always 
> delete the region(s), then add.
>
>
> src/share/vm/utilities/linkedlist.hpp
>
> Nice catch.
>
> -Zhengyu
>
>>> The loop can stop when rgn->end_addr >= addr + size
>>>
>>> -Zhengyu
>>>
>>>
>>> On 05/13/2016 03:39 PM, Max Ockner wrote:
>>>> Hello,
>>>> Please review this fix which allows NMT to handle overlapping commits.
>>>>
>>>> Though it would make sense to uncommit space before committing over 
>>>> it,  this is not always the way things are done. Kitchensink 
>>>> encounters some intentional overlapping commits in gc, which 
>>>> previously would cause NMT to throw an assert. Now, these 
>>>> overlapping commits are handled by recording an uncommit of the 
>>>> newly committed region before recording the commit. This 
>>>> effectively chops any of the already-committed space off of the 
>>>> existing committed region before adding the new commit.  In order 
>>>> for this all to work, 
>>>> ReservedMemoryRegion::remove_uncommitted_region was changed to 
>>>> support uncommitting an arbitrary region. The target behavior of 
>>>> this operation is to truncate (or split) committed regions which 
>>>> overlap with the region being uncommitted, while only subtracting 
>>>> from the total  the amount of memory which was actually cleared.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8138705
>>>> Webrev: http://cr.openjdk.java.net/~mockner/8138705/
>>>>
>>>> Tested with jtreg NMT tests. Survived 72 hours of Kitchensink.
>>>> Added a test: runtime/NMT/CommitOverlappingRegions.java
>>>> This test performs various commits and uncommits that overlap in 
>>>> different ways, and checks that it computes the amount of committed 
>>>> space correctly.
>>>>
>>>> Thanks,
>>>> Max
>>>
>>
>