RFR: 8138705: Kitchen sink stress test fails
Max Ockner
max.ockner at oracle.com
Tue May 24 21:14:08 UTC 2016
Thanks for the reviews. I will submit this now.
Max
On 5/23/2016 2:47 PM, Zhengyu Gu wrote:
> Sorry, my bad memory. Forgot the other half of comparisons.
>
> Change look good to me.
>
> -Zhengyu
>
> On 05/23/2016 02:25 PM, Max Ockner wrote:
>> 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
>>>>>
>>>>
>>>
>>
>
More information about the hotspot-runtime-dev
mailing list