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

Enable container support by default #3184

Merged

Conversation

ashu-mehra
Copy link
Contributor

@ashu-mehra ashu-mehra commented Oct 9, 2018

Enable container support by default. To disable container support
use -XX:-UseContainerSupport option.

Doc issue eclipse-openj9/openj9-docs#106
Fixes: #2263

Signed-off-by: Ashutosh Mehra asmehra1@in.ibm.com

@ashu-mehra
Copy link
Contributor Author

fyi - @mpirvu
@DanHeidinga can you please review.

@@ -1890,7 +1890,7 @@ IDATA VMInitStages(J9JavaVM *vm, IDATA stage, void* reserved) {
argIndex = FIND_AND_CONSUME_ARG(EXACT_MATCH, VMOPT_XXUSECONTAINERSUPPORT, NULL);
argIndex2 = FIND_AND_CONSUME_ARG(EXACT_MATCH, VMOPT_XXNOUSECONTAINERSUPPORT, NULL);

if (argIndex > argIndex2) {
if ((-1 == argIndex2) || (argIndex > argIndex2)) {
Copy link
Member

Choose a reason for hiding this comment

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

If neither option is specified, the argIndex's will be the same. Isn't it sufficient to do if (argIndex >= argIndex2) plus add a comment to indicate that -XX:+ContainerSupport is the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that would work as well.

@DanHeidinga DanHeidinga self-assigned this Oct 9, 2018
@DanHeidinga
Copy link
Member

Jenkins test sanity plinux,aix,win jdk8

@DanHeidinga
Copy link
Member

Thrown some heavier testing at this to validate platforms where -XX:ContainerSupport won't have any effect

Enable container support by default. To disable container support
use -XX:-UseContainerSupport option.

Signed-off-by: Ashutosh Mehra <asmehra1@in.ibm.com>
@ashu-mehra ashu-mehra force-pushed the enable_container_support_default branch from 7aae135 to 0a5212d Compare October 9, 2018 16:22
@ashu-mehra
Copy link
Contributor Author

Updated the condition as suggested.

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,aix,win jdk8

@pshipton
Copy link
Member

pshipton commented Oct 9, 2018

@SueChaplain fyi a new doc change.

@pshipton
Copy link
Member

pshipton commented Oct 9, 2018

@ashu-mehra please create an issue at https://github.com/eclipse/openj9-docs for the change in behavior.

@ashu-mehra
Copy link
Contributor Author

@pshipton created eclipse-openj9/openj9-docs#106 for the doc changes.

@DanHeidinga DanHeidinga merged commit 27aedbb into eclipse-openj9:master Oct 10, 2018
@DanHeidinga DanHeidinga added this to the Release 0.11.0 milestone Oct 10, 2018
@ashu-mehra ashu-mehra deleted the enable_container_support_default branch December 9, 2019 00:18
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

3 participants