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

Heap size startup hints #4168

Merged
merged 1 commit into from
Jan 8, 2019
Merged

Conversation

amicic
Copy link
Contributor

@amicic amicic commented Jan 2, 2019

Initial contribution of feature where on PHASE_NOT_STARTUP phase change
GC will store current heap sizes (optionally, averaged with already
existing stored values) into Shared Cache. During early startup GC will
use these hints to expand heap beyond default or even explicitly
specified sizes.

This should help large workloads to avoid early GCs while heap is
aggressively expanding, which in turn may help with startup times.

In this contribution, Standard group of GC policies are covered, with
Balanced GC following at a later point.

For Flat heap configurations (optthruput/optavgpause) only the total
heap size is maintained/expanded, while for generational configuration,
Tenure and Nursery are separately maintained/expanded.

The feature is disabled by default, but can be enabled by specifying
value [1-100] to -XXgc:heapSizeStatupHintWeightNewValue= command line
option. Reasonable value is 80, which should typically take about 5
restarts for hints to converge to stable values. Value 0 (currently
defaults) means that new values are never accounted for, which means
that values are always 0, which effectively means that feature is
disabled. Value 100 would mean that only use the hints from the most
recent run, hence no historical averaging.

Another tuning options is -XXgc:heapSizeStatupHintConservativeFactor=
Default value is 70. It means that conservatively only 70% of the hint
values are taken.

An example of verbose GC stanzas reporting the expand just after
initialized stanza is complete, before even first GC:

.....
</initialized>

<heap-resize id="2" type="expand" space="nursery" amount="1461583872"
count="1" timems="1.738" reason="hint from previous runs"
timestamp="2019-01-03T12:59:54.909" />
<heap-resize id="3" type="expand" space="tenure" amount="200212480"
count="1" timems="0.930" reason="hint from previous runs"
timestamp="2019-01-03T12:59:54.910" />

Signed-off-by: Aleksandar Micic amicic@ca.ibm.com

@amicic
Copy link
Contributor Author

amicic commented Jan 2, 2019

Dependent on eclipse/omr#3415

hintDefault = (UDATA)(extensions->heapSizeStatupHintWeightNewValue * hintDefault + (1.0 - extensions->heapSizeStatupHintWeightNewValue) * hintDefaultOld);
hintTenure = (UDATA)(extensions->heapSizeStatupHintWeightNewValue * hintTenure + (1.0 - extensions->heapSizeStatupHintWeightNewValue) * hintTenureOld);

vm->sharedClassConfig->storeGCHints(currentThread, hintDefault, hintTenure, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

as we discussed we should double check are we allowed to update shared cache every JVM run

Copy link
Member

@pshipton pshipton Jan 7, 2019

Choose a reason for hiding this comment

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

The hints in the shared cache can be updated, although it does lock down the cache while doing so. Fewer updates are better. The hints are updated in place, there is no shared cache expansion involved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is only a single update per VM run (on PHASE_NOT_STARTUP transition).

Copy link
Member

Choose a reason for hiding this comment

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

It basically serializes JVM startup. i.e. say 100 JVMs are started at the same time, all of them will wait to update the shared cache, one at a time.

Copy link
Contributor

@dmitripivkine dmitripivkine left a comment

Choose a reason for hiding this comment

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

I believe this is good start point implementation. We should double check:

  • Are we allowed to modify shared cache every JVM run?
  • Is it cheap to call findGCHints() twice (or values should be stored in GC Extensions)?
  • Can asynchronous call of gcExpandHeapOnStartup() potentially interfere with any concurrent GC activity?

@amicic
Copy link
Contributor Author

amicic commented Jan 3, 2019

@dmitripivkine, all valid concerns.

  • Best to my current understanding, shared cache will indeed grow an every update. It's not a large update, in order of 100 bytes, but still, it would be bad to allow unbounded updates for the final solution.
    Based on my limited testing, I'd like to have the updating capability, since I can see that hint sizes need a few update before they really converge. So, we can cap the number of updates to some number (10 or so) or we can stop updating when we sense that values did converge (for example, the updated value is within 10-20% of previous value). We'll know better if the updates are really useful at all and if yes till what point after some more feedback from other people/workloads. Since, the feature is disabled by default, I dont see the unbounded updates as immediate concern for this initial implementation.
  • Not sure about this, but i'll try to find out
  • Indeed, it's quite possible we'll need to ensure that overlap does not happen. Changing boundaries of Tenure/Nursery in a middle of Concurrent Mark/Scavenger for various reasons is dangerous. We should probably add same expand functionality at a beginning of any GC, so that which ever one comes first (async one at startup or first GC) does it.

@dmitripivkine dmitripivkine changed the title Heap size startup hints WIP:Heap size startup hints Jan 3, 2019
@amicic amicic force-pushed the heapsizehints branch 2 times, most recently from 59752e4 to 43d5578 Compare January 3, 2019 17:23
@dmitripivkine
Copy link
Contributor

dmitripivkine commented Jan 7, 2019

I spoke with @DanHeidinga and @pshipton about my questions. Just to summarize:

Are we allowed to modify shared cache every JVM run?

Yes. Adding a limit of number of times to be updated seems is a good idea. Value is updated at place, no structure grows occur. However writing to shared classes cache would require monitor enter (possible slow down)

Is it cheap to call findGCHints() twice (or values should be stored in GC Extensions)?

Yes. These values are cached.


if (NULL != sharedClassConfig) {
if (extensions->isStandardGC()) {
uintptr_t hintDefault, hintTenure;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add default values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Copy link
Contributor

@charliegracie charliegracie left a comment

Choose a reason for hiding this comment

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

Approved based on my one suggestion being applied

Initial contribution of feature where on PHASE_NOT_STARTUP phase change
GC will store current heap sizes (optionally, averaged with already
existing stored values) into Shared Cache. During early startup GC will
use these hints to expand heap beyond default or even explicitly
specified sizes.

This should help large workloads to avoid early GCs while heap is
aggressively expanding, which in turn may help with startup times.

In this contribution, Standard group of GC policies are covered, with
Balanced GC following at a later point.

For Flat heap configurations (optthruput/optavgpause) only the total
heap size is maintained/expanded, while for generational configuration,
Tenure and Nursery are separately maintained/expanded.

The feature is disabled by default, but can be enabled by specifying
value [1-100] to -XXgc:heapSizeStatupHintWeightNewValue= command line
option. Reasonable value is 80, which should typically take about 5
restarts for hints to converge to stable values. Value 0 (currently
defaults) means that new values are never accounted for, which means
that values are always 0, which effectively means that feature is
disabled. Value 100 would mean that only use the hints from the most
recent run, hence no historical averaging.

Another tuning options is -XXgc:heapSizeStatupHintConservativeFactor=
Default value is 70. It means that conservatively only 70% of the hint
values are taken.

An example of verbose GC stanzas reporting the expand just after
initialized stanza is complete, before even first GC:

```
.....
</initialized>

<heap-resize id="2" type="expand" space="nursery" amount="1461583872"
count="1" timems="1.738" reason="hint from previous runs"
timestamp="2019-01-03T12:59:54.909" />
<heap-resize id="3" type="expand" space="tenure" amount="200212480"
count="1" timems="0.930" reason="hint from previous runs"
timestamp="2019-01-03T12:59:54.910" />
```

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

Jenkins test sanity xLinux jdk11

@dmitripivkine dmitripivkine changed the title WIP:Heap size startup hints Heap size startup hints Jan 8, 2019
@dmitripivkine dmitripivkine merged commit 8d181eb into eclipse-openj9:master Jan 8, 2019
@amicic amicic mentioned this pull request Jan 9, 2019
amicic added a commit to amicic/openj9 that referenced this pull request Jan 9, 2019
Caused by eclipse-openj9#4168

20:51:30 mmparseXXgc.cpp(950) : error C2220: warning treated as error -
no 'object' file generated
20:51:30 mmparseXXgc.cpp(950) : warning C4244: '=' : conversion from
'double' to 'float', possible loss of data
20:51:30 mmparseXXgc.cpp(964) : warning C4244: '=' : conversion from
'double' to 'float', possible loss of data

Signed-off-by: Aleksandar Micic <amicic@ca.ibm.com>
pshipton added a commit to pshipton/openj9 that referenced this pull request Jan 9, 2019
Related to eclipse-openj9#4168 and eclipse-openj9#3743

Signed-off-by: Peter Shipton <Peter_Shipton@ca.ibm.com>
@pshipton
Copy link
Member

pshipton commented Jan 9, 2019

btw the options contain a typo "Statup"

@pshipton
Copy link
Member

@amicic do you want me to open a new issue for the typo in the option names?
Also the new options have not been documented in the user guide. Is this the desired behavior while we experiment with the new options?

@amicic
Copy link
Contributor Author

amicic commented Jan 11, 2019

@pshipton Thanks for noticing the typo. It is about to be fixed (#4240). I don't think we really need an issue for that.

We will probably introduce a anew public option (@charliegracie mentioned something like -XX:+/-UseGCStartupHints, you are laos welcome to suggest something else - should we mention sharedcache in the name?) that we'd document. The two existing ones, I would not document (and I'm not even completely sure they will stay around after we do some more experimentation).

@pshipton
Copy link
Member

I'm thinking UseGCStartupCacheHints or a variation of this that mentions the shared cache would be better.

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

Successfully merging this pull request may close these issues.

None yet

4 participants