RFR: JDK-8280940: gtest os.release_multi_mappings_vm is racy [v2]

[Thomas Stuefe]

On Wed, 9 Feb 2022 06:47:36 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Dans remarks
>
> test/hotspot/gtest/runtime/test_os.cpp line 449:
> 
>> 447:   if (MemTracker::tracking_level() > NMT_off) {
>> 448:     return;
>> 449:   }
> 
> Not a review (I'm not an expert in the relevant area), just a couple drive-by comments.  However, GitHub UI won't let me comment on the parts that I want.  So getting as close as I can.
> 
> (1) The `TEST_VM` line should be outdented.
> 
> (2) After 8277822, I think the tracking level is always going to be > `NMT_off` in a debug build, so we'll only be testing in product builds.  That seems problematic.

Hi Kim,

thanks for taking a look!

> Not a review (I'm not an expert in the relevant area), just a couple drive-by comments. However, GitHub UI won't let me comment on the parts that I want. So getting as close as I can.
> 
> (1) The `TEST_VM` line should be outdented.

Sure.

> 
> (2) After 8277822, I think the tracking level is always going to be > `NMT_off` in a debug build, so we'll only be testing in product builds. That seems problematic.

We run the gtests in all NMT modes (off, summary, default), see:

https://github.com/openjdk/jdk/blob/072e7b4da0449ab7c1ab1ba0cfbb3db233823e7c/test/hotspot/jtreg/gtest/NMTGtests.java#L30-L36

so we run these tests in debug builds with NMT=off. The NMT gtests have been introduced with 8256844 and extended to cover the os* tests with 8277822.

That said, I should take a look at 8263464, see if this still is a problem.

Cheers, Thomas

-------------

PR: https://git.openjdk.java.net/jdk/pull/7288