8284115: [IR Framework] Compilation is not found due to rare safepoint while dumping PrintIdeal/PrintOptoAssembly

[chhagedorn]

This is yet another manifestation of the safepointing problem while printing a PrintIdeal/PrintOptoAssembly block.

In this case here, a safepoint is done and the <!-- safepoint while printing --> message is emitted while dumping a PrintIdeal block of retainDenominator() inside the hotspot_pid file. During this interruption, another test class method is enqueued for compilation which is logged to the hotspot_pid file before the printing of the PrintIdeal block resumes:

# PrintIdeal output of retainDenominator()
 3 Start === 3 0 [[ 3 5 6 7 8 9 13 11 ]] #{0:control, 1:abIO, 2:memory, 3:rawptr:BotPTR, 4:return_address, 5:compiler/c2/irTests/DivLNodeIdealizationTests:NotNull *, 6:long, 7:half, 8:long, 9:half}
 36 CallStaticJava === 34 6 7 8 9 ( 35 1 1 1 1 1 26 1 27 1 ) [[ 37 ]] # Static uncommon_trap(reason=&apos;div0_check&apos; action=&apos;maybe_recompile&apos; debug_id=&apos;0&apos;) void ( int ) C=0.000100 DivLNodeIdealizationTests::retainDenominator @ bci:4 (line 130) !jvms: DivLNodeIdealizationTests::retainDenominator<!-- safepoint while printing -->

# Safepoint interruption
<writer thread='40451'/>

# Enqueuing of another test class method identityThird()
<task_queued compile_id='205' method='compiler.c2.irTests.DivLNodeIdealizationTests identityThird (JJ)J' bytes='6' count='6000' iicount='6000' blocking='1' stamp='1.046' comment='whitebox' hot_count='6000'/>
<writer thread='23811'/>
 @ bci:4 (line 130)

# Continue to dump PrintIdeal of retainDenominator()
 41 DivL === 33 26 13 [[ 42 ]] !jvms: DivLNodeIdealizationTests::retainDenominator @ bci:4 (line 130)
 9 Parm === 3 [[ 42 36 ]] ReturnAdr !jvms: DivLNodeIdealizationTests::retainDenominator @ bci:-1 (line 130)

The HotSpotPidFileParser looks for these enqueue messages containing the method name in order to find and correctly map the corresponding PrintIdeal and PrintOptoAssembly outputs. However, the HotSpotPidFileParser does not expect such an enqueuing message to be found inside a PrintIdeal/PrintOptoAssembly block and thus ignores it. As a result, we later do not parse the PrintIdeal and PrintOptoAssembly output of the enqueued method during the safepoint and fail with the assertion that we did not find any compilation output for the method.

In the example above, the assertion says that we did not find the compilation output of identityThird() whose enqueue message was ignored inside the PrintIdeal block of retainDominator().

The proposed fix is to make HotSpotPidFileParser aware of the possibility of a safepoint while reading the PrintIdeal or PrintOptoAssembly output and therefore add a check if there was a method enqueued for compilation while reading inside BlockOutputReader::readBlock().

Thanks,
Christian

---

Progress

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

Issue

• JDK-8284115: [IR Framework] Compilation is not found due to rare safepoint while dumping PrintIdeal/PrintOptoAssembly

Reviewers

• Vladimir Kozlov (@vnkozlov - Reviewer)
• Tobias Hartmann (@TobiHartmann - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8692

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

Using diff file

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

[bridgekeeper]

👋 Welcome back chagedorn! 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]

@chhagedorn 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

• 00: Full (e5e1c787)

[vnkozlov]

I am concern that PrintIdeal output is interrupted by output from other threads. It may cause other issues in future again. Can we redirect PrintIdeal output into a separate file or reorder output like LogCompilation since it is used for IR testing now (automatic tool)? Originally it was not matter since such output was not used in any tool.

[chhagedorn]

Thanks for your review Vladimir!

I am concern that PrintIdeal output is interrupted by output from other threads. It may cause other issues in future again. Can we redirect PrintIdeal output into a separate file or reorder output like LogCompilation since it is used for IR testing now (automatic tool)? Originally it was not matter since such output was not used in any tool.

Can you elaborate more on what you mean by reordering the LogCompilation output?

I agree that we could make this safer by dumping PrintIdeal and PrintOptoAssembly to a separate file. However, this would require a change to the VM to generate the new file which probably also needs to be documented and might be a security concern? With this new file, I think we should also add a new flag to enable/disable such a redirection because it will only be used by the IR framework. Since this kind of bug is quite rare and only concerns the IR framework, I'm not sure if it would be justified to make such a change to the VM. The current fix seems to be less invasive and hopefully now completes the entire handling of safepoints while printing. What do you think?

[TobiHartmann]

I would also prefer to not introduce another log file just for IR matching. Together with #8647, this should hopefully fix all issues related to safepointing while printing. The change looks good to me.

[openjdk]

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

8284115: [IR Framework] Compilation is not found due to rare safepoint while dumping PrintIdeal/PrintOptoAssembly

Reviewed-by: kvn, thartmann

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

• c044cb8: 8286399: Address possibly lossy conversions in JDK Build Tools
• b884db8: 8285844: TimeZone.getTimeZone(ZoneOffset) does not work for all ZoneOffsets and returns GMT unexpected
• dbd3737: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF
• 743c779: 8286390: Address possibly lossy conversions in jdk.incubator.foreign moved to java.base
• 22139c3: 8286704: G1: Call offset_of directly in subclasses of G1CardSetContainer
• 77dfbb4: 8178701: Compile error with switch statement on protected enum defined in parent inner class
• f4258a5: 8209137: Add ability to bind to specific local address to HTTP client
• 65da38d: 8284585: PushPromiseContinuation test fails intermittently in timeout
• 652044d: 8286297: G1: Simplify parallel and serial verification code paths
• 0155e4b: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview)
• ... and 128 more: https://git.openjdk.java.net/jdk/compare/4f5d73f2d411aa6147c5388b024e0d2996378d5a...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.

[vnkozlov]

Okay if you say so.

[chhagedorn]

Thanks Tobias and Vladimir for your reviews! If we find more problems in the future or other limitations, we can still come back to this question if we want to introduce a new file.

[chhagedorn]

/integrate

[openjdk]

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

• b434b1f: 8286808: Loom: Simplify generate_cont_thaw by passing thaw_kind directly
• 6a77093: 8283544: HttpClient GET method adds Content-Length: 0 header
• ac41b78: 8284367: JQuery UI upgrade from 1.12.1 to 1.13.1
• 0948c09: 8272094: compiler/codecache/TestStressCodeBuffers.java crashes with "failed to allocate space for trampoline"
• 8c97705: 8286475: Drop --enable-preview from instanceof pattern matching related tests
• 63cace7: 8286660: codestrings gtest fails on AArch64: "udf" in padding
• 5e5500c: 6782021: It is not possible to read local computer certificates with the SunMSCAPI provider
• d65fba4: 8286452: The array length of testSmallConstArray should be small and const
• 125efe6: 8286744: failure_handler: dmesg command on macos fails to collect data due to permission issues
• 40f4dab: 8286756: Cleanup foreign API benchmarks
• ... and 142 more: https://git.openjdk.java.net/jdk/compare/4f5d73f2d411aa6147c5388b024e0d2996378d5a...master

Your commit was automatically rebased without conflicts.

[openjdk]

@chhagedorn Pushed as commit 3984253.

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