8272970: Parallelize runtime/InvocationTests/ #5250

shipilev · 2021-08-25T11:58:27Z

Current runtime/InvocationTests/ are long and serial. They take about half an hour on a modern desktop, mostly using a single CPU. They can be parallelized better, at least on sub-test level. This would be important as JDK-8272914 would start to run them in tier3.

Sample run times:

$ time CONF=linux-x86_64-server-fastdebug make run-test TEST=runtime/InvocationTests/

# Before
real	32m2.188s
user	59m12.818s
sys	0m45.612s

# After
real	17m21.110s
user	63m31.851s
sys	0m55.391s

I think we can technically go even deeper, for example parallelising by packageSet in the generators themselves, but that would be much more intrusive. I think 17 minutes is already quite good, given other tests running in prospective hotspot:tier3.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8272970: Parallelize runtime/InvocationTests/

Reviewers

David Holmes (@dholmes-ora - Reviewer)
Igor Ignatyev (@iignatev - Reviewer) ⚠️ Review applies to 3651efa

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5250/head:pull/5250
$ git checkout pull/5250

Update a local copy of the PR:
$ git checkout pull/5250
$ git pull https://git.openjdk.java.net/jdk pull/5250/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5250

View PR using the GUI difftool:
$ git pr show -t 5250

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5250.diff

bridgekeeper · 2021-08-25T11:58:51Z

👋 Welcome back shade! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-08-25T12:00:59Z

@shipilev The following label will be automatically applied to this pull request:

hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2021-08-25T12:04:45Z

Webrevs

01: Full - Incremental (f16b642d)
00: Full (3651efa3)

dholmes-ora · 2021-08-26T02:38:36Z

Hi Aleksey,

I can't find any jtreg documentation that indicates that multiple @test entries within a single Java file will be run concurrently. ??

That aside the split you have done seem quite reasonable.

Thanks,
David

openjdk · 2021-08-26T02:42:24Z

@shipilev This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8272970: Parallelize runtime/InvocationTests/

Reviewed-by: dholmes, iignatyev

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 33 new commits pushed to the master branch:

0e14bf7: 8273176: handle latest VS2019 in abstract_vm_version
f1c5e26: 8273206: jdk/jfr/event/gc/collection/TestG1ParallelPhases.java fails after JDK-8159979
e600fe1: 8272618: Unnecessary Attr.visitIdent.noOuterThisPath
2fce7cb: 8272963: Update the java manpage markdown source
18a731a: 8269770: nsk tests should start IOPipe channel before launch debuggee - Debugee.prepareDebugee
9c392d0: 8273197: ProblemList 2 jtools tests due to JDK-8273187
3d657eb: 8262186: Call X509KeyManager.chooseClientAlias once for all key types
c1e0aac: 8273186: Remove leftover comment about sparse remembered set in G1 HeapRegionRemSet
683e30d: 8273169: java/util/regex/NegativeArraySize.java failed after JDK-8271302
1996f64: 8273092: Sort classlist in JDK image
... and 23 more: https://git.openjdk.java.net/jdk/compare/f55d5ab5177b6e08e8499abc181a320a98b28a5f...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

shipilev · 2021-08-26T07:11:01Z

I can't find any jtreg documentation that indicates that multiple @test entries within a single Java file will be run concurrently. ??

Not sure how well the parallelization aspect is documented, but multiple @test blocks per file imply unique tests, under different labels, which by extension means they can execute in parallel. We use this technique often in the OpenJDK tests :)

dholmes-ora · 2021-08-26T07:28:57Z

Sure but uniquely named tests just gives uniquely named output directories, otherwise they'd overwrite each other. It says nothing about concurrent execution. I'm not saying it doesn't execute concurrently as apparently it does, but it is not documented and certainly not something that can easily be inferred from the docs. It is easy to imagine each file being processed in parallel, but somewhat more involved to parallelize the processing of each test within a file.

I did an experiment running a multi-test with -concurrency:1 and -concurrency:16 and it made no difference to real time consumed.

shipilev · 2021-08-27T09:49:36Z

I did an experiment running a multi-test with -concurrency:1 and -concurrency:16 and it made no difference to real time consumed.

Weird. As I said, we use that technique every so often in other tests. Look:

$ cat ParallelTest.java 
/*
 * @test
 * @run main/othervm ParallelTest
 */

/*
 * @test
 * @run main/othervm ParallelTest
 */

/*
 * @test
 * @run main/othervm ParallelTest
 */

/*
 * @test
 * @run main/othervm ParallelTest
 */
public class ParallelTest {
  public static void main(String... args) throws Throwable{
     Thread.sleep(10_000);
  }
}

$ time ~/Install/jtreg-6+1/bin/jtreg -concurrency:1 -verbose .
runner starting test: ParallelTest.java#id0
runner finished test: ParallelTest.java#id0
Passed. Execution successful
runner starting test: ParallelTest.java#id1
runner finished test: ParallelTest.java#id1
Passed. Execution successful
runner starting test: ParallelTest.java#id2
runner finished test: ParallelTest.java#id2
Passed. Execution successful
runner starting test: ParallelTest.java#id3
runner finished test: ParallelTest.java#id3
Passed. Execution successful
Test results: passed: 4
Report written to /home/shade/temp/jtreg/JTreport/html/report.html
Results written to /home/shade/temp/jtreg/JTwork

real	0m41.839s
user	0m3.885s
sys	0m0.367s

$ time ~/Install/jtreg-6+1/bin/jtreg -concurrency:4 -verbose .
runner starting test: ParallelTest.java#id0
runner starting test: ParallelTest.java#id2
runner starting test: ParallelTest.java#id3
runner starting test: ParallelTest.java#id1
runner finished test: ParallelTest.java#id3
Passed. Execution successful
runner finished test: ParallelTest.java#id2
Passed. Execution successful
runner finished test: ParallelTest.java#id0
Passed. Execution successful
runner finished test: ParallelTest.java#id1
Passed. Execution successful
Test results: passed: 4
Report written to /home/shade/temp/jtreg/JTreport/html/report.html
Results written to /home/shade/temp/jtreg/JTwork

real	0m11.556s
user	0m7.118s
sys	0m0.524s

dholmes-ora · 2021-08-27T13:13:08Z

I wonder whether "othervm" versus "driver" mode makes the difference here?

shipilev · 2021-08-27T13:20:04Z

I wonder whether "othervm" versus "driver" mode makes the difference here?

It does not matter here. @run driver ParallelTest also runs all four configs in parallel, in the example above.

shipilev · 2021-08-31T10:38:34Z

Anyone else wants to take a look? @iignatev, maybe?

iignatev

LGTM. one nit: it should be possible to reduce clutter by removing @compile actions, could you please either do that here or file an RFE?

PS I wish jtreg had a way not to duplicate all these tags, perhaps w/ some kind of a preprocessor.

dholmes-ora · 2021-09-01T04:20:28Z

@iignatev At least some of the compile commands must remain. The top level test doesn't reference the actual test classes directly so must explicitly compile them. Any classes those directly reference do not need an explicit compile-command.

shipilev · 2021-09-01T07:56:41Z

LGTM. one nit: it should be possible to reduce clutter by removing @compile actions, could you please either do that here or file an RFE?

Right. The generators are referencing the shared/ stuff implicitly, so we can drop @compile for them. Tests still pass without them. See new commit.

shipilev · 2021-09-02T07:56:06Z

Thanks, I think this is good to go in.

/integrate

openjdk · 2021-09-02T07:57:04Z

Going to push as commit 6cfe314.
Since your change was applied there have been 42 commits pushed to the master branch:

a9a83b2: 8273256: runtime/cds/appcds/TestEpsilonGCWithCDS.java fails due to Unrecognized VM option 'ObjectAlignmentInBytes=64' on x86_32
1a5a2b6: 8271745: Correct block size for KW,KWP mode and use fixed IV for KWP mode for SunJCE
2f01a6f: 8273157: Add convenience methods to Messager
9689f61: 8272788: Nonleaf ranked locks should not be safepoint_check_never
4ee0dac: 8273248: ProblemList java/lang/instrument/BootClassPath/BootClassPathTest.sh on all configs
655ea6d: 8270489: Support archived heap objects in EpsilonGC
dacd197: 8273217: Make ParHeapInspectTask _safepoint_check_never
02822e1: 8272377: assert preconditions that are ensured when created in add_final_edges
a58cf16: 8272563: assert(is_double_stack() && !is_virtual()) failed: type check
0e14bf7: 8273176: handle latest VS2019 in abstract_vm_version
... and 32 more: https://git.openjdk.java.net/jdk/compare/f55d5ab5177b6e08e8499abc181a320a98b28a5f...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-09-02T07:57:19Z

@shipilev Pushed as commit 6cfe314.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8272970: Parallelize runtime/InvocationTests/

3651efa

openjdk bot added the rfr label Aug 25, 2021

openjdk bot added the hotspot-runtime label Aug 25, 2021

dholmes-ora approved these changes Aug 26, 2021

View changes

openjdk bot added the ready label Aug 26, 2021

iignatev approved these changes Aug 31, 2021

View changes

shipilev added 2 commits Sep 1, 2021

Trim down redundant compile tags

27e6127

Merge branch 'master' into JDK-8272970-parallel-invocation-tests

f16b642

dholmes-ora approved these changes Sep 1, 2021

View changes

openjdk bot closed this Sep 2, 2021

openjdk bot added integrated and removed ready rfr labels Sep 2, 2021

shipilev deleted the JDK-8272970-parallel-invocation-tests branch Sep 13, 2021

8272970: Parallelize runtime/InvocationTests/ #5250

8272970: Parallelize runtime/InvocationTests/ #5250

shipilev commented Aug 25, 2021 •

edited by openjdk[bot] bot

bridgekeeper bot commented Aug 25, 2021

openjdk bot commented Aug 25, 2021

mlbridge bot commented Aug 25, 2021 •

edited

dholmes-ora commented Aug 26, 2021

openjdk bot commented Aug 26, 2021 •

edited

shipilev commented Aug 26, 2021

dholmes-ora commented Aug 26, 2021

shipilev commented Aug 27, 2021

dholmes-ora commented Aug 27, 2021

shipilev commented Aug 27, 2021

shipilev commented Aug 31, 2021 •

edited

iignatev left a comment

dholmes-ora commented Sep 1, 2021

shipilev commented Sep 1, 2021

shipilev commented Sep 2, 2021

openjdk bot commented Sep 2, 2021

openjdk bot commented Sep 2, 2021

8272970: Parallelize runtime/InvocationTests/ #5250

8272970: Parallelize runtime/InvocationTests/ #5250

Conversation

shipilev commented Aug 25, 2021 • edited by openjdk[bot] bot

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Aug 25, 2021

openjdk bot commented Aug 25, 2021

mlbridge bot commented Aug 25, 2021 • edited

Webrevs

dholmes-ora commented Aug 26, 2021

openjdk bot commented Aug 26, 2021 • edited

shipilev commented Aug 26, 2021

dholmes-ora commented Aug 26, 2021

shipilev commented Aug 27, 2021

dholmes-ora commented Aug 27, 2021

shipilev commented Aug 27, 2021

shipilev commented Aug 31, 2021 • edited

iignatev left a comment

dholmes-ora commented Sep 1, 2021

shipilev commented Sep 1, 2021

shipilev commented Sep 2, 2021

openjdk bot commented Sep 2, 2021

openjdk bot commented Sep 2, 2021

shipilev commented Aug 25, 2021 •

edited by openjdk[bot] bot

mlbridge bot commented Aug 25, 2021 •

edited

openjdk bot commented Aug 26, 2021 •

edited

shipilev commented Aug 31, 2021 •

edited