RFR(S) 8241004: NMT tests fail on unalinged thread size with debug build

[Boris Ulasevich]

Hi Zhengyu,

On 14.05.2020 15:44, Zhengyu Gu wrote:
> Hi Boris and Thomas,
>
> Thanks for fixing it.
>
> On 5/13/20 2:23 PM, Thomas Stüfe wrote:
>> Hi Boris,
>>
>> I think this looks good. Zhengyu should look at this to make sure.
>>
>> When looking at os::committed_in_range() I remembered that this was a
>> rather specific function. Its name suggests that it counts all committed
>> pages in a range; but it only counts the first committed contiguous 
>> range,
>> then stops.
>
> This function was introduced mainly for stack tracking, and it is the 
> only use at this moment.
>
> But I have ideas for other cases, e.g. having NMT no tracking 
> committed regions, but scanning reserved regions for committed 
> regions. This not only eliminates the most of complexity in virtual 
> memory tracking, but also yields more accurate information (because 
> os::commit() does not actually commit memory).

I'm not sure I got the idea.VirtualMemoryTracker is already doing what 
you said: "scanning reserved regions for committed regions":
- snapshot_thread_stacks() iterates over reserved regions
- for each mtThreadStack region SnapshotThreadStackWalker iterates over 
its committed ranges
- committed ranges of a given region are provided by linux kernel and 
summed up by ReservedMemoryRegion::add_committed_region

> So, I wonder if it is better solution to leave os::committed_in_range 
> alone, but make adjustment in SnapshotThreadStackWalker[1], if it is 
> thread stack only problem?

Yes, it is unaligned thread stack issue reproduced with 
-XX:NativeMemoryTracking option.

> -Zhengyu
>
> [1] 
> http://hg.openjdk.java.net/jdk/jdk/file/a44396903fd8/src/hotspot/share/services/virtualMemoryTracker.cpp#l531
>
>>
>> Thanks, Thomas
>>
>>
>>
>> On Wed, May 13, 2020 at 6:29 PM Boris Ulasevich 
>> <boris.ulasevich at bell-sw.com>
>> wrote:
>>
>>> Hi Thomas,
>>>
>>> Good point! Thanks.
>>> I reworked the function to ease both restrictions.
>>> http://cr.openjdk.java.net/~bulasevich/8241004/webrev.01
>>>
>>> regards,
>>> Boris
>>>
>>> On 13.05.2020 08:59, Thomas Stüfe wrote:
>>>
>>> Hi Boris,
>>>
>>> Note that we have unaligned stack boundaries on AIX too, not so 
>>> strange :)
>>>
>>> What I do not like about the solution is that it loosens the 
>>> restriction
>>> on the input size while keeping the restriction on the start 
>>> address. I'd
>>> rather see this function either in its current form (requiring the 
>>> input
>>> ranges to be page aligned) or the input range to be completely
>>> unrestricted. This function is currently only used by NMT but it is 
>>> useful
>>> and may be used in other places too.
>>>
>>> Can you make it work with arbitrary input ranges? I do not think this
>>> would require much changes from your patch, you do most of the work 
>>> already.
>>>
>>> Thanks!
>>>
>>> ..Thomas
>>>
>>>
>>>
>>> On Thu, May 7, 2020 at 2:56 PM boris <boris.ulasevich at bell-sw.com> 
>>> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please review the following change.
>>>>
>>>> http://cr.openjdk.java.net/~bulasevich/8241004/webrev.00
>>>> http://bugs.openjdk.java.net/browse/JDK-8241004
>>>>
>>>> The stack size created by the implementation of the pthread library is
>>>> not necessarily page aligned. It is weird, but it is not forbidden: 
>>>> the
>>>> pthread library may add some gap on it own. My change removes stack 
>>>> size
>>>> alignment assert checks and tunes NMT committed range calculation for
>>>> the case of unaligned stack size.
>>>>
>>>> thanks,
>>>> Boris
>>>>
>>>
>>>
>>
>