Skip to content

Conversation

babsingh
Copy link
Contributor

@babsingh babsingh commented Aug 28, 2025

Introduce distinct segment categories (SUB4G, FREQUENTLY_ACCESSED,
INFREQUENTLY_ACCESSED).

Separates hot vs. cold J9Class data for better memory efficiency.

SUB4G segments are sized in multiples of aligned J9Class headers
and tagged with MEMORY_TYPE_RAM_CLASS_SUB4G.

Related: #20644

Depends on eclipse-omr/omr#7916

@babsingh
Copy link
Contributor Author

babsingh commented Sep 2, 2025

fyi @tajila

@tajila tajila self-requested a review September 3, 2025 14:04
allocationRequests[RAM_STATICS_FRAGMENT].alignment = sizeof(U_64);
allocationRequests[RAM_STATICS_FRAGMENT].alignedSize = totalStaticSlots * sizeof(UDATA);
allocationRequests[RAM_STATICS_FRAGMENT].address = NULL;
allocationRequests[RAM_STATICS_FRAGMENT].segmentKind = INFREQUENTLY_ACCESSED;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is frequently accessed

Copy link
Contributor

Choose a reason for hiding this comment

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

the rest is not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Now, only RAM_STATICS_FRAGMENT is tagged as FREQUENTLY_ACCESSED, and the rest are tagged as INFREQUENTLY_ACCESSED.

Comment on lines +4361 to +4362
const UDATA headersPerSegment = 32;
classAllocationIncrement = sizeof(J9Class) * headersPerSegment;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should alignment be considered here?

Suggested change
const UDATA headersPerSegment = 32;
classAllocationIncrement = sizeof(J9Class) * headersPerSegment;
UDATA mask = J9_REQUIRED_CLASS_ALIGNMENT - 1;
UDATA unitSegment = (sizeof(J9Class) + mask) & ~mask;
const UDATA headersPerSegment = 32;
classAllocationIncrement = unitSegment * headersPerSegment;

Introduce distinct segment categories (SUB4G, FREQUENTLY_ACCESSED,
INFREQUENTLY_ACCESSED).

Separates hot vs. cold J9Class data for better memory efficiency.

SUB4G segments are sized in multiples of aligned J9Class headers
and tagged with MEMORY_TYPE_RAM_CLASS_SUB4G.

Related: eclipse-openj9#20644

Co-authored-by: Nick Kamal <[email protected]>
Signed-off-by: Babneet Singh <[email protected]>
@tajila
Copy link
Contributor

tajila commented Sep 9, 2025

jenkins test sanity alinux64 jdk17

@tajila
Copy link
Contributor

tajila commented Sep 9, 2025

jenkins test sanity.functional win jdk11

@tajila
Copy link
Contributor

tajila commented Sep 9, 2025

jenkins test sanity plinux jdk21

@tajila
Copy link
Contributor

tajila commented Sep 9, 2025

jenkins test sanity.functional win jdk11

@tajila tajila merged commit 2e29283 into eclipse-openj9:master Sep 10, 2025
11 checks passed
@mpirvu
Copy link
Contributor

mpirvu commented Sep 17, 2025

@babsingh While I was tracking an older Liberty multiapp start-up regression I found out that this PR introduces a 21% start-up regression for that app. I confirmed the regression by reverting this commit. I will evaluate other apps as well (acmeairee8 does not seem to be affected).

@babsingh
Copy link
Contributor Author

@tajila suggested that the fix may be fairly simple, such as increasing the size of sub-4G segments. I have begun drafting profiling changes per RAM class subtype in the javacore, which should help determine how to adjust segment sizes.

@mpirvu
Copy link
Contributor

mpirvu commented Sep 22, 2025

I was chasing a 5% AcmeAir throughput regression when using JITServer and tracked it down to this PR. I don't yet understand how this PR can affect throughput with JITServer.

@vij-singh
Copy link

Also PMA-raised issue https://github.ibm.com/runtimes/javanext/issues/548 mentions this issue as the regression source...

@mpirvu
Copy link
Contributor

mpirvu commented Sep 25, 2025

Given the large number of regressions I think we should revert this PR for now.

@babsingh
Copy link
Contributor Author

I am investigating the regressions. Other PRs (JFR and javacore updates) depend on this one, so a full revert is no longer straightforward. We will first spend a few weeks trying to resolve the regressions. If no workaround is found by the deadline given to me, we have a way to revert to the original behavior without rolling back the entire set.

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.

4 participants