8285965: TestScenarios.java does not check for "<!-- safepoint while printing -->" correctly

[chhagedorn]

This is another rare occurrence of <!-- safepoint while printing --> that is not handled correctly by TestScenarios.java.

We wrongly search this safepoint message in the test VM output with getTestVMOutput():

jdk/test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/Utils.java

Lines 44 to 53 in 9c25484

	public static boolean anyBailedOut(Scenario... scenarios) {
	return Arrays.stream(scenarios).anyMatch(s -> s.getTestVMOutput().contains(IRMatcher.SAFEPOINT_WHILE_PRINTING_MESSAGE));
	}
	/**
	* Is there at least one scenario which did not hit a safepoint while printing (i.e. a bailout)?
	*/
	public static boolean notAllBailedOut(Scenario... scenarios) {
	return Arrays.stream(scenarios).anyMatch(s -> !s.getTestVMOutput().contains(IRMatcher.SAFEPOINT_WHILE_PRINTING_MESSAGE));
	}

But this does not help since the IR matcher is parsing the hotspot_pid file for IR matching and not the test VM output. We could therefore find this safepoint message in the hotspod_pid file and bail out of IR matching while the test VM output does not contain it. This lets TestScenarios.java fail.

The fix we did for other IR framework tests is to redirect the output of the JTreg test VM itself to a stream in order to search it for <!-- safepoint while printing -->. We are dumping this message as part of a warning when the IR matcher bails out:

jdk/test/hotspot/jtreg/compiler/lib/ir_framework/driver/irmatching/IRMatcher.java

Lines 86 to 96 in 9c25484

	// In some very rare cases, the VM output to regex match on contains "<!-- safepoint while printing -->"
	// (emitted by ttyLocker::break_tty_for_safepoint) which might be the reason for a matching error.
	// Do not throw an exception in this case (i.e. bailout).
	private void throwIfNoSafepointWhilePrinting(String failures, String compilations) {
	if (!compilations.contains(SAFEPOINT_WHILE_PRINTING_MESSAGE)) {
	throw new IRViolationException(failures, compilations);
	} else {
	System.out.println("Found " + SAFEPOINT_WHILE_PRINTING_MESSAGE + ", bail out of IR matching");
	}
	}
	}

Output for the reported failure:

Scenario #3 - [-XX:TLABRefillWasteFraction=53]:
[...]
Found <!-- safepoint while printing -->, bail out of IR matching

I suggest to use the same fix for TestScenarios.

Thanks,
Christian

---

Progress

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

Issue

• JDK-8285965: TestScenarios.java does not check for "" correctly

Reviewers

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

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8647

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8647.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 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
• hotspot
• hotspot-compiler
• hotspot-gc
• hotspot-jfr
• hotspot-runtime
• i18n
• ide-support
• javadoc
• jdk
• jmx
• kulla
• net
• nio
• security
• serviceability
• shenandoah

[chhagedorn]

/label add hotspot-compiler

[openjdk]

@chhagedorn
The hotspot-compiler label was successfully added.

[mlbridge]

Webrevs

• 00: Full (476b5f5f)

[TobiHartmann]

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:

8285965: TestScenarios.java does not check for "<!-- safepoint while printing -->" correctly

Reviewed-by: thartmann, kvn

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

• 9ec27d0: 8287872: Disable concurrent execution of hotspot docker tests
• 91e6bf6: 8287919: ProblemList java/lang/CompressExpandTest.java
• 3da7e39: 8287840: Dead copy region node blocks IfNode's fold-compares
• c41a283: 8273853: Update the Java manpage for automatic CDS archive updating
• 41d5809: 8287811: JFR: jfr configure error message should not print stack trace
• 5d4ea9b: 8273346: Expand library mappings to IEEE 754 operations
• 96641c0: 8287745: jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
• 67f1bd7: 8286620: Create regression test for verifying setMargin() of JRadioButton
• 062db59: 8286206: Missing cases for RECORD
• ee4a6c2: 8287799: JFR: Less noisy platform threads with jfr print
• ... and 465 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.

[chhagedorn]

Thanks Tobias for your review!

[chhagedorn]

May I get a second review for this?

[vnkozlov]

Good.

[chhagedorn]

Thanks Vladimir for your review!

[chhagedorn]

/integrate

[openjdk]

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

• d959c22: 8288000: compiler/loopopts/TestOverUnrolling2.java fails with release VMs
• 230726e: 8287735: Provide separate event category for dll operations
• ecf0078: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
• 5c39a36: 8287522: StringConcatFactory: Add in prependers and mixers in batches
• 47d3c2a: 8287980: Build is broken due to SuperWordMaxVectorSize when C2 is disabled after JDK-8287697
• bf0e625: 8286451: C2: assert(nb == 1) failed: only when the head is not shared
• 0960ecc: 8287700: C2 Crash running eclipse benchmark from Dacapo
• 590337e: 8286625: C2 fails with assert(!n->is_Store() && !n->is_LoadStore()) failed: no node with a side effect
• 4662e06: 8277307: Pre shared key sent under both session_ticket and pre_shared_key extensions
• 7df48f9: 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException
• ... and 486 more: https://git.openjdk.java.net/jdk/compare/4f5d73f2d411aa6147c5388b024e0d2996378d5a...master

Your commit was automatically rebased without conflicts.

[openjdk]

@chhagedorn Pushed as commit 6e3e470.

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