RFR: 8267155: runtime/os/TestTracePageSizes times out [v3]

[Aleksey Shipilev]

On Tue, 18 May 2021 04:42:00 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
>
> Thomas Stuefe has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - heap size -> 128m
>  - fixes

Yes, I don't mind this going in. I was just pointing the way you could fix the test on <3.8 kernels either here on in separate PR.

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

> 99:     private static LinkedList<RangeWithPageSize> ranges = new LinkedList<>();
> 100:     private static boolean debug = false;
> 101:     private static int run = 0;

Suggestion:

    private static boolean debug;
    private static int run;

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

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

Suggestion:

        static final Pattern VMFLAGS_PATT = Pattern.compile("^VmFlags: ([\\w\? ]*)");
        String start;

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

> 134:         String ps;
> 135:         String vmFlags;
> 136:         int lineno = 0;

Suggestion:

        int lineno;

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

> 159:             debug = true;
> 160:         } else {
> 161:             debug = false;

Let's keep the code shape with explicit initialization here? Also drop the init at the field declaration?

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

> 160:                 end = matSectionStart.group(2);
> 161:                 ps = null;
> 162:                 vmFlags = null;

I can see that `Range` code handles `vmFlags = null` specifically. Does it handle `ps == null`? I cannot see it.

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

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