8234502: Merge GenCollectedHeap and SerialHeap #16927

LizBing · 2023-12-01T16:47:28Z

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-8234502: Merge GenCollectedHeap and SerialHeap (Enhancement - P4)

Reviewers

Albert Mingkun Yang (@albertnetymk - Reviewer)
Chris Plummer (@plummercj - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16927

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-12-01T16:48:19Z

👋 Welcome back LizBing! 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 · 2023-12-01T16:50:00Z

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

hotspot

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.

LizBing · 2023-12-01T16:54:24Z

/label add hotspot-gc

openjdk · 2023-12-01T16:55:49Z

@LizBing
The hotspot-gc label was successfully added.

mlbridge · 2023-12-01T16:58:20Z

Webrevs

LizBing · 2023-12-01T16:58:30Z

I wonder if the files in 'src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/' which contain the java class 'GenCollectedHeap' should also be modified in this pr.

albertnetymk · 2023-12-01T18:34:21Z

I wonder if the files in 'src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/' which contain the java class 'GenCollectedHeap' should also be modified in this pr.

They should reflect the actual type inside VM. Some can probably be done before this PR though, e.g. class PointerFinder -- it's unclear why we report extra location info for SerialGC only.

This should be enough for all GCs. (I don't think the additional info is that useful.)

if (heap.isIn(a)) {
  loc.heap = heap;
  return loc;
}

(There may be more examples that can be done before this large PR.)

LizBing · 2023-12-02T05:09:45Z

It's weird to see GathererTest::testMassivelyComposedGatherers fail when testing in linux-x86.

LizBing · 2023-12-02T05:31:51Z

#16928 found this.

openjdk · 2023-12-07T17:11:40Z

@LizBing this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8234502
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

albertnetymk

As you pointed it out previously, some files in src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ need to changed (before or inside this PR). The bottom line is that GenCollectedHeap should not exist in the codebase after this PR.

src/hotspot/share/gc/shared/space.cpp

bridgekeeper · 2024-01-06T09:06:10Z

@LizBing This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

albertnetymk · 2024-01-09T13:31:30Z

@LizBing Now that #16842 is merged, could you sync this PR to master?

albertnetymk

Running testing now. The patch mostly looks good.

src/hotspot/share/gc/shared/vmStructs_gc.hpp

albertnetymk · 2024-01-10T09:28:42Z

make test CONF=debug TEST=serviceability/sa/ClhsdbThreadContext.java JTREG_JAVA_OPTIONS=-XX:+UseG1GC causes "AssertionFailure: Should have unknown location" in my testing.

LizBing · 2024-01-10T15:50:27Z

make test CONF=debug TEST=serviceability/sa/ClhsdbThreadContext.java JTREG_JAVA_OPTIONS=-XX:+UseG1GC causes "AssertionFailure: Should have unknown location" in my testing.

Have a try again? I may fix this problem in the latest version. I wrongly moved 'isInTLAB()' to the upper if-statement before.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerLocation.java

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerFinder.java

src/hotspot/share/gc/shared/vmStructs_gc.hpp

plummercj

Sorry about the false alarms. Not noticing that vmStructs_serial.hpp was also updated threw me off. The changes look fine. I ran the SA tests locally and they passed.

openjdk · 2024-01-11T21:27:20Z

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

8234502: Merge GenCollectedHeap and SerialHeap

Reviewed-by: ayang, cjplummer

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

3e19bf8: 8323529: Relativize test image dependencies in microbenchmarks
ba23025: 8322957: Generational ZGC: Relocation selection must join the STS
7c3a39f: 8323297: Fix incorrect placement of precompiled.hpp include lines
e72723d: 8323296: java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java#id1 timed out
66520be: 8280056: gtest/LargePageGtests.java#use-large-pages failed "os.release_one_mapping_multi_commits_vm"
82a63a0: 8258979: The image didn't show correctly with GTK LAF
8d9814a: 8322757: Enable -Wparentheses warnings
0ff2dea: 8320673: PageFormat/CustomPaper.java has no Pass/Fail buttons; multiple instructions
8e12053: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)
e4389d8: 8323571: Regression in source resolution process
... and 52 more: https://git.openjdk.org/jdk/compare/438ab7c115249d7501edfbb2d3c62e96ae824181...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@albertnetymk, @plummercj) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

LizBing · 2024-01-12T05:14:54Z

Sorry about the false alarms. Not noticing that vmStructs_serial.hpp was also updated threw me off. The changes look fine. I ran the SA tests locally and they passed.

It's alright. btw, thanks for the review.

plummercj · 2024-01-12T05:18:42Z

It's alright. btw, thanks for the review.

You're welcome. And just to be clear, I'm okaying just the SA changes. You'll still need another reviewer for the hotspot changes.

albertnetymk

Tier1-6 pass.

I used git diff --color-moved=dimmed_zebra <old> <new> to check the change inside hotspot and it's mostly code-moving, as expected. There are some still format issues (header include order and broken indentation), which can probably be fixed in followup PRs.

LizBing · 2024-01-12T10:41:51Z

Tier1-6 pass.

I used git diff --color-moved=dimmed_zebra <old> <new> to check the change inside hotspot and it's mostly code-moving, as expected. There are some still format issues (header include order and broken indentation), which can probably be fixed in followup PRs.

I will have a try. Thanks for the review.
/integrate

openjdk · 2024-01-12T10:43:14Z

@LizBing
Your change (at version ae2817d) is now ready to be sponsored by a Committer.

albertnetymk · 2024-01-12T10:56:00Z

/sponsor

The followup ticket: https://bugs.openjdk.org/browse/JDK-8323660

openjdk · 2024-01-12T10:56:53Z

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

ed18222: 8323190: Segfault during deoptimization of C2-compiled code
3e19bf8: 8323529: Relativize test image dependencies in microbenchmarks
ba23025: 8322957: Generational ZGC: Relocation selection must join the STS
7c3a39f: 8323297: Fix incorrect placement of precompiled.hpp include lines
e72723d: 8323296: java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java#id1 timed out
66520be: 8280056: gtest/LargePageGtests.java#use-large-pages failed "os.release_one_mapping_multi_commits_vm"
82a63a0: 8258979: The image didn't show correctly with GTK LAF
8d9814a: 8322757: Enable -Wparentheses warnings
0ff2dea: 8320673: PageFormat/CustomPaper.java has no Pass/Fail buttons; multiple instructions
8e12053: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)
... and 53 more: https://git.openjdk.org/jdk/compare/438ab7c115249d7501edfbb2d3c62e96ae824181...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-01-12T10:57:02Z

@albertnetymk @LizBing Pushed as commit 7dc9dd6.

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

merge 'CollectedHeap' and 'SerialHeap'

4ef3a2e

openjdk bot changed the title ~~8234502~~ 8234502: Merge GenCollectedHeap and SerialHeap Dec 1, 2023

openjdk bot added hotspot hotspot-dev@openjdk.org rfr Pull request is ready for review labels Dec 1, 2023

openjdk bot added the hotspot-gc hotspot-gc-dev@openjdk.org label Dec 1, 2023

LizBing added 2 commits December 2, 2023 19:02

line-break for EOF

bdf57c8

restore comment

e297b42

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Dec 7, 2023

Merge branch 'master' of https://git.openjdk.org/jdk into JDK-8234502

fef7f8a

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Dec 8, 2023

resolve conflict

e6d7dfe

albertnetymk reviewed Dec 8, 2023

View reviewed changes

src/hotspot/share/gc/shared/space.cpp Outdated Show resolved Hide resolved

LizBing added 3 commits December 9, 2023 09:07

remove 'GenCollectedHeap' from 'jdk.hotspot.agent'

f6ca517

fix import statement

6d8381c

fix import statement

992b7ab

LizBing added 2 commits January 9, 2024 23:48

Merge branch 'master' of https://git.openjdk.org/jdk into JDK-8234502

ac55436

resolve conflicts

92a96cc

albertnetymk reviewed Jan 9, 2024

View reviewed changes

src/hotspot/share/gc/shared/vmStructs_gc.hpp Outdated Show resolved Hide resolved

remove two '*Generation'

abd3e3c

fix

628de1a

plummercj reviewed Jan 10, 2024

View reviewed changes

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerLocation.java Outdated Show resolved Hide resolved

albertnetymk reviewed Jan 10, 2024

View reviewed changes

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerFinder.java Show resolved Hide resolved

LizBing commented Jan 11, 2024

View reviewed changes

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerFinder.java Outdated Show resolved Hide resolved

restore and rename 'GenCollectedHeap' to 'SerialHeap'

ae2817d

plummercj suggested changes Jan 11, 2024

View reviewed changes

src/hotspot/share/gc/shared/vmStructs_gc.hpp Show resolved Hide resolved

src/hotspot/share/gc/shared/vmStructs_gc.hpp Show resolved Hide resolved

src/hotspot/share/gc/shared/vmStructs_gc.hpp Show resolved Hide resolved

src/hotspot/share/gc/shared/vmStructs_gc.hpp Show resolved Hide resolved

plummercj approved these changes Jan 11, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 11, 2024

albertnetymk approved these changes Jan 12, 2024

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Jan 12, 2024

openjdk bot added the integrated Pull request has been integrated label Jan 12, 2024

openjdk bot closed this Jan 12, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Jan 12, 2024

LizBing deleted the JDK-8234502 branch January 26, 2024 03:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8234502: Merge GenCollectedHeap and SerialHeap #16927

8234502: Merge GenCollectedHeap and SerialHeap #16927

LizBing commented Dec 1, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Dec 1, 2023

openjdk bot commented Dec 1, 2023

LizBing commented Dec 1, 2023

openjdk bot commented Dec 1, 2023

mlbridge bot commented Dec 1, 2023 •

edited

Loading

LizBing commented Dec 1, 2023

albertnetymk commented Dec 1, 2023

LizBing commented Dec 2, 2023

LizBing commented Dec 2, 2023

openjdk bot commented Dec 7, 2023

albertnetymk left a comment

bridgekeeper bot commented Jan 6, 2024

albertnetymk commented Jan 9, 2024

albertnetymk left a comment

albertnetymk commented Jan 10, 2024

LizBing commented Jan 10, 2024

plummercj left a comment

openjdk bot commented Jan 11, 2024 •

edited

Loading

LizBing commented Jan 12, 2024

plummercj commented Jan 12, 2024

albertnetymk left a comment

LizBing commented Jan 12, 2024

openjdk bot commented Jan 12, 2024

albertnetymk commented Jan 12, 2024

openjdk bot commented Jan 12, 2024

openjdk bot commented Jan 12, 2024

8234502: Merge GenCollectedHeap and SerialHeap #16927

8234502: Merge GenCollectedHeap and SerialHeap #16927

Conversation

LizBing commented Dec 1, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Dec 1, 2023

openjdk bot commented Dec 1, 2023

LizBing commented Dec 1, 2023

openjdk bot commented Dec 1, 2023

mlbridge bot commented Dec 1, 2023 • edited Loading

Webrevs

LizBing commented Dec 1, 2023

albertnetymk commented Dec 1, 2023

LizBing commented Dec 2, 2023

LizBing commented Dec 2, 2023

openjdk bot commented Dec 7, 2023

albertnetymk left a comment

Choose a reason for hiding this comment

bridgekeeper bot commented Jan 6, 2024

albertnetymk commented Jan 9, 2024

albertnetymk left a comment

Choose a reason for hiding this comment

albertnetymk commented Jan 10, 2024

LizBing commented Jan 10, 2024

plummercj left a comment

Choose a reason for hiding this comment

openjdk bot commented Jan 11, 2024 • edited Loading

LizBing commented Jan 12, 2024

plummercj commented Jan 12, 2024

albertnetymk left a comment

Choose a reason for hiding this comment

LizBing commented Jan 12, 2024

openjdk bot commented Jan 12, 2024

albertnetymk commented Jan 12, 2024

openjdk bot commented Jan 12, 2024

openjdk bot commented Jan 12, 2024

LizBing commented Dec 1, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Dec 1, 2023 •

edited

Loading

openjdk bot commented Jan 11, 2024 •

edited

Loading