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

Add GC Mappings #7723

Merged
merged 1 commit into from Dec 6, 2019
Merged

Add GC Mappings #7723

merged 1 commit into from Dec 6, 2019

Conversation

AlenBadel
Copy link
Contributor

@AlenBadel AlenBadel commented Nov 7, 2019

This commit maps the following Hotspot options:
-XX:ParallelCMSThreads=N to -XconcurrentbackgroundN
-XX:ConcGCThreads=N to -XconcurrentbackgroundN
-XX:ParallelGCThreads=N to -XgcthreadsN

Doc PR eclipse-openj9/openj9-docs#464

Signed-off-by: AlenBadel Alen.Badel@ibm.com

@AlenBadel
Copy link
Contributor Author

See Issue #4930

@AlenBadel
Copy link
Contributor Author

FYI @dmitripivkine

@dmitripivkine
Copy link
Contributor

please squash commits to one

This commit maps the following Hotspot options:
-XX:ParallelCMSThreads=N to -XconcurrentbackgroundN
-XX:ConcGCThreads=N to -XconcurrentbackgroundN
-XX:ParallelGCThreads=N to -XgcthreadsN

Signed-off-by: AlenBadel <Alen.Badel@ibm.com>
@AlenBadel
Copy link
Contributor Author

AlenBadel commented Nov 13, 2019

@DanHeidinga Could you kindly take a quick look? I know you had concerns earlier if GC options should be mapped with registerCmdLineMapping.

@AlenBadel
Copy link
Contributor Author

@DanHeidinga Could you kindly take a look? This is slated for R0.18

@gita-omr
Copy link
Contributor

gita-omr commented Dec 4, 2019

@DanHeidinga Could you kindly take a quick look? I know you had concerns earlier if GC options should be mapped with registerCmdLineMapping.

@DanHeidinga could you review please?

@gita-omr
Copy link
Contributor

gita-omr commented Dec 6, 2019

@gacholio I think @DanHeidinga is very busy, maybe you can take a look? This PR is holding the rest of the option migration work which we really want to get into 0.18.

@AlenBadel
Copy link
Contributor Author

@pshipton Feel free to chime in. I know you're quite versed with past options migrations and mapping.

Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

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

lgtm

@DanHeidinga
Copy link
Member

Jenkins test sanity plinux,zlinux,win jdk8

@pshipton
Copy link
Member

pshipton commented Dec 6, 2019

Is there a doc PR created for this?

@pshipton
Copy link
Member

pshipton commented Dec 6, 2019

Not sure what's up with the breakage in the PR testing, checking with Adam.

@pshipton
Copy link
Member

pshipton commented Dec 6, 2019

Should be fixed now by #8010

Jenkins test sanity plinux,zlinux,win jdk8

@pshipton
Copy link
Member

pshipton commented Dec 6, 2019

I forgot, I think the commit needs to be rebased in order to fix the PR testing.

@pshipton
Copy link
Member

pshipton commented Dec 6, 2019

Well apparently not, because the PR tests are running.

@AlenBadel
Copy link
Contributor Author

AlenBadel commented Dec 6, 2019

Is there a doc PR created for this?

No, but I just created an issue for it.

See
eclipse-openj9/openj9-docs#453

@pshipton
Copy link
Member

pshipton commented Dec 6, 2019

@AlenBadel note you also need to create the doc PR. The docs team will review but doesn't create the doc changes any more. See https://github.com/eclipse/openj9-docs/tree/master/process

@gita-omr
Copy link
Contributor

gita-omr commented Dec 6, 2019

@pshipton can we merge this PR so that @AlenBadel can proceed with the next one that is dependent on it?

@pshipton
Copy link
Member

pshipton commented Dec 6, 2019

can we merge this PR so that AlenBadel can proceed with the next one that is dependent on it?

The reviewers approved and the PR testing has passed, so I'll go ahead and merge.

@pshipton pshipton merged commit 260d138 into eclipse-openj9:master Dec 6, 2019
@AlenBadel AlenBadel deleted the mapping branch December 7, 2019 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants