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

[23.1] Set MaxRAM and MaxRAMPercentage in launcher for native-image driver #365

Merged
merged 1 commit into from
Sep 15, 2023
Merged

[23.1] Set MaxRAM and MaxRAMPercentage in launcher for native-image driver #365

merged 1 commit into from
Sep 15, 2023

Conversation

zakkak
Copy link
Collaborator

@zakkak zakkak commented Sep 15, 2023

Closes graalvm/mandrel#557

Candidate for master branch as well, but opening against 23.1 first due to graalvm/mandrel#558

@zakkak zakkak requested a review from jerboaa September 15, 2023 08:46
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 15, 2023
jerboaa
jerboaa approved these changes Sep 15, 2023
View reviewed changes
Copy link
Collaborator

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

LGTM, provided CI is OK.

@jerboaa
Copy link
Collaborator

jerboaa commented Sep 15, 2023

We should bring this upstream once we have some experience with this.

@zakkak
Copy link
Collaborator Author

zakkak commented Sep 15, 2023

provided CI is OK.

CI running the full Quarkus test suite: https://github.com/graalvm/mandrel/actions/runs/6196136877

@zakkak
Copy link
Collaborator Author

zakkak commented Sep 15, 2023

provided CI is OK.

CI look good.

@zakkak zakkak merged commit 6c406e3 into graalvm:23.1 Sep 15, 2023
5 checks passed
@zakkak zakkak deleted the 2023-09-15-set-maxRam-for-driver branch September 15, 2023 13:57
@zakkak zakkak mentioned this pull request Sep 15, 2023
Closed
@jerboaa
Copy link
Collaborator

jerboaa commented Oct 6, 2023

@zakkak How do you feel about bringing this upstream? Fabio was interested in having it for the bash launcher too once we have some experience. Thoughts?

@zakkak
Copy link
Collaborator Author

zakkak commented Oct 6, 2023

My only (minor) concern is that this has not been tested in the wild yet. We can wait for Mandrel 23.1 to be released and used by Quarkus for some time first. Although this is not expected to affect GraalVM CE builds, so we could go ahead without waiting as well.

@jerboaa
Copy link
Collaborator

jerboaa commented Oct 6, 2023

OK. Let's wait for the 23.1 release to happen, then move Quarkus to it and re-evaluate in a few months time.

@jerboaa
Copy link
Collaborator

jerboaa commented Feb 13, 2024

We should bring this upstream once we have some experience with this.

@zakkak We have been running with this for a while now with no issues. Should we bring this upstream?

@zakkak
Copy link
Collaborator Author

zakkak commented Feb 13, 2024

Yes, I haven't seen any issues with it so far.

@zakkak
Copy link
Collaborator Author

zakkak commented Feb 13, 2024

Created oracle/graal#8366

@jerboaa
Copy link
Collaborator

jerboaa commented Feb 13, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jerboaa jerboaa jerboaa approved these changes

Assignees
No one assigned
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zakkak @jerboaa