Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SapMachine #1552: Backport to 17: 8269914: Factor out heap printing for G1 young and full gc #1555

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

GoeLin
Copy link
Member

@GoeLin GoeLin commented Nov 28, 2023

Backport of https://bugs.openjdk.org/browse/JDK-8269914: Factor out heap printing for G1 young and full gc

For details see also openjdk/jdk17u-dev#1900, this is an identical copy
of the change proposed there regarding 8269914.

fixes #1552

@GoeLin GoeLin changed the base branch from sapmachine to sapmachine17 November 28, 2023 16:19
@SapMachine
Copy link
Member

Hello @GoeLin, this pull request fulfills all formal requirements.

@GoeLin
Copy link
Member Author

GoeLin commented Nov 28, 2023

Failing tests: infra issues: "Failed permission check: goelin is missing the Overall/Read permisson"

@RealCLanger
Copy link
Member

Failing tests: infra issues: "Failed permission check: goelin is missing the Overall/Read permisson"

No, that's not the issue. Please try once more whether you can access the Jenkins build logs. If it doesn't work, we should check together what's wrong. However, there's probably an issue in the Pipeline script that does the merging. I'll check this.

@RealCLanger
Copy link
Member

retest this please

@SapMachine
Copy link
Member

Hello @GoeLin, this pull request fulfills all formal requirements.

@GoeLin
Copy link
Member Author

GoeLin commented Nov 30, 2023

Pre-submit tests: rerun did not help, still looks like an infra issue to me.

@RealCLanger
Copy link
Member

Pre-submit tests: rerun did not help, still looks like an infra issue to me.

no, it's all good. The failing tests are "validate-pr-22" which were not supposed to run for a sapmachine17 backport. Probably your initial version of the PR went against branch "sapmachine" and you changed it afterwards?

The validate-pr-17 ran fine, the one test issue for linuxppc64le is an unrelated known problem that will be fixed with an OpenJDK backport.

@reinrich
Copy link
Member

reinrich commented Dec 1, 2023

Hi Götz,
https://bugs.openjdk.org/browse/JDK-8269914 has issues that are fixed by https://bugs.openjdk.org/browse/JDK-8272651. Will you backport the latter as well?

@GoeLin
Copy link
Member Author

GoeLin commented Dec 1, 2023

Yes, I intend to do exactly what I did in the OpenJDK PR mentioned above.
That includes 8272651, too.

@reinrich
Copy link
Member

reinrich commented Dec 1, 2023

Yes, I intend to do exactly what I did in the OpenJDK PR mentioned above. That includes 8272651, too.

I'm confused because this pr is not an exact copy of the OpenJDK PR.
Are you planning to backport 8272651 in a separate pr?

@GoeLin
Copy link
Member Author

GoeLin commented Dec 1, 2023

Yes, I intend to do 3 PRs on SapMachine, this one, then 8272651 and last 8291753.
It is good practice do keep the granularity of OpenJDK changes.

Copy link
Member

@reinrich reinrich left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Cheers, Richard.

@RealCLanger RealCLanger merged commit 4b0c2c7 into sapmachine17 Dec 1, 2023
69 of 79 checks passed
@RealCLanger RealCLanger deleted the goetz-backport_8269914 branch December 1, 2023 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backport to 17.0.10: 8269914: Factor out heap printing for G1 young and full gc
4 participants