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

Fix daily native build by limiting Java heap used and spread load in matrix #1523

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

michalvavrik
Copy link
Member

@michalvavrik michalvavrik commented Nov 16, 2023

Summary

2023-11-16T03:28:34.0733613Z 03:28:34,030 INFO   - 4.44GB of memory (28.5% of 15.61GB system memory, set via '-Xmx5g')
2023-11-16T03:28:34.0734917Z 03:28:34,030 INFO   - 4 thread(s) (100.0% of 4 available processor(s), determined at start)
2023-11-16T03:35:32.9806773Z 03:35:32,969 INFO  Terminating due to java.lang.OutOfMemoryError: GC overhead limit exceeded

IIUC https://www.graalvm.org/latest/reference-manual/native-image/optimizations-and-performance/MemoryManagement/ it says that GC also needs 2x of the maximum heap size is the worst case and https://quarkus.io/guides/native-reference says minimum allowed is 4 GB.

So this PR might slower down native CI execution, but GH runners seems weak, I suggest to try it and optimize based on results.

Please select the relevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • Dependency update
  • Refactoring
  • Backport
  • New scenario (non-breaking change which adds functionality)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

@michalvavrik michalvavrik requested a review from jsmrcka November 16, 2023 11:22
@michalvavrik
Copy link
Member Author

PS: this CI isn't really testing this PR :-) but let's wait at least for validation (thought that's not testing this PR either)...

Copy link
Contributor

@jsmrcka jsmrcka left a comment

Choose a reason for hiding this comment

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

Well, it looks like it's worth a shot. Thanks for the analysis.

@jsmrcka jsmrcka merged commit 1dc6761 into quarkus-qe:main Nov 16, 2023
8 checks passed
@michalvavrik michalvavrik deleted the feature/fix-daily-native-om branch November 16, 2023 13:39
@michalvavrik
Copy link
Member Author

Well, it looks like it's worth a shot. Thanks for the analysis.

Yeah, it's not worth testing it because native run execution time is really volatile, IMO we need to take a bit of guessing and wait.

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