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

Stefan Johansson sjohanss at openjdk.java.net
Tue May 18 11:33:45 UTC 2021


On Tue, 18 May 2021 06:53:19 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:
> 
>  - Update test/hotspot/jtreg/runtime/os/TestTracePageSizes.java
>    
>    Co-authored-by: Aleksey Shipilëv <shade at redhat.com>
>  - Update test/hotspot/jtreg/runtime/os/TestTracePageSizes.java
>    
>    oops
>    
>    Co-authored-by: Aleksey Shipilëv <shade at redhat.com>

Added some initial comments to the code.

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 + " to " + p2 + "...");

Consider using `debug(...)` for this and the other `System.out.println(..)`calls.

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

> 117:     // will generate a match.
> 118:     private static void parseSmaps() throws Exception {
> 119:         // We can override the smaps file to parse to pass in a pre-fetched one

Please update the above comment to match the new parsing.

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

> 309:         // * ht - Meaning the range is mapped using explicit huge pages.
> 310:         // * hg - Meaning the range is madvised huge.
> 311:         if (vmFlags != null) {

Update comment to explain the smaps sometimes don't have `vmFlags`.

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

Changes requested by sjohanss (Reviewer).

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


More information about the hotspot-runtime-dev mailing list