8298215: gc/g1/TestVerifyGCType.java failed with "Missing expected verification pattern Verifying After GC for: Pause Young (Prepare Mixed): expected true, was false"

[kimbarrett]

Please review this fix to the calculation of the number of young regions to use.

That calculation was recently changed (JDK-8297186), but that change has a
problem. It ensures the desired number of young regions calculated at the end
of a collection includes at least one eden region, to ensure progress is made.
It accomplished this by ensuring the minimum length was the number of already
allocated regions + 1 when doing the calculation at the end of a GC. But for
a revision of the length during the mutator phase it intentionally didn't do
that, as we want to trigger a collection rather than forceably expand.

However, a revision may occur immediately after the GC (indeed that's fairly
likely after JDK-8137022).

A better calculation is to ensure the desired number of young regions is at
least the number of survivor regions + 1. (That's effectively what the old
code was doing in the end-of-GC calculation, since at that time all allocated
regions are survivor regions.) So that's the basic change. This also removes
the need to distinguish between updating for GC or during the mutator phase
that was introduced by JDK-8297186 (the "after-gc" argument).

Also changed the test to also include the subprocess log in the error message
when the check for verification messages fails, to aid any future debugging.

Testing:

mach5 tier1-3. This isn't all that revealing, since the test failure was very
intermittent.

Hand tested with my changes to address JDK-8293824. With just those changes
the test was failing pretty reliably (instead of intermittently). Adding the
changes proposed here lets that test (seemingly) reliably pass. (There might
still be some work to do for JDK-8293824.)

There are some naming and description comment improvements around
update_young_length_bounds that I'd like to make, but I'm deferring those as a
separate RFE and keeping this bug fix fairly minimal.

---

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-8298215: gc/g1/TestVerifyGCType.java failed with "Missing expected verification pattern Verifying After GC for: Pause Young (Prepare Mixed): expected true, was false"

Reviewers

• Ivan Walulya (@walulyai - Reviewer)
• Thomas Schatzl (@tschatzl - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 52

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk20/pull/52.diff

[bridgekeeper]

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

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

• hotspot-gc

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 (34ef4d34)

[walulyai]

Lgtm!

Pre-existing issue:
update_young_length_bounds at the end of gc and update_young_length_bounds in revise_young_list_target_length are called with different tuning parameters. Thus, they are often giving totally contradicting values for targets.

[openjdk]

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

8298215: gc/g1/TestVerifyGCType.java failed with "Missing expected verification pattern Verifying After GC for: Pause Young (Prepare Mixed): expected true, was false"

Reviewed-by: iwalulya, tschatzl

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 no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential 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.

[kimbarrett]

Pre-existing issue: update_young_length_bounds at the end of gc and update_young_length_bounds in revise_young_list_target_length are called with different tuning parameters. Thus, they are often giving totally contradicting values for targets.

Yes, and the update at the end of gc is almost a waste of time - since JDK-8137022 it will be revised almost immediately after the gc safepoint ends.

[kimbarrett]

Thanks @tschatzl and @walulyai for reviews.

[kimbarrett]

/integrate

[openjdk]

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

• d0a7679: 4958969: ObjectOutputStream example leads to non-working code
• f07acfc: 8298699: java/lang/reflect/IllegalArgumentsTest.java times out with slowdebug bits
• 3e17e3c: 4512626: Non-editable JTextArea provides no visual indication of keyboard focus
• 188aaef: 8277074: Qualified exported types show up in JavaDoc
• 2c69c41: 8298894: java/lang/Thread/virtual/stress/Skynet.java timed out and threw OutOfMemoryError

Your commit was automatically rebased without conflicts.

[openjdk]

@kimbarrett Pushed as commit ea40f29.

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