RFR: 8262188: Add test to verify trace page sizes logging on Linux

[Thomas Stuefe]

On Tue, 23 Feb 2021 21:23:18 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

> Please review this new test that checks the output from `-Xlog:pagesize` and compares it to what's really used according to `/proc/self/smaps`.
> 
> We already have a few tests that check the output from `-Xlog:pagesize`, the difference is that those tests expect some page size given the parameters used. This new test instead check if the values printed out are correct with regards to what is recorded in `/proc/self/maps`.
> 
> I wrote the test to see if all our logging was correct, and found those two issues that are now fixed:
> [JDK-8261029: Code heap page sizes not traced correctly using os::trace_page_sizes](https://bugs.openjdk.java.net/browse/JDK-8261029)
> [JDK-8261230: GC tracing of page sizes are wrong in a few places](https://bugs.openjdk.java.net/browse/JDK-8261230)
> 
> The test now passes and it will be a good way to prevent incorrect page size logging to be added going forward.
> 
> **Testing**
> The new test has been run through mach5 without any problems and also manually on configurations with explicit large pages enabled.

Hi Stefan,

looks good. Minor points below.

Cheers, Thomas

test/hotspot/jtreg/runtime/os/TestTracePageSizes.java line 39:

> 37:  * @requires os.arch=="amd64" | os.arch=="x86_64"
> 38:  * @requires vm.gc != "Z"
> 39:  * @run main/othervm -XX:+AlwaysPreTouch -Xlog:pagesize:ps-%p.log -XX:+UseLargePages -XX:LargePageSizeInBytes=1g TestTracePageSizes

Should we also test with -XX:LargePageSizeInBytes=2M?

test/hotspot/jtreg/runtime/os/TestTracePageSizes.java line 51:

> 49:  * @run main/othervm -XX:+AlwaysPreTouch -Xlog:pagesize:ps-%p.log -XX:-SegmentedCodeCache -XX:+UseLargePages TestTracePageSizes
> 50:  * @run main/othervm -XX:+AlwaysPreTouch -Xlog:pagesize:ps-%p.log -XX:-SegmentedCodeCache -XX:+UseTransparentHugePages TestTracePageSizes
> 51:  */

When do you use multiple @run, when multiple @test?

test/hotspot/jtreg/runtime/os/TestTracePageSizes.java line 81:

> 79:  * @run main/othervm -XX:+AlwaysPreTouch -Xlog:pagesize:ps-%p.log -XX:+UseSerialGC -XX:+UseLargePages TestTracePageSizes
> 80:  * @run main/othervm -XX:+AlwaysPreTouch -Xlog:pagesize:ps-%p.log -XX:+UseSerialGC -XX:+UseTransparentHugePages TestTracePageSizes
> 81: */

I see you don't explicitly test UseSHM. Because the number of tests would be too large?

test/hotspot/jtreg/runtime/os/TestTracePageSizes.java line 104:

> 102:     private static void parseSmaps() throws Exception {
> 103:         Pattern smapsPattern = Pattern.compile(RangeWithPageSize.smapsPatternString, Pattern.DOTALL);
> 104:         Scanner smapsScanner = new Scanner(new File("/proc/self/smaps"));

Would that work if mappings are concurrently modified? (I guess it has to otherwise one could not read reliably from /proc at all)

test/hotspot/jtreg/runtime/os/TestTracePageSizes.java line 102:

> 100:     // match as little as possible so each "segment" in the file
> 101:     // will generate a match.
> 102:     private static void parseSmaps() throws Exception {

Code may be easier to read if the pattern were defined up here.

test/hotspot/jtreg/runtime/os/TestTracePageSizes.java line 168:

> 166: 
> 167:         // The test needs to be run with page size logging printed to ps-$pid.log.
> 168:         Scanner fileScanner = new Scanner(new File("./ps-" + ProcessHandle.current().pid() + ".log"));

So we scan the log file while it is being written by ourselves? This seems slightly unsafe. Are we sure it has been completely flushed at this point? I'm mainly concerned about things like running this test in a current dir on NFS or somesuch.

An alternative would be to let the testee write the log file and print out a copy of /proc/self/smaps to stdout, then to scan and compare in a separate process. 

But if you think this is safe enough, I'm fine with this too.

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

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