RFR: 8267155: runtime/os/TestTracePageSizes times out

[Aleksey Shipilev]

On Mon, 17 May 2021 16:09:32 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> This Linux-specific test parses `/proc/self/smaps` using a dotall regular expression. If part of the expression don't match it explodes in complexity, leading to timeouts.
> 
> In our case, `VmFlags` tag was missing from smaps, which was introduced with kernel 3.8. I am actually not able to determine how slow they were; on one machine they ran for two hours before getting killed.
> 
> I tried to fiddle with the regular expression and gave up, instead opting to rewrite the parser to get a simple one-pass scan. This is way faster than before - on our old-kernel machines the tests complete in 2 minutes. On new kernels the test is a bit faster too.
> 
> In addition to rewriting the parser, I added code which copies the smaps file into the test directory before parsing it. I do this to minimize problems should the underlying proc fs content change while parsing, and to have a way to retain the parsed smaps files.
> 
> I also added a way to feed an external smaps file into the test. Of course the test would fail, but it was a way to test the parser.
> 
> Unfortunately, this does not make the test succeed. The timeouts are gone, but we have still have no way to know if TPHs are enabled or not. That is a separate issue though.
> 
> Thanks, Thomas

Cursory review follows. 

The alternative would be to disable this on kernels < 3.8? See the commits from my last PR:
https://github.com/openjdk/jdk/pull/3415/files/c7a6136cedb827260f43ecc850a8910492cfefcc..1df37de73ed6615db565aeafe76edf30dcee108d

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

> 105:     private static Path copySmaps() throws Exception {
> 106:         Path p1 = Paths.get("/proc/self/smaps");
> 107:         Path p2 = Paths.get("smaps-copy-" +  ProcessHandle.current().pid() + "-" + run++ + ".txt");

Suggestion:

        Path p2 = Paths.get("smaps-copy-" +  ProcessHandle.current().pid() + "-" + (run++) + ".txt");

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

> 107:         Path p2 = Paths.get("smaps-copy-" +  ProcessHandle.current().pid() + "-" + run++ + ".txt");
> 108:         Files.copy(p1, p2, StandardCopyOption.REPLACE_EXISTING);
> 109:         System.out.println("Copied " + p1.getFileName() + " to " + p2.getFileName() + "...");

`Path.toString()` is defined, I think, so...

Suggestion:

        System.out.println("Copied " + p1 + " to " + p2 + "...");

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

> 134:         Pattern sectionStartPat = Pattern.compile("^([a-f0-9]+)-([a-f0-9]+) [\\-rwpsx]{4}.*");
> 135:         Pattern kernelPageSizePat = Pattern.compile("^KernelPageSize:\\s*(\\d*) kB");
> 136:         Pattern vmFlagsPat = Pattern.compile("^VmFlags: ([\\w\\? ]*)");

Suggestion:

        static final Pattern SECTION_START_PATT = Pattern.compile("^([a-f0-9]+)-([a-f0-9]+) [\-rwpsx]{4}.*");
        static final Pattern KERNEL_PAGESIZE_PATT = Pattern.compile("^KernelPageSize:\\s*(\\d*) kB");
        static final Pattern VMFLAGS_PATT = Pattern.compile("^VmFlags: ([\\w\? ]*)");

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

> 137:         int lineno = 0;
> 138:         void reset() {
> 139:             start = end = ps = vmFlags = null;

Suggestion:

            start = null;
            end = null;
            ps = null;
            vmFlags = null;

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

> 150: 
> 151:         void eatNext(String line) {
> 152:             debug("" + lineno++ + " " + line);

Suggestion:

            debug("" + (lineno++) + " " + line);

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

> 156:                 start = matSectionStart.group(1);
> 157:                 end = matSectionStart.group(2);
> 158:                 ps = vmFlags = null;

Suggestion:

                ps = null;
                vmFlags = null;

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

> 178:         SmapsParser parser2 = new SmapsParser();
> 179:         Files.lines(smapsFileToParse).forEach(parser2::eatNext);
> 180:         parser2.finish();

Suggestion:

        SmapsParser parser = new SmapsParser();
        Files.lines(smapsFileToParse).forEach(parser::eatNext);
        parser.finish();

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

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