8342862: JDK-8339507 appears to be causing 8GB build machines to hang

[hanklo6]

We generate each test randomly. The asmtest.out.h is currently around 100 KB. We have added the --full argument to allow users to create a complete test set.

---

Progress

• [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue

Issue

• JDK-8342862: JDK-8339507 appears to be causing 8GB build machines to hang (Bug - P2)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21670/head:pull/21670
$ git checkout pull/21670

Update a local copy of the PR:
$ git checkout pull/21670
$ git pull https://git.openjdk.org/jdk.git pull/21670/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21670

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21670.diff

Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back hanklo6! 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[bot]]

@hanklo6 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:

8342862: Gtest added by 8339507 appears to be causing 8GB build machines to hang

Reviewed-by: kvn, sviswanathan

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 29 new commits pushed to the master branch:

• 3c14c2babbdfb46a77636ed80e083ef2f8be2b45: 8341566: Add Reader.of(CharSequence)
• b0ac633b2d0076d64b463b2a6ce19abf6b12c50f: 8342075: HttpClient: improve HTTP/2 flow control checks
• 85774b713edf8782f162ac25b61ce99a77e116f4: 8342882: RISC-V: Unify handling of jumps to runtime
• 2c31c8eeb42188ad6fd15eca50db4342cd791fb2: 8339730: Windows regression after removing ObjectMonitor Responsible
• f0b130e54f33d3190640ce33c991e35f27e9f812: 8339296: Record deconstruction pattern in switch fails to compile
• e96b4cf0a81914c6a615bb4f62ea3f139a4737f3: 8342387: C2 SuperWord: refactor and improve compiler/loopopts/superword/TestDependencyOffsets.java
• f7a61fce949ea2d15ec09d295c643d0c2eabea1e: 8342931: ProblemList failing tests from JDK-8335912
• 25c2f48d458bfd92423c311a887679ad3e1e4041: 8338544: Dedicated Array class descriptor implementation
• 158b93d19a518d2b9d3d185e2d4c4dbff9c82aab: 8335912: Add an operation mode to the jar command when extracting to not overwriting existing files
• 28d23ada6dde007ed60b8538cc159afc62d76db3: 8340177: Malformed system classes loaded by bootloader crash the JVM in product builds
• ... and 19 more: https://git.openjdk.org/jdk/compare/c61d2c5a3403ad5f48daad22bdfb031f42bc3dd5...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@vnkozlov, @sviswa7) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk[bot]]

@hanklo6 To determine the appropriate audience for reviewing this pull request, one or more labels corresponding to different subsystems will normally be applied automatically. However, no automatic labelling rule matches the changes in this pull request. In order to have an "RFR" email sent to the correct mailing list, you will need to add one or more applicable labels manually using the /label pull request command.

Applicable Labels

• build
• client
• compiler
• core-libs
• graal
• hotspot
• hotspot-compiler
• hotspot-gc
• hotspot-jfr
• hotspot-runtime
• i18n
• ide-support
• javadoc
• jdk
• jmx
• kulla
• net
• nio
• security
• serviceability
• shenandoah

[hanklo6]

/label hotspot-compiler

[openjdk[bot]]

@hanklo6 The hotspot-compiler label was successfully added.

[mlbridge[bot]]

Webrevs

• 01: Full - Incremental (e79a5a65)
• 00: Full (dae652eb)

[vnkozlov]

Please, change title of the bug in JBS in this PR to: "Gtest added by 8339507 appears to be causing 8GB build machines to hang"

It seems "JDK-8339507" at the beginning of title confuses our tools/bots to where add PR link and may cause issues later too.

[vnkozlov]

What if we split it into several files? Will combined compilation time be different? At least it will not consume a lot of memory and page swapping.

[hanklo6]

Please, update instructions comment in test_assemblerx86.cpp.

What subset of instructions will be generated by default? Did you compare compilation time between files with default and "full" sets of instructions.

@vnkozlov The default test set includes the original map0/map1 instructions with random registers, address, and immediate. I tested on my machine (i9-13900k with 32cores), with the compilation time being 10s for the default test and 30s for the full test.

[hanklo6]

What if we split it into several files? Will combined compilation time be different? At least it will not consume a lot of memory and page swapping.

But splitting into several files still results in large code size. We are trying to reduce the code size as well.

[dholmes-ora]

What if you split into separate test files (.cpp) as well so that rather than one gtest that checks everything you have several smaller ones. That will reduce compilation overhead as required and also allow full coverage, potentially running tests in parallel.

[sviswa7]

What if you split into separate test files (.cpp) as well so that rather than one gtest that checks everything you have several smaller ones. That will reduce compilation overhead as required and also allow full coverage, potentially running tests in parallel.

Do we really want that? Just to put things in perspective the original file size was 45963 lines and the reduced one is now 1293 lines so we are talking about 35 such files. What Hank has done in this PR is to test each instruction with random register, address, and immediate instead of all the combinations that he had before so we still have a decent coverage. The test generator tool continues to have the ability to generate the full combination optionally.

[TobiHartmann]

What Hank has done in this PR is to test each instruction with random register, address, and immediate instead of all the combinations that he had before so we still have a decent coverage. The test generator tool continues to have the ability to generate the full combination optionally.

I think that is good enough and preferable.

[hanklo6]

/integrate

[openjdk[bot]]

@hanklo6 Your change (at version e79a5a65fe2517dd79efb4ac38db4a34b9db9a6c) is now ready to be sponsored by a Committer.

[sviswa7]

/sponsor

[openjdk[bot]]

Going to push as commit 7d5eefa50673d6f7c5bd916f63271cf7898d6dee. Since your change was applied there have been 30 commits pushed to the master branch:

• d8c3b0f834c603fe115ef4ca442727948b7a834e: 8342768: GTest AssemblerX86.validate_vm failed: assert(VM_Version::supports_bmi1()) failed: tzcnt instruction not supported
• 3c14c2babbdfb46a77636ed80e083ef2f8be2b45: 8341566: Add Reader.of(CharSequence)
• b0ac633b2d0076d64b463b2a6ce19abf6b12c50f: 8342075: HttpClient: improve HTTP/2 flow control checks
• 85774b713edf8782f162ac25b61ce99a77e116f4: 8342882: RISC-V: Unify handling of jumps to runtime
• 2c31c8eeb42188ad6fd15eca50db4342cd791fb2: 8339730: Windows regression after removing ObjectMonitor Responsible
• f0b130e54f33d3190640ce33c991e35f27e9f812: 8339296: Record deconstruction pattern in switch fails to compile
• e96b4cf0a81914c6a615bb4f62ea3f139a4737f3: 8342387: C2 SuperWord: refactor and improve compiler/loopopts/superword/TestDependencyOffsets.java
• f7a61fce949ea2d15ec09d295c643d0c2eabea1e: 8342931: ProblemList failing tests from JDK-8335912
• 25c2f48d458bfd92423c311a887679ad3e1e4041: 8338544: Dedicated Array class descriptor implementation
• 158b93d19a518d2b9d3d185e2d4c4dbff9c82aab: 8335912: Add an operation mode to the jar command when extracting to not overwriting existing files
• ... and 20 more: https://git.openjdk.org/jdk/compare/c61d2c5a3403ad5f48daad22bdfb031f42bc3dd5...master

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@sviswa7 @hanklo6 Pushed as commit 7d5eefa50673d6f7c5bd916f63271cf7898d6dee.

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