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

Force fixed heap size when using NoGC #1264

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Conversation

wks
Copy link
Collaborator

@wks wks commented Jan 16, 2025

The dynamic heap size trigger needs GC to adjust the heap size, but NoGC can't do GC. So it doesn't make sense to use dynamic heap size with NoGC. Currently, if the user selects the NoGC plan and the dynamic heap size trigger, it will trigger GC at the minimum heap size and then panic immediately.

With this change, MMTk will give a warning and use fixed heap size trigger instead, using the maximum heap size specified in the dynamic trigger as the heap size.

@wks wks requested a review from qinsoon January 16, 2025 06:55
@wks
Copy link
Collaborator Author

wks commented Jan 16, 2025

Note: This is intended to fix the problem #1263 addresses. With this PR, if CRuby user only selects the NoGC plan and doesn't specify a trigger, the default behavior will be using the fixed-size trigger with 80% physical memory (the default max heap size). This should be big enough for experiment purpose. I don't think it's practical to use Ruby with NoGC in production, but the user can still specify a fixed heap size if needed.

Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

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

LGTM

@wks wks added this pull request to the merge queue Jan 16, 2025
Merged via the queue into mmtk:master with commit c61e6c8 Jan 16, 2025
33 checks passed
@wks wks deleted the fix/nogc-force-fixed branch January 16, 2025 13:06
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