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

Use LDR_CNTRL on AIX to use 64KB pages and modify removeAixExpectedVars #749

Open
wants to merge 1 commit into
base: openj9
Choose a base branch
from

Conversation

rmnattas
Copy link
Contributor

@rmnattas rmnattas commented Mar 1, 2024

We can improve performance on AIX by setting LDR_CNTRL to use 64KB pages by default.

OpenJ9 Tracker: eclipse-openj9/openj9#19052

Signed-off-by: Abdulrahman Alattas <rmnattas@gmail.com>
Copy link
Member

@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

Checking for the options that control enablement of this need to be added (see eclipse-openj9/openj9#19052 (comment)).

Comment on lines +315 to +319
const char * ldrCntrlName = "LDR_CNTRL";
const char *ldrCntrlValue = "TEXTPSIZE=64K@DATAPSIZE=64K@STACKPSIZE=64K@SHMPSIZE=64K";
if (setenv(ldrCntrlName, ldrCntrlValue, 0) != 0) {
fprintf(stderr, "setenv('LDR_CNTRL=TEXTPSIZE=64K@DATAPSIZE=64K@STACKPSIZE=64K@SHMPSIZE=64K') failed: performance may be affected\n");
}
Copy link
Member

Choose a reason for hiding this comment

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

For consistency, please remove the space after * on line 315.
Also suggest adding this before setting LDR_CNTRL to keep things organized.

Comment on lines -814 to +816
// OpenJ9 adds MALLOCOPTIONS
// OpenJ9 adds MALLOCOPTIONS and LDR_CNTRL
cleanedVars = cleanedVars.replace("MALLOCOPTIONS=multiheap,considersize,", "");
cleanedVars = cleanedVars.replace("LDR_CNTRL=TEXTPSIZE=64K@DATAPSIZE=64K@STACKPSIZE=64K@SHMPSIZE=64K,", "");
Copy link
Member

Choose a reason for hiding this comment

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

The method comment should be updated to list all three expected variables.
Likewise, the comment here should name and the code should remove the variables in alphabetic order (the sentence in the comment should end with a period).

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.

2 participants