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 SCC disclaiming and tweak GC DNSS for InstantOn #20568

Merged
merged 3 commits into from
Nov 14, 2024

Conversation

ymanton
Copy link
Member

@ymanton ymanton commented Nov 12, 2024

This PR includes patches that will

  1. Enable SCC disclaiming when CRIU or CRaC support is enabled.
  2. Tweak the GC parameters dnssExpectedRatioMaximum and dnssExpectedRatioMinimum to be 2x larger when CRIU or CRaC support is enabled. Increasing this threshold favours reducing nursery expansion and average memory consumption at the cost of increased GC overhead.
  3. Increases SCC disclaim frequency to every 500 ms.

SCC disclaiming increases scavenge times, which causes nursery expansion, and offsets most of the memory savings we get from disclaiming the SCC, hence the adjustment to the thresholds. Another way to tackle this problem is to reduce the impact of SCC disclaim on scavenge by re-organizing the SCC layout or caching the SCC data touched during scavenge separately, but these options require further development to realize.

See #20498 for additional background info on SCC disclaiming.

FYI @vijaysun-omr @mpirvu @amicic

@ymanton ymanton requested a review from dsouzai as a code owner November 12, 2024 00:02
@mpirvu mpirvu added comp:jit comp:gc criu Used to track CRIU snapshot related work labels Nov 12, 2024
}
}
#endif /* defined(J9VM_OPT_CRIU_SUPPORT) */

Copy link
Contributor

@amicic amicic Nov 12, 2024

Choose a reason for hiding this comment

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

this is somewhat an ok place to deal with this, but since these fields are already initially set in OMR's MM_ConfigurationGenerational::initialize(), the matching place in J9 to override the values would be MM_ConfigurationDelegate::initialize()

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I agree it's a better place. Fixed.

@mpirvu mpirvu self-assigned this Nov 12, 2024
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

The changes look good to me.

This patch reduces minTimeBetweenMemoryDisclaims from 5000 to 500 and
increases the time between disclaims for the data cache, code cache, and
iprofiler data by a factor of 10 to maintain the current behavior.

For the SCC it removes the scaling of minTimeBetweenMemoryDisclaims such
that we will disclaim the SCC every 500 ms.

Signed-off-by: Younes Manton <[email protected]>
This patch enables SCC disclaiming when CRIU or CRaC support is enabled.

Signed-off-by: Younes Manton <[email protected]>
This patch scales dnssExpectedRatioMaximum and dnssExpectedRatioMinimum
by 2x when neither is user-specified and either CRIU or CRaC support is
enabled. This favours reduced memory consumption at the expense of GC
overhead.

Signed-off-by: Younes Manton <[email protected]>
@mpirvu
Copy link
Contributor

mpirvu commented Nov 13, 2024

jenkins test sanity xlinux,plinux,zlinux,aix jdk21

@mpirvu
Copy link
Contributor

mpirvu commented Nov 13, 2024

I started jenkins sanity tests assuming @amicic will agree with the new version of the code.

@mpirvu mpirvu merged commit 27fba55 into eclipse-openj9:master Nov 14, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:gc comp:jit criu Used to track CRIU snapshot related work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants