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

Enable container support by default #2263

Closed
ashu-mehra opened this issue Jun 26, 2018 · 10 comments
Closed

Enable container support by default #2263

ashu-mehra opened this issue Jun 26, 2018 · 10 comments

Comments

@ashu-mehra
Copy link
Contributor

Currently OpenJ9 enables container/cgroup support only when -XX:+UseContainerSupport option is specified. Java 10 OpenJDK has this option enabled by default.
I think we can also make this option enabled by default now. Any concerns? @DanHeidinga @pshipton

fyi - @mpirvu

@DanHeidinga
Copy link
Member

@ashu-mehra Can you clarify what -XX:+UseContainerSupport current enables / changes and what additional features are expected to be controlled by this option in the near future?

@ashu-mehra
Copy link
Contributor Author

At present -XX:+UseContainerSupport enables following:

  1. Updates default max heap size to 75% of the container memory if it is > 2G
  2. Value returned by Runtime.availableProcessors() and JVMTI API GetAvailableProcessors() considers CPU quota, if set
  3. Number of GC threads and JIT active threads are also determined by considering CPU quota, if set.

In near future we also want JIT to consider free memory in the container for determining JIT scratch space (#1371) under this option.
I think that's all related to -XX:+UseContainerSupport.

During discussion with @mpirvu on enabling -XX:+UseContainerSupport by default, we felt it would be better if we can execute some existing tests in our regular builds in container environment before enabling this option be default.
I will open a new issue for discussion on testing required for -XX:+UseContainerSupport.

@DanHeidinga
Copy link
Member

it would be better if we can execute some existing tests in our regular builds in container environment before enabling this option be default.

+1 to adding container tests to the builds though this will add a dependency to our build / test machines to have Docker installed.

fyi - @smlambert @llxia @AdamBrousseau @jdekonin

@DanHeidinga DanHeidinga added this to To do in Container-aware JVM via automation Jul 12, 2018
@DanHeidinga
Copy link
Member

Container support has been enabled by default in Adopt's docker images: AdoptOpenJDK/openjdk-docker#39

@mpirvu
Copy link
Contributor

mpirvu commented Oct 3, 2018

@dinogun @ashu-mehra @DanHeidinga @pshipton Since docker images for OpenJ9 already use -XX+UseContainerSupport and outside a container this option becomes a no-op, should we enable this option by default?
This would provide container support to people that create their own Docker images and don't build on top of the images at AdoptOpenJDK.

@DanHeidinga
Copy link
Member

I think this makes sense. Can someone gather some startup measurements outside of a container with and without -XX+UseContainerSupport to validate that there's no cost to making this the default?

@mpirvu
Copy link
Contributor

mpirvu commented Oct 4, 2018

I evaluated Liberty start-up with no application on top on the assumption that the code for XX:+UseContainerSupport support has a fixed cost during bootstrap and therefore very short applications are going to be affected more. Experiments were not done in a container.
From the results the presence of -XX:+UseContainerSupport on the command line produces a 0.3% regression which is the same as the confidence interval, so statistically the two runs are identical.
I will do another big batch of experiments to verify.

Results for JDK=/home/mpirvu/sdks/jvmxa6480sr6-20181001_01 jvmOpts=-Xquickstart -Xmx256m -Xshareclasses:enableBCI,name=liberty -Xscmx60M -Xscmaxaot8m
StartupTime     avg=3234        min=3071        max=3590        stdDev=89.8     maxVar=16.9%    confInt=0.30%   samples=233
Footprint       avg=140057      min=133180      max=151132      stdDev=4216.5   maxVar=13.5%    confInt=0.32%   samples=240
CThreadTime     avg= 741        min= 653        max= 878        stdDev=52.0     maxVar=34.5%    confInt=0.75%   samples=240
ProcessTime     avg=3854        min=3680        max=4110        stdDev=90.2     maxVar=11.7%    confInt=0.25%   samples=240
Results for JDK=/home/mpirvu/sdks/jvmxa6480sr6-20181001_01 jvmOpts=-XX:+UseContainerSupport -Xquickstart -Xmx256m -Xshareclasses:enableBCI,name=liberty -Xscmx60M -Xscmaxaot8m
StartupTime     avg=3246        min=3077        max=3638        stdDev=106.8    maxVar=18.2%    confInt=0.35%   samples=235
Footprint       avg=140544      min=133700      max=154604      stdDev=4448.9   maxVar=15.6%    confInt=0.34%   samples=240
CThreadTime     avg= 741        min= 641        max= 877        stdDev=54.4     maxVar=36.8%    confInt=0.78%   samples=240
ProcessTime     avg=3860        min=3690        max=4180        stdDev=97.4     maxVar=13.3%    confInt=0.27%   samples=240

@mpirvu
Copy link
Contributor

mpirvu commented Oct 4, 2018

A new batch of experiments show a 0.5% regression with a confidence interval of 0.3%

Results for JDK=/home/mpirvu/sdks/jvmxa6480sr6-20181001_01 jvmOpts=-Xquickstart -Xmx256m -Xshareclasses:enableBCI,name=liberty -Xscmx60M -Xscmaxaot8m
StartupTime     avg=3216        min=3069        max=3570        stdDev=80.1     maxVar=16.3%    confInt=0.27%   samples=232
Footprint       avg=140199      min=133132      max=150144      stdDev=4364.3   maxVar=12.8%    confInt=0.33%   samples=240
CThreadTime     avg= 747        min= 656        max= 904        stdDev=53.7     maxVar=37.8%    confInt=0.77%   samples=240
ProcessTime     avg=3864        min=3640        max=4160        stdDev=96.6     maxVar=14.3%    confInt=0.27%   samples=240
Results for JDK=/home/mpirvu/sdks/jvmxa6480sr6-20181001_01 jvmOpts=-XX:+UseContainerSupport -Xquickstart -Xmx256m -Xshareclasses:enableBCI,name=liberty -Xscmx60M -Xscmaxaot8m
StartupTime     avg=3234        min=3071        max=3590        stdDev=88.8     maxVar=16.9%    confInt=0.30%   samples=235
Footprint       avg=140863      min=132512      max=151452      stdDev=4449.4   maxVar=14.3%    confInt=0.34%   samples=240
CThreadTime     avg= 737        min= 627        max= 880        stdDev=54.5     maxVar=40.4%    confInt=0.79%   samples=240
ProcessTime     avg=3862        min=3650        max=4160        stdDev=102.4    maxVar=14.0%    confInt=0.28%   samples=240

And yet another batch comparing -XX:-UseContainerSupport and -XX:+UseContainerSupport shows a 0.4% regression.

Results for JDK=/home/mpirvu/sdks/jvmxa6480sr6-20181001_01 jvmOpts=-XX:-UseContainerSupport -Xquickstart -Xmx256m -Xshareclasses:enableBCI,name=liberty -Xscmx60M -Xscmaxaot8m
StartupTime     avg=3212        min=3062        max=3511        stdDev=74.7     maxVar=14.7%    confInt=0.25%   samples=230
Footprint       avg=140470      min=133136      max=152740      stdDev=4321.0   maxVar=14.7%    confInt=0.33%   samples=240
CThreadTime     avg= 741        min= 639        max= 891        stdDev=56.5     maxVar=39.4%    confInt=0.81%   samples=240
ProcessTime     avg=3852        min=3680        max=4100        stdDev=96.1     maxVar=11.4%    confInt=0.27%   samples=240
Results for JDK=/home/mpirvu/sdks/jvmxa6480sr6-20181001_01 jvmOpts=-XX:+UseContainerSupport -Xquickstart -Xmx256m -Xshareclasses:enableBCI,name=liberty -Xscmx60M -Xscmaxaot8m
StartupTime     avg=3226        min=3066        max=3592        stdDev=92.5     maxVar=17.2%    confInt=0.31%   samples=228
Footprint       avg=141260      min=133584      max=152964      stdDev=4320.8   maxVar=14.5%    confInt=0.33%   samples=240
CThreadTime     avg= 737        min= 653        max= 878        stdDev=54.6     maxVar=34.5%    confInt=0.79%   samples=240
ProcessTime     avg=3854        min=3670        max=4120        stdDev=98.1     maxVar=12.3%    confInt=0.27%   samples=240

With some many experiments I am inclined to say that there could be a tiny startup regression worth 10-15ms
attn: @vijaysun-omr

@vijaysun-omr
Copy link
Contributor

I can accept a 0.4% to startup in this most lightweight of scenarios to have container support on all the time.

@DanHeidinga
Copy link
Member

Given:

  • it's already enabled in Adopt's docker images,
  • the start up effect is minimal outside a container, and
  • it's a no-op outside a container,

I'm onboard with enabling this by default.

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

No branches or pull requests

5 participants