8371284: GenShen: Avoid unnecessary card marking

[nityarai08]

Exclude young-young, old-old and honor UseCondCardMark in dirty card marking.

---

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-8371284: GenShen: Avoid unnecessary card marking (Enhancement - P4)

Reviewers

• Aleksey Shipilev (@shipilev - Reviewer) 🔄 Re-review required (review applies to 123078b2)
• Y. Srinivas Ramakrishna (@ysramakrishna - Reviewer) 🔄 Re-review required (review applies to 123078b2)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28204

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

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

@nityarai08 This change is no longer ready for integration - check the PR body for details.

[openjdk]

@nityarai08 The following labels will be automatically applied to this pull request:

• hotspot-gc
• shenandoah

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 04: Full - Incremental (4054a52e)
• 03: Full - Incremental (123078b2)
• 02: Full - Incremental (1c85da72)
• 01: Full - Incremental (59f7a0d0)
• 00: Full (09ad25ab)

[earthling-amzn]

Let's make sure we don't mark cards for null objects.

[shipilev]

Generally looks fine, just tighten up the comments.

[shipilev]

Looks fine. How's performance? I suppose this is a frequent path, and is_in_young checks are not necessarily free.

[nityarai08]

We are seeing mixed results, I need to do more validation.

[nityarai08]

Here are GenShen results over 100+ sample collections

+1.90% philosophers/philosophers_duration p=0.00142 (Wilcoxon)
Control: 2.620s (+/-173.82ms) 120
Test: 2.670s (+/-205.24ms) 120

+1.68% finagle-chirper/finagle-chirper_duration p=0.00343 (Wilcoxon)
Control: 2.802s (+/-130.38ms) 360
Test: 2.850s (+/-131.25ms) 360

+1.04% rx-scrabble/rx-scrabble_duration p=0.00000 (Wilcoxon)
Control: 148.308ms (+/- 0.88ms) 320
Test: 149.855ms (+/- 0.60ms) 320

-2.48% scrabble/scrabble_duration p=0.00000 (Wilcoxon)
Control: 169.599ms (+/- 2.90ms) 200
Test: 165.399ms (+/- 3.40ms) 200

-2.24% scala-kmeans/scala-kmeans_duration p=0.00000 (Wilcoxon)
Control: 479.973ms (+/- 1.92ms) 200
Test: 469.219ms (+/- 1.98ms) 200

[ysramakrishna]

Description states:

Exclude young-young, old-old and honor UseCondCardMark in dirty card marking.

I don't see any change for the latter. I believe UseCondCardMark is already enabled with GenShen, and the barrier ode respects it.

Also Exclude young-young, old-old skips the part of the change that also skips young->old. I'd reword the synopsis to:

Skips card marks for stores in young generation objects, old -> old, and null stores.

which I think is what the code does.

In the performance comparisons, did you see any effect on remembered set scan times (there are card-scan stats that can be enabled in a "release" (non product) build, as needed).

Left some suggestions for tightening up documentation comment.

Rest looks good. As Aleksey noted, the conditional card marks can of course slow down the mutator, so we want to see if the savings in card scan times (because of skipping old->old, or null writes in old) would make up.