Skip to content

8342214: GenShen: Reduce code duplication in shFreeSet with iterator abstraction#143

Closed
earthling-amzn wants to merge 25 commits intoopenjdk:masterfrom
earthling-amzn:sync-up-shen-21u-with-upstream
Closed

8342214: GenShen: Reduce code duplication in shFreeSet with iterator abstraction#143
earthling-amzn wants to merge 25 commits intoopenjdk:masterfrom
earthling-amzn:sync-up-shen-21u-with-upstream

Conversation

@earthling-amzn
Copy link
Contributor

@earthling-amzn earthling-amzn commented Dec 11, 2024

These were almost all clean. All pipeline tests have passed.


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 Committer)

Issues

  • JDK-8342214: GenShen: Reduce code duplication in shFreeSet with iterator abstraction (Task - P4)
  • JDK-8342255: GenShen: Remove unnecessary enum initial values (Task - P4)
  • JDK-8342278: GenShen: Move non-generational mode test out of generational test configuration (Task - P4)
  • JDK-8342560: GenShen: Fix confusing method name (Task - P4)
  • JDK-8342564: GenShen: Only reference young/old generation names in generational mode (Task - P4)
  • JDK-8342580: GenShen: TestChurnNotifications fails executing in unintended test-id modes with ShenandoahGCMode=generational (Sub-task - P4)
  • JDK-8342848: Shenandoah: Marking bitmap may not be completely cleared in generational mode (Bug - P4)
  • JDK-8342919: GenShen: Fix whitespace (Task - P4)
  • JDK-8343227: GenShen: Fold resource mark into management of preselected regions (Task - P4)
  • JDK-8344151: GenShen: Improve comments, clean up unnecessary changes (Task - P4)
  • JDK-8344152: GenShen: Initialize RegionData in product builds (Task - P4)
  • JDK-8344260: GenShen: Excess unaffiliated regions reserved for old will not be transferred to young (Bug - P4)
  • JDK-8344264: GenShen: Improve comments and method names (Task - P4)
  • JDK-8344263: GenShen: Reduce extraneous log messages at INFO level (Task - P4)
  • JDK-8344339: GenShen: Avoid using namespaces (Task - P4)
  • JDK-8344320: GenShen: Possible null pointer usage in shGenerationalHeap (Bug - P4)
  • JDK-8344321: GenShen: Fix various sonar scan warnings (Bug - P4)
  • JDK-8344592: GenShen: Remove unnecessary comments and changes (Task - P4)
  • JDK-8344638: GenShen: Verifier should not touch claim token (Task - P4)
  • JDK-8344640: GenShen: Reuse existing card mark barrier function when dropping references (Task - P4)
  • JDK-8344670: GenShen: Use concurrent worker session for concurrent mark phase (Task - P4)
  • JDK-8344779: GenShen: Consolidate shared heap region closures into one header (Task - P4)
  • JDK-8344797: GenShen: Update and rename confusing method (Task - P4)
  • JDK-8344985: GenShen: Refactor arraycopy barrier for generational mode (Task - P4)
  • JDK-8342734: GenShen: Test failure gc/shenandoah/TestReferenceRefersToShenandoah.java#generational (Task - P4)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/shenandoah-jdk21u.git pull/143/head:pull/143
$ git checkout pull/143

Update a local copy of the PR:
$ git checkout pull/143
$ git pull https://git.openjdk.org/shenandoah-jdk21u.git pull/143/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 143

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/shenandoah-jdk21u/pull/143.diff

Using Webrev

Link to Webrev Comment

William Kemper and others added 25 commits December 9, 2024 11:38
…d test-id modes with ShenandoahGCMode=generational

Reviewed-by: wkemper, kdnilsen
Reviewed-by: xpeng, kdnilsen
…ot be transferred to young

Reviewed-by: ysr, shade, kdnilsen
@bridgekeeper
Copy link

bridgekeeper bot commented Dec 11, 2024

👋 Welcome back wkemper! 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
Copy link

openjdk bot commented Dec 11, 2024

@earthling-amzn 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:

8342214: GenShen: Reduce code duplication in shFreeSet with iterator abstraction
8342255: GenShen: Remove unnecessary enum initial values
8342278: GenShen: Move non-generational mode test out of generational test configuration
8342560: GenShen: Fix confusing method name
8342564: GenShen: Only reference young/old generation names in generational mode
8342580: GenShen: TestChurnNotifications fails executing in unintended test-id modes with ShenandoahGCMode=generational
8342848: Shenandoah: Marking bitmap may not be completely cleared in generational mode
8342919: GenShen: Fix whitespace
8343227: GenShen: Fold resource mark into management of preselected regions
8344151: GenShen: Improve comments, clean up unnecessary changes
8344152: GenShen: Initialize RegionData in product builds
8344260: GenShen: Excess unaffiliated regions reserved for old will not be transferred to young
8344264: GenShen: Improve comments and method names
8344263: GenShen: Reduce extraneous log messages at INFO level
8344339: GenShen: Avoid using namespaces
8344320: GenShen: Possible null pointer usage in shGenerationalHeap
8344321: GenShen: Fix various sonar scan warnings
8344592: GenShen: Remove unnecessary comments and changes
8344638: GenShen: Verifier should not touch claim token
8344640: GenShen: Reuse existing card mark barrier function when dropping references
8344670: GenShen: Use concurrent worker session for concurrent mark phase
8344779: GenShen: Consolidate shared heap region closures into one header
8344797: GenShen: Update and rename confusing method
8344985: GenShen: Refactor arraycopy barrier for generational mode
8342734: GenShen: Test failure gc/shenandoah/TestReferenceRefersToShenandoah.java#generational

Reviewed-by: ysr

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 1 new commit pushed to the master branch:

  • 562224c: 8346044: GenShen: ShenandoahGenerationalHeap entry missing from Universe.java

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

@earthling-amzn
Copy link
Contributor Author

/issue add 8342214,8342255,8342278,8342560,8342564,8342580,8342848,8342919,8343227,8344151,8344152,8344260,8344264,8344263,8344339,8344320,8344321,8344592,8344638,8344640,8344670,8344779,8344797,8344985,8342734

@openjdk openjdk bot changed the title Backport numerous commits from generational mode pull request 8342214: GenShen: Reduce code duplication in shFreeSet with iterator abstraction Dec 11, 2024
@openjdk
Copy link

openjdk bot commented Dec 11, 2024

@earthling-amzn The primary solved issue for a PR is set through the PR title. Since the current title does not contain an issue reference, it will now be updated.

Adding additional issue to issue list: 8342255: GenShen: Remove unnecessary enum initial values.

Adding additional issue to issue list: 8342278: GenShen: Move non-generational mode test out of generational test configuration.

Adding additional issue to issue list: 8342560: GenShen: Fix confusing method name.

Adding additional issue to issue list: 8342564: GenShen: Only reference young/old generation names in generational mode.

Adding additional issue to issue list: 8342580: GenShen: TestChurnNotifications fails executing in unintended test-id modes with ShenandoahGCMode=generational.

Adding additional issue to issue list: 8342848: Shenandoah: Marking bitmap may not be completely cleared in generational mode.

Adding additional issue to issue list: 8342919: GenShen: Fix whitespace.

Adding additional issue to issue list: 8343227: GenShen: Fold resource mark into management of preselected regions.

Adding additional issue to issue list: 8344151: GenShen: Improve comments, clean up unnecessary changes.

Adding additional issue to issue list: 8344152: GenShen: Initialize RegionData in product builds.

Adding additional issue to issue list: 8344260: GenShen: Excess unaffiliated regions reserved for old will not be transferred to young.

Adding additional issue to issue list: 8344264: GenShen: Improve comments and method names.

Adding additional issue to issue list: 8344263: GenShen: Reduce extraneous log messages at INFO level.

Adding additional issue to issue list: 8344339: GenShen: Avoid using namespaces.

Adding additional issue to issue list: 8344320: GenShen: Possible null pointer usage in shGenerationalHeap.

Adding additional issue to issue list: 8344321: GenShen: Fix various sonar scan warnings.

Adding additional issue to issue list: 8344592: GenShen: Remove unnecessary comments and changes.

Adding additional issue to issue list: 8344638: GenShen: Verifier should not touch claim token.

Adding additional issue to issue list: 8344640: GenShen: Reuse existing card mark barrier function when dropping references.

Adding additional issue to issue list: 8344670: GenShen: Use concurrent worker session for concurrent mark phase.

Adding additional issue to issue list: 8344779: GenShen: Consolidate shared heap region closures into one header.

Adding additional issue to issue list: 8344797: GenShen: Update and rename confusing method.

Adding additional issue to issue list: 8344985: GenShen: Refactor arraycopy barrier for generational mode.

Adding additional issue to issue list: 8342734: GenShen: Test failure gc/shenandoah/TestReferenceRefersToShenandoah.java#generational.

@openjdk openjdk bot added the rfr Pull request is ready for review label Dec 11, 2024
@mlbridge
Copy link

mlbridge bot commented Dec 11, 2024

Webrevs

@ysramakrishna
Copy link
Member

I did a cursory check and it looks good. As regards:

Issue is already resolved. Consider making this a "backport pull request" by setting the PR title to Backport with the hash of the original commit.

May be check if that works better and quiets the complaints of the robot overlords? I am guessing \backport takes only a single parameter, and multiple \backports can't be composed/nested.

Not sure what the right process is that is recommended here. May be @shipilev has a suggestion?

Copy link
Member

@ysramakrishna ysramakrishna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM based on a cursory glance at the changes, the correctness of the testing, and the clean-ness of the backports.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Dec 11, 2024
@earthling-amzn earthling-amzn changed the title 8342214: GenShen: Reduce code duplication in shFreeSet with iterator abstraction Backport 60c6dd9d274ae5b880406d4242f6fe0f6658d0c4 Dec 13, 2024
@openjdk openjdk bot changed the title Backport 60c6dd9d274ae5b880406d4242f6fe0f6658d0c4 8342214: GenShen: Reduce code duplication in shFreeSet with iterator abstraction Dec 13, 2024
@openjdk
Copy link

openjdk bot commented Dec 13, 2024

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added the backport Port of a pull request already in a different code base label Dec 13, 2024
@earthling-amzn
Copy link
Contributor Author

/integrate

@openjdk
Copy link

openjdk bot commented Dec 13, 2024

Going to push as commit 71c448f.
Since your change was applied there has been 1 commit pushed to the master branch:

  • 562224c: 8346044: GenShen: ShenandoahGenerationalHeap entry missing from Universe.java

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Dec 13, 2024
@openjdk openjdk bot closed this Dec 13, 2024
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 13, 2024
@openjdk
Copy link

openjdk bot commented Dec 13, 2024

@earthling-amzn Pushed as commit 71c448f.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Port of a pull request already in a different code base integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

3 participants