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

Changes to utilize default +UseContainerSupport in new releases of Java #47

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

Conversation

imranzunzani
Copy link

Updates in run-java.sh to use latest VM provisions and switches for running Java in containers.
-XX:InitialRAMPercentage
-XX:MaxRAMPercentage
-XX:MinRAMPercentage

@imranzunzani
Copy link
Author

imranzunzani commented Jul 2, 2019

@rhuss,
Could you please review?
These are changes to run-java.sh as per new features in the JDK.
We discussed about these over #46

@rhuss
Copy link
Contributor

rhuss commented Jul 2, 2019

Oops. Sorry for the delay, the notification slipped through somehow ;-(

You must not change run-java.sh directly, but send a PR to https://github.com/fabric8io-images/run-java-sh The files here a auto-generated with a framework called fish-pepper.

So the steps would be:

Sorry for the delayed response ...

@imranzunzani
Copy link
Author

Hi @rhuss,
I created the PR but the vm_test(s) are failing owing to assertions for +UseParallelGC which is not required with the new switches and the tests are also not recognising the new options.
Could you please take a look and let me know what should be changed regarding these old assertions?

@rhuss
Copy link
Contributor

rhuss commented Jul 3, 2019

Hi @rhuss,
I created the PR but the vm_test(s) are failing owing to assertions for +UseParallelGC which is not required with the new switches and the tests are also not recognising the new options.
Could you please take a look and let me know what should be changed regarding these old assertions?

Yes, I can do but I can't promise a timeframe as I have currently quite a bit on my table (sorry). Please ping me again, when I should come back to you within the next weeks.

However, maybe @astefanutti could jump in ?

@imranzunzani
Copy link
Author

@rhuss,
It took me some time and digging to figure out the test logic and relations among the artefacts for CI (Tests).
I have made changes, so that the tests are now in sync with the new features and switches of the JDK.
Could you please help by reviewing the PR or by asking someone to review it, and then merge if things look good?
By the way, the new logic will work only with version > 8u212 which means that JDK 7 won't work, but since the Tests were for JDK 8 and JDK 11, I believe it is alright.

@imranzunzani
Copy link
Author

@rhuss,
Just reminding you.
Please take a look.

@Paramchoudhary
Copy link

Good job

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.

3 participants