8282509: [exploded image] ResolvedClassTest fails with similar output

[backwaterred]

This is a tentative solution to a failure observed on AIX. The solution is tentative because I require help to ensure that something deeper and more problematic is not happening.

The test fails because the output produced by PrintCompilation produces LambdaForm$MH/0x00000007c0002400 instead of Invokers$Holder as it does on other platforms. There is one other place the output is different, when DirectMethodHandle$Holder is replaced with LambdaForm$DMH/0x00000007c0001c00. Ignoring these name changes, the output of PrintCompilation is identical. I observe the same compilations (including the OSR/non-OSR, and same level) in the same order.

I would be grateful for help understanding the root of the difference behind the change. I have a few ideas, but I will let you build your own interpretations free from my potentially incorrect understanding (i.e. no spoilers). Thanks in advance.

---

Progress

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

Issue

• JDK-8282509: [exploded image] ResolvedClassTest fails with similar output

Reviewers

• Martin Doerr (@TheRealMDoerr - Reviewer)
• Dean Long (@dean-long - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7701

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

Using diff file

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

[bridgekeeper]

👋 Welcome back backwaterred! 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.

[backwaterred]

Dean Long mentioned on the issue that they were able to reproduce the issue with an exploded image. I don't have access to JBS (yet!) to comment there directly, so I'll mention here that I am building with make images, and not the default target.

[openjdk]

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

• hotspot-compiler

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]

Webrevs

• 02: Full (0455aff6)
• 01: Full - Incremental (4149b86c)
• 00: Full (8e77ff1b)

[backwaterred]

I believe I have a better handle on this now. In a lucky break, some logging output I added ended up in build/.../images/jdk/lib/classlist and caused a build failure on x86. The same failure (with the same logging code) did not occur on AIX. I now believe the difference in test output is due to CDS, which I am surprised to learn is not implemented on AIX.

My working understanding is as follows. For systems which support CDS, PrintCompilation sees the code as coming from Invokers$Holder in Invokers.java where is is shared among JVM instances. On AIX, CDS is not (fully?) implemented, and the code is not stored in Invokers$Holder. Instead, the code is referred to by a LambdaForm at the displayed memory address.

With this piece of the puzzle in place (and hopefully confirmed by others), I believe the check proposed here to be a sane choice to resolve this failure.

[TheRealMDoerr]

Looks reasonable. Please update the PR title: "[exploded image] ResolvedClassTest fails with similar output"

[backwaterred]

Thanks for the review @TheRealMDoerr. Should I wait for another review, or is one sufficient in this case?

---

For future reference, it looks to me that this failure is related to missing pre-generated invokers. As I understand it, these are either not created, or just not linked in the exploded image, and are never created on AIX.

[TheRealMDoerr]

I've added your additional hint to the issue and linked both issues. Thanks for your investigation.
Your fix could be considered as trivial, but the underlying issue is not. So, please wait for a 2nd review!
The test name in title is not yet correct: Please use "ResolvedClassTest"

[openjdk]

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

8282509: [exploded image] ResolvedClassTest fails with similar output

Reviewed-by: mdoerr, dlong

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

• 9c88c5b: 8282948: JDK-8274980 missed correct handling of MACOSX_BUNDLE_BUILD_VERSION
• 83d7718: 8282893: Remove MacroAssembler::push/pop_callee_saved_registers
• 6a3a7b9: 6218162: DefaultTableColumnModel.getColumn() method should mention ArrayIndexOutOfBoundsException
• 5b78a82: 7017094: ParsedSynthStyle: parameter name "direction" should be changed to "tabIndex"
• 8aba4de: 8249592: Robot.mouseMove moves cursor to incorrect location when display scale varies and Java runs in DPI Unaware mode
• ff76620: 8282641: Make jdb "threadgroup" command with no args reset the current threadgroup back to the default
• 70318e1: 8282884: Provide OID aliases for MD2, MD5, and OAEP
• 6d8d156: 8280494: (D)TLS signature schemes
• 5df2a05: 8282628: Potential memory leak in sun.font.FontConfigManager.getFontConfig()
• d07f7c7: 8282665: [REDO] ByteBufferTest.java: replace endless recursion with RuntimeException in void ck(double x, double y)
• ... and 28 more: https://git.openjdk.java.net/jdk/compare/f0995abe62b81cf9c96cc07caa0ac27d00c96ff1...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 (@TheRealMDoerr, @dean-long) 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).

[dean-long]

This does seem like the best fix.

[backwaterred]

Thanks for the reviews.

/integrate

[openjdk]

@backwaterred
Your change (at version 0455aff) is now ready to be sponsored by a Committer.

[TheRealMDoerr]

/sponsor

[openjdk]

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

• 9c88c5b: 8282948: JDK-8274980 missed correct handling of MACOSX_BUNDLE_BUILD_VERSION
• 83d7718: 8282893: Remove MacroAssembler::push/pop_callee_saved_registers
• 6a3a7b9: 6218162: DefaultTableColumnModel.getColumn() method should mention ArrayIndexOutOfBoundsException
• 5b78a82: 7017094: ParsedSynthStyle: parameter name "direction" should be changed to "tabIndex"
• 8aba4de: 8249592: Robot.mouseMove moves cursor to incorrect location when display scale varies and Java runs in DPI Unaware mode
• ff76620: 8282641: Make jdb "threadgroup" command with no args reset the current threadgroup back to the default
• 70318e1: 8282884: Provide OID aliases for MD2, MD5, and OAEP
• 6d8d156: 8280494: (D)TLS signature schemes
• 5df2a05: 8282628: Potential memory leak in sun.font.FontConfigManager.getFontConfig()
• d07f7c7: 8282665: [REDO] ByteBufferTest.java: replace endless recursion with RuntimeException in void ck(double x, double y)
• ... and 28 more: https://git.openjdk.java.net/jdk/compare/f0995abe62b81cf9c96cc07caa0ac27d00c96ff1...master

Your commit was automatically rebased without conflicts.

[openjdk]

@TheRealMDoerr @backwaterred Pushed as commit 7c8ea9f.

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