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

Shared cache hints for GC heap size #3743

Closed
pshipton opened this issue Nov 20, 2018 · 64 comments
Closed

Shared cache hints for GC heap size #3743

pshipton opened this issue Nov 20, 2018 · 64 comments

Comments

@pshipton
Copy link
Member

pshipton commented Nov 20, 2018

Remembering the previous heap size settings (-Xmn, -Xmo) after startup can provide a significant startup benefit in subsequent runs, and can improve footprint as well. The GC data can be stored in the shared cache as a hint.
https://unbscholar.lib.unb.ca/islandora/object/unbscholar%3A8100/datastream/PDF/view
https://ieeexplore.ieee.org/document/8121911

Since a shared cache can be used to run more than one application, which may have different heap requirements, the hint should be associated with an application, or at least a main class.

As by design the GC is initialized before the shared cache, new GC APIs will be needed to adjust the GC heap parameters after initialization. Since the heap parameters can be adjusted before any objects are created, it can happen with very low cost.

Doc issue eclipse-openj9/openj9-docs#324

@pshipton
Copy link
Member Author

@vijaysun-omr @mpirvu @amicic @hangshao0 fyi

@hangshao0
Copy link
Contributor

I guess we are doing this only on gencon ?

@vijaysun-omr
Copy link
Contributor

Maybe balanced GC too ? We have'nt done any experimenting with balanced GC to say if/how much it helps but I'd like to think that we need to stop treating balanced as a second class citizen if it's not a huge amount of extra work.

@pshipton
Copy link
Member Author

bin/java -Xgcpolicy:balanced -verbose:sizes

  -Xmns8M         initial new space size
  -Xmnx128M       maximum new space size

  -Xmos8M         initial old space size
  -Xmox512M       maximum old space size

@amicic
Copy link
Contributor

amicic commented Nov 22, 2018

-Xmn is relevant in Balanced as much as in Gencon (although they have a bit differen meaning: total Nursery for Gencon which is both Allocate and Survivor vs only Allocate (actually called Eden) in Balanced). Bottom line, we should treat them same (we should set Xmns based on recommendation stored in SC)

I thought that -Xmo had no semantical meaning in Balanced, but we do seem to obey the command and do something about it. I just did a quick test and what I can tell is that effectively ends up affecting the total heap sizing (pretty much acting as Xmx/Xms commands). See for example this:

./java -verbose:gc,sizes -Xmx64M -Xmos32M -Xgcpolicy:gencon

-Xmns10880K initial new space size
-Xmnx16M maximum new space size
-Xms43648K initial memory size
-Xmos32M initial old space size
-Xmox54656K maximum old space size
-Xmx64M memory maximum

vs

./java -verbose:gc,sizes -Xmx64M -Xmos32M -Xgcpolicy:balanced

-Xmns16M initial new space size
-Xmnx16M maximum new space size
-Xms32M initial memory size
-Xmos32M initial old space size
-Xmox64M maximum old space size
-Xmx64M memory maximum

While, having different meaning for Gencon and Balanced (old space vs all space), I think it would still be ok to use it.

In short, -Xmns and Xmos could be used for both GC policies to uniquely determine initial total heap and division between Nursery/Eden vs the rest of the heap.

@dmitripivkine
Copy link
Contributor

However options should be applied wisely because it might depend on GC policy they were taken. Should stored values be accompanied by GC policy or we can make them invariant?

@pshipton
Copy link
Member Author

Yes, we'll have to work out all the relevant environment options that need to match. Or we could store the entire command line.

  • application (main class name)
  • gc policy
  • Xmx

@DanHeidinga
Copy link
Member

We should design the data record to be extensible in the future. I think there are other values that would be interesting to store from run to run.

@mpirvu
Copy link
Contributor

mpirvu commented Nov 23, 2018

Another point to consider: Most of the information stored in SCC is read only, one notable exception being the JIT hints. For most flexibility it would be best to allow RW access to these GC hints.

@hangshao0
Copy link
Contributor

We can store the main class name and GC policy. If we want to store Xmx, we might want to store Xms as well. There could be so many combinations of Xmx and Xms.

@hangshao0
Copy link
Contributor

hangshao0 commented Nov 23, 2018

Probably we should turn this feature off if user specified any of -Xmns/-Xmnx/-Xmn/-Xmos/-Xmox/-Xmo/-Xms/-Xmx. I guess -Xmoi, -Xmine, -Xmaxe also matter here ?

@pshipton
Copy link
Member Author

pshipton commented Nov 23, 2018

If we want to store Xmx, we might want to store Xms as well. There could be so many combinations of Xmx and Xms.

The suggestion to store -Xmx was for the purpose of validating the hint. i.e. a different -Xmx invalidates the hint. However, I'm leaning towards storing the entire command line. As long as the command line remains the same, the gc sizing hints for that command line remain valid. There can be different hints stored for different command lines. I think storing the entire command line should be the first approach. Afterwards we could consider filtering out specific command line options as not being relevant to the GC hints, but not sure this is a necessary feature. During production I expect the command lines don't change.

Probably we should turn this feature off if user specified any of -Xmns/-Xmnx/-Xmn/-Xmos/-Xmox/-Xmo

Agreed. If the values are set explicitly then they shouldn't be overridden.

@hangshao0
Copy link
Contributor

hangshao0 commented Nov 26, 2018

Here is an example of the entire command line options if I run a simple app:

java -verbose:init --module-path /bluebird/builds/bld_403141/jvmtest/test/SE90/functional/cmdLineTests/utils/utils.jar -m utils/org.openj9.test.ivj.Hanoi 2

Option 0 optionString="-Xoptionsfile=/team/hangshao/JVM29/jvmxa6490/lib/options.default" extraInfo=(nil) from environment variable ="N/A"
Option 1 optionString="-Xlockword:mode=default,noLockword=java/lang/String,noLockword=java/util/MapEntry,noLockword=java/util/HashMap$Entry,noLockword=org/apache/harmony/luni/util/ModifiedMap$Entry,noLockword=java/util/Hashtable$Entry,noLockword=java/lang/invoke/MethodType,noLockword=java/lang/invoke/MethodHandle,noLockword=java/lang/invoke/CollectHandle,noLockword=java/lang/invoke/ConstructorHandle,noLockword=java/lang/invoke/ConvertHandle,noLockword=java/lang/invoke/ArgumentConversionHandle,noLockword=java/lang/invoke/AsTypeHandle,noLockword=java/lang/invoke/ExplicitCastHandle,noLockword=java/lang/invoke/FilterReturnHandle,noLockword=java/lang/invoke/DirectHandle,noLockword=java/lang/invoke/ReceiverBoundHandle,noLockword=java/lang/invoke/DynamicInvokerHandle,noLockword=java/lang/invoke/FieldHandle,noLockword=java/lang/invoke/FieldGetterHandle,noLockword=java/lang/invoke/FieldSetterHandle,noLockword=java/lang/invoke/StaticFieldGetterHandle,noLockword=java/lang/invoke/StaticFieldSetterHandle,noLockword=java/lang/invoke/IndirectHandle,noLockword=java/lang/invoke/InterfaceHandle,noLockword=java/lang/invoke/VirtualHandle,noLockword=java/lang/invoke/PrimitiveHandle,noLockword=java/lang/invoke/InvokeExactHandle,noLockword=java/lang/invoke/InvokeGenericHandle,noLockword=java/lang/invoke/VarargsCollectorHandle,noLockword=java/lang/invoke/ThunkTuple" extraInfo=(nil) from environment variable ="N/A"
Option 2 optionString="-Xjcl:jclse9_29" extraInfo=(nil) from environment variable ="N/A"
Option 3 optionString="-Dcom.ibm.oti.vm.bootstrap.library.path=/team/hangshao/JVM29/jvmxa6490/lib/amd64/compressedrefs:/team/hangshao/JVM29/jvmxa6490/lib/amd64" extraInfo=(nil) from environment variable ="N/A"
Option 4 optionString="-Dsun.boot.library.path=/team/hangshao/JVM29/jvmxa6490/lib/amd64/compressedrefs:/team/hangshao/JVM29/jvmxa6490/lib/amd64" extraInfo=(nil) from environment variable ="N/A"
Option 5 optionString="-Djava.library.path=/team/hangshao/JVM29/jvmxa6490/lib/amd64/compressedrefs:/team/hangshao/JVM29/jvmxa6490/lib/amd64:/usr/local/cuda-5.5/lib64:.:/usr/lib64:/usr/lib" extraInfo=(nil) from environment variable ="N/A"
Option 6 optionString="-Djava.home=/team/hangshao/JVM29/jvmxa6490" extraInfo=(nil) from environment variable ="N/A"
Option 7 optionString="-Duser.dir=/team/hangshao/JVM29/jvmxa6490/bin" extraInfo=(nil) from environment variable ="N/A"
Option 8 optionString="-Djava.runtime.version=pxa6490ea-20170614_01" extraInfo=(nil) from environment variable ="N/A"
Option 9 optionString="-verbose:init" extraInfo=(nil) from environment variable ="N/A"
Option 10 optionString="--module-path=/bluebird/builds/bld_403141/jvmtest/test/SE90/functional/cmdLineTests/utils/utils.jar" extraInfo=(nil) from environment variable ="N/A"
Option 11 optionString="-Djdk.module.main=utils" extraInfo=(nil) from environment variable ="N/A"
Option 12 optionString="-Dsun.java.command=utils/org.openj9.test.ivj.Hanoi 2" extraInfo=(nil) from environment variable ="N/A"
Option 13 optionString="-Dsun.java.launcher=SUN_STANDARD" extraInfo=(nil) from environment variable ="N/A"
Option 14 optionString="-Dsun.java.launcher.pid=21548" extraInfo=(nil) from environment variable ="N/A"
Option 15 optionString="_org.apache.harmony.vmi.portlib" extraInfo=0x7f278000ca20 from environment variable ="N/A"

There are so many default options prepended/appended, which are not related to this feature at all. The old space and new space sizes are only two numbers, but the entire CML is such a long string. I guess it is not worth storing the whole CML, Also there is an option -Dsun.java.launcher.pid, which will be different from run to run.

If GC is going to check for the presence of -Xmns/-Xmnx/-Xmn/-Xmos/-Xmox/-Xmo/-Xmx/-Xms/... and gc policy to decide whether to turn off this feature, storing the main class probably should be sufficient.

@pshipton
Copy link
Member Author

pshipton commented Nov 26, 2018

Just the main class isn't sufficient, other parameters such as -Xmx, -Xms and gcpolicy need to be stored as well. If they change then the hint is invalidated. We can try to find a balance between parameters that matter and parameters that don't. We can filter all the default options out of the parameter list and maybe some other specific parameters as well, but in general I think if options are modified then the behavior of the app can change and invalidate the hint.

@hangshao0
Copy link
Contributor

hangshao0 commented Nov 27, 2018

I will save the new/old space sizes as well as the following info:
main module/main class (sun.java.command)
-Xgc and -Xgcpolicy
-Xmx
-Xms
-Xsoftmx
-Xmoi
java.class.path
jdk.module.path

Do you see any GC options that are missing here ? @amicic @dmitripivkine

@amicic
Copy link
Contributor

amicic commented Nov 27, 2018

As @pshipton suggested, I would not even try to recognize various -Xm? options (or any other option). If anything in options changed, the hints would be invalidated. It would be, in general, complicated to try to interpret -Xm? options to validate that they effectively mean the same initial/total heap sizing, even though the option themselves are different.

For example, these two things are effectively same:

  1. -Xmx4G -Xms4G
  2. -Xmx4G -Xms4G -Xmn1G
    but I would still invalidate the hints.

The hints for initial heap sizing would come from internal API and have no relationship with the options. We are yet to agree what API is to be used, but effectively we need two:

  • get/set total initial heap size (what normally is controlled by -Xms option)
  • get/set Nursery/Eden initial heap size (what normally is controlled by -Xmns option).
    On the first run those would be default ones (8M/2M respectively for total/Nursery), but on subsequent, they would be higher values, as suggested by SC hints.

@dmitripivkine
Copy link
Contributor

I believe we need keep GC policy as well

@DanHeidinga
Copy link
Member

If anything in options changed, the hints would be invalidated.

+1 this to approach. For the initial implementation, we should just validate the commandline is the same.

@hangshao0
Copy link
Contributor

hangshao0 commented Nov 27, 2018

I believe we need keep GC policy as well

Yes, I should say -Xgc and -Xgcpolicy

@hangshao0
Copy link
Contributor

OK. Then I am going the save the entire command line.

@DanHeidinga
Copy link
Member

@amicic gentle ping. Can you update this with the outlook?

@amicic
Copy link
Contributor

amicic commented Jun 4, 2019

With some luck there is not much work left:

  • make sure everything is still in order. I already tried it briefly a few days ago and while it seemed to work most of the time, it seemed like it would intermittently lose hints. Need to investigate more.
  • introduce a clean option to enable/disable the feature (right now I control it with an internal tuning/weight parameter which is set to 0, which effectively disables the feature).
  • test race conditions between early first GC and heap expansion based on hints
  • ideally, test high JVM count simultaneous startup scenario (SC lock contention or anything else) - I'm unlikely to do anything about it, since I don't have such scenario available.

Balanced GC will not be covered in the first release.

@pshipton
Copy link
Member Author

pshipton commented Jun 4, 2019

test high JVM count simultaneous startup scenario

This is just a matter of starting a number of JVMs at approximately the same time, such as starting up 20 JVMs running the SwingSet demo.

@DanHeidinga
Copy link
Member

The option has been added for 0.15. Moving to 0.16 to consider enabling by default

@amicic
Copy link
Contributor

amicic commented Jun 18, 2019

I've done some basic testing to confirm everything is order, and there are no races with first GC.

@pshipton
Copy link
Member Author

@amicic has the concern about always writing to the cache been addressed? i.e. the GC code will only attempt to write a hint if it has calculated a different value than has already been stored to the cache? If this is addressed, I suggest we enable the feature by default just after branching for the 0.15 release, to give time to identify any problems before the next release.

@amicic
Copy link
Contributor

amicic commented Jun 18, 2019

The hint is always updated. Values will almost always be different. I have not done anything to test if there is a problem with excessive updates.
We could still try to reduce it, blindly. Perhaps update it if 1) it differs by some margin (>10%) or 2) with low probability (10%) even if within the margin. The latter is to ensure it's updated in very gradual run-to-run load change.

@pshipton
Copy link
Member Author

pshipton commented Jun 19, 2019

We shouldn't enable the feature by default until this has been resolved. Writing to the shared cache on every startup affects starting up multiple JVMs concurrently, as each JVM will serialize on the write to the cache.

@amicic
Copy link
Contributor

amicic commented Jun 19, 2019

A multiJVM test... I used a fairly simple workload (single threaded, very few classes and methods). Started 50 JVMs simultaneously, on a machine with 120 threads. GC thread and compilation thread count reduced to 2 and 1 respectively.
Measured average time to complete the tests as reported by Linux time command, in seconds.

4 group of tests, with 4-6 sequential runs within each group. Before each group, SC was wiped.

SC alone appears to regress completion time (since the test is so simple, there is probably no benefit of SC, but there is some cost of maintaining SC?). However, use of hints does not seem to introduce additional regression.

no SC
7.7104
7.08792
7.25356
8.0851

SC, no hints
9.23496
8.95598
9.325
8.82136

SC, use hints
7.41268
10.0347
7.37226
8.4999
8.2855
8.60048

SC, use hints, skip insignificant updates
7.53726
8.9249
9.76732
8.5943
10.1544
7.5088

@hangshao0
Copy link
Contributor

SC alone appears to regress completion time (since the test is so simple, there is probably no benefit of SC, but there is some cost of maintaining SC?).

Might be the same issue as #5918. Using -Xshareclasses:noTimestampChecks can workaround this issue.

@amicic
Copy link
Contributor

amicic commented Jun 19, 2019

@pshipton I understand the concern. There might be a real problem, but I'm reluctant to add additional logic before observing a problem and proving the logic helps. If you or somebody else try and succeed to create a problem with their own scenarios, I'm more than willing to investigate...

@pshipton
Copy link
Member Author

pshipton commented Jun 19, 2019

@amicic based on the results in #3743 (comment), I agree it seems fine as-is. What platform did you test on? nm, I see Linux.

@pshipton
Copy link
Member Author

@amicic do you agree to enable -XX:+UseGCStartupHints by default next week after the branch for the 0.15 release?

@DanHeidinga
Copy link
Member

@amicic We've branched for 0.15.0. Can enable -XX:+UseGCStartupHints by default?

amicic added a commit to amicic/omr that referenced this issue Jun 25, 2019
As requested by
eclipse-openj9/openj9#3743 (comment)
(the feature is really only available in OpenJ9)


Signed-off-by: Aleksandar Micic <amicic@ca.ibm.com>
@amicic
Copy link
Contributor

amicic commented Jun 26, 2019

PR that enables it by default has just been merged.

@pshipton
Copy link
Member Author

pshipton commented Jun 26, 2019

Since eclipse/omr#4079 is merged, I believe this can be closed.

oneturkmen added a commit to oneturkmen/openj9 that referenced this issue Jul 23, 2019
Some of the methods in SymbolReferenceTable in OMR
are used only by OpenJ9.

Fixes: eclipse-openj9#3743 (OMR repo)

Signed-off-by: Batyr Nuryyev <nuryyev@ualberta.ca>
oneturkmen added a commit to oneturkmen/openj9 that referenced this issue Aug 1, 2019
Some of the methods in SymbolReferenceTable in OMR are used only by OpenJ9.

Fixes: [eclipse-openj9#3743](eclipse/omr#3743) (in OMR repo)

Signed-off-by: Batyr Nuryyev <nuryyev@ualberta.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants