-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Set MaxRAM and MaxRAMPercentage in launcher for native-image driver #8366
Conversation
See oracle#7277 (comment) for motivation. We have been using this as the default in Mandrel since the 23.1 release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would bring the shell/cmd launchers on par with the native image version of the launcher.
Sure that makes sense, although this changes it for all users of the launcher scripts, including Truffle languages, no? |
Yes. We have no experience with those. If they have a similar setup of launching a JVM process driving native-image generation, it should still be a safe option to use those. I.e. keep the launching process (which is usually tiny) within certain bounds. |
Ok, let me take a look at this in the next couple of days. |
Rather than modifying the templates, I would pass these Java arguments to the For example:
|
BTW, should we pass only Looking at #7277 (comment), this seems closer to what the native version does. |
+1, Xmx seems more standard. On a machine with 64GB memory, I get a max heap of 206M with the two flags, and on one with 1GB I get 199.19M (see below). So maybe fine-tuning is not really worth it?
|
No. |
That makes sense, thanks! |
We could of course set |
I think @ansalond was suggesting to replace both flags with just the -Xmx flag. |
Sure. I'm arguing that it makes more sense to set |
Ok, let's go with the two flags then, especially because they have been tested with Mandrel. @zakkak could you update the PR according to #8366 (comment)? Otherwise, I can take care of it and make the adjustment when merging your other PR. |
Let me take care of it, will share a PR when it's ready. |
OK if that works better for you, thanks @fniephaus. |
Superseded by #8384 |
See #7277 (comment) for motivation.
We have been using this as the default in Mandrel since the 23.1 release.