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.

Sign up for GitHub

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

Jump to bottom

Remove gc_option parameter and rename to set_xmx_if_not_set #58

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

Remove gc_option parameter and rename to set_xmx_if_not_set #58

wants to merge 1 commit into from

Conversation

hanslovsky
Copy link
Member

I thought it would be cool and convenient to specify default value for garbage collection but the opposite is true: It is impossible to use a different GC when calling main_from_endpoint. Instead, do not specify a default GC and only set default -Xmx if requested by the user (which it is the case by default)

Otherwise it is impossible to change the garbage collector to, e.g., g1gc.

This may not be the best solution to this but we should probably solve this issue this way or another.

@hanslovsky
Remove gc_option parameter and rename to set_xmx_if_not_set
4b8ab08 
Otherwise it is impossible to change the garbage collector to, e.g., g1gc
@hanslovsky hanslovsky mentioned this pull request Mar 23, 2020
Closed
@marktsuchida
Copy link

This is essential for the Python API to work with Java 17 because some time between JDK 11 and 17 -XX: UseConcMarkSweepGC became invalid (already deprecated in JDK 9).

@hanslovsky
Copy link
Member Author

I am not actively working on jgo anymore. @ctrueden would know if this PR is the right approach for setting garbage collector.

@ctrueden
Copy link
Member

ctrueden commented Jul 5, 2022

I think this is a good solution, but would not rename the existing function to set_xmx_if_not_set. Even though the default args added are memory only after this change, that might not always be the case in the future. The addition of the flag to control whether these default args are inferred and added is a good change, though. Anyway @marktsuchida I can clean up and merge this after I'm back from vacation next week.

@marktsuchida
Copy link

Sounds good to me. I'm not blocked by this issue at the moment, just wanted to note my observation.

@ctrueden ctrueden self-assigned this Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ctrueden ctrueden

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hanslovsky @marktsuchida @ctrueden