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

Adding Tenure bytes deviation to modify concurrent kickoff #4134

Merged
merged 1 commit into from Jul 25, 2019

Conversation

@cedrichansen
Copy link
Contributor

commented Jul 16, 2019

  • Added heuristics to keep track of bytes moving from nursery to tenure, along with the deviation of this value.
  • _avgTenureBytes keeps track of average bytes that are being tenured, while _avgTenureBytesDeviation keeps track of the average amount of deviation in bytes being tenured.
  • Adding this deviation into the calculation for concurrent kickoff
  • The default boost for the deviation on top of the existing kickoff logic is 2x. This field is called tenureBytesDeviationBoost
  • This change results in far less aborts/percolates, but at the expense of more global GC cycles
  • Will be accompanied by a change on the j9 side for explicitly setting tenureBytesDeviationBoost. Setting this to 0 will effectively disable it's effects
  • This calculation is done for both SOA bytes (if OMR_GC_LARGE_OBJECT_AREA flag is set), and tenured bytes in general if not set

Signed-off-by: Cedric Hansen cedric.hansen@ibm.com

cedrichansen added a commit to cedrichansen/openj9 that referenced this pull request Jul 16, 2019
Parsing Command line option for tenureSOABytesDeviationBoost
- example usage: -Xgc:tenureSOABytesDeviationBoost=25 , will result in a 2.5 boost.
- Refer to eclipse/omr#4134

Signed-off-by: Cedric Hansen <cedric.hansen@ibm.com>

@cedrichansen cedrichansen force-pushed the cedrichansen:soaDeviation branch 12 times, most recently from 2919c4b to 442e727 Jul 16, 2019

@@ -267,6 +267,7 @@ class MM_GCExtensionsBase : public MM_BaseVirtual {
uintptr_t oldHeapSizeOnLastGlobalGC;
uintptr_t freeOldHeapSizeOnLastGlobalGC;
float concurrentKickoffTenuringHeadroom; /**< percentage of free memory remaining in tenure heap. Used in conjunction with free memory to determine concurrent mark kickoff */
float tenureSOABytesDeviationBoost;

This comment has been minimized.

Copy link
@amicic

amicic Jul 18, 2019

Contributor

add a comment somethink like: boost factor for tenuring deviation used for concurrent mark kickoff math

@cedrichansen cedrichansen force-pushed the cedrichansen:soaDeviation branch 2 times, most recently from 0d2d05e to 364483f Jul 18, 2019

@cedrichansen cedrichansen changed the title Adding Tenure SOA bytes deviation to modify concurrent kickoff Adding Tenure bytes deviation to modify concurrent kickoff Jul 18, 2019

@cedrichansen cedrichansen force-pushed the cedrichansen:soaDeviation branch 4 times, most recently from 3f68651 to 73eb9f5 Jul 18, 2019

@cedrichansen cedrichansen changed the title Adding Tenure bytes deviation to modify concurrent kickoff WIP: Adding Tenure bytes deviation to modify concurrent kickoff Jul 23, 2019

@cedrichansen cedrichansen force-pushed the cedrichansen:soaDeviation branch 2 times, most recently from af38dfa to 82c4882 Jul 23, 2019

@cedrichansen cedrichansen force-pushed the cedrichansen:soaDeviation branch from 82c4882 to cea1960 Jul 24, 2019

Adding Tenure bytes deviation to modify concurrent kickoff
- Added heuristics to keep track of bytes moving from nursery to tenure, along with the deviation of this value.
- `_avgTenureBytes` keeps track of average bytes that are being tenured, while `_avgTenureBytesDeviation` keeps track of the average amount of deviation in bytes being tenured.
- Adding this deviation into the calculation for concurrent kickoff
- The default boost for the deviation on top of the existing kickoff logic is 2x. This field is called `tenureBytesDeviationBoost`
- This change results in far less aborts/percolates, but at the expense of more global GC cycles
- Will be accompanied by a change on the j9 side for explicitly setting `tenureBytesDeviationBoost`. Setting this to 0 will effectively disable it's effects
- This calculation is done for both SOA bytes (if OMR_GC_LARGE_OBJECT_AREA flag is set), and tenured bytes in general if not set

Signed-off-by: Cedric Hansen <cedric.hansen@ibm.com>

@cedrichansen cedrichansen force-pushed the cedrichansen:soaDeviation branch from cea1960 to aec40ce Jul 24, 2019

@cedrichansen cedrichansen changed the title WIP: Adding Tenure bytes deviation to modify concurrent kickoff Adding Tenure bytes deviation to modify concurrent kickoff Jul 24, 2019

@amicic
amicic approved these changes Jul 24, 2019
@@ -835,7 +835,20 @@ MM_Scavenger::calcGCStats(MM_EnvironmentStandard *env)
#if defined(OMR_GC_LARGE_OBJECT_AREA)
scavengerGCStats->_avgTenureSOABytes = (uintptr_t)MM_Math::weightedAverage((float)scavengerGCStats->_avgTenureSOABytes,
(float)(scavengerGCStats->_tenureAggregateBytes - scavengerGCStats->_tenureLOABytes),
TENURE_BYTES_HISTORY_WEIGHT);
TENURE_BYTES_HISTORY_WEIGHT);

This comment has been minimized.

Copy link
@amicic

amicic Jul 24, 2019

Contributor

For easier reading introduce a local variable:

uintptr_t tenureSOABytes = scavengerGCStats->_tenureAggregateBytes - scavengerGCStats->_tenureLOABytes;

if ((scavengerGCStats->_tenureAggregateBytes - scavengerGCStats->_tenureLOABytes) > (scavengerGCStats->_avgTenureSOABytes)) {
scavengerGCStats->_tenureSOABytesDeviation = (scavengerGCStats->_tenureAggregateBytes - scavengerGCStats->_tenureLOABytes) - (scavengerGCStats->_avgTenureSOABytes);
} else {
scavengerGCStats->_tenureSOABytesDeviation = (scavengerGCStats->_avgTenureSOABytes) - (scavengerGCStats->_tenureAggregateBytes - scavengerGCStats->_tenureLOABytes) ;

This comment has been minimized.

Copy link
@amicic

amicic Jul 24, 2019

Contributor

_tenureSOABytesDeviation could be float, in which case you don't have to worry about overflow (what's larger or not in the subtraction). deviation is later squared, so the sign is irrelevant.

scavengerGCStats->_tenureSOABytesDeviation = (scavengerGCStats->_avgTenureSOABytes) - (scavengerGCStats->_tenureAggregateBytes - scavengerGCStats->_tenureLOABytes) ;
}

scavengerGCStats->_avgTenureSOABytesDeviation = (uintptr_t)sqrtf(MM_Math::weightedAverage((float)(scavengerGCStats->_avgTenureSOABytesDeviation * scavengerGCStats->_avgTenureSOABytesDeviation),

This comment has been minimized.

Copy link
@amicic

amicic Jul 24, 2019

Contributor

_avgTenureSOABytesDeviation could be float

@@ -1836,7 +1836,8 @@ MM_ConcurrentGC::potentialFreeSpace(MM_EnvironmentBase *env, MM_AllocateDescript
headRoom = (uintptr_t)(_extensions->concurrentKickoffTenuringHeadroom * _extensions->lastGlobalGCFreeBytesLOA);
} else {
assume0(SOA == _meteringType);
nurseryPromotion = scavengerStats->_avgTenureSOABytes == 0 ? 1 : scavengerStats->_avgTenureSOABytes;
nurseryPromotion = scavengerStats->_avgTenureSOABytes == 0 ? 1 : (uintptr_t)(scavengerStats->_avgTenureSOABytes + (env->getExtensions()->tenureSOABytesDeviationBoost * scavengerStats->_avgTenureSOABytesDeviation));

This comment has been minimized.

Copy link
@amicic

amicic Jul 24, 2019

Contributor

need to cover the case when LOA is not compile time enabled

This comment has been minimized.

Copy link
@amicic

amicic Jul 24, 2019

Contributor

I would not worry about the case when LOA metering is enabled (deviation there could be very large and completely unpredictable). we would maintain and consume tenuring deviation only into SOA or total tenure, depending if LOA is enabled or not.

@@ -111,6 +111,8 @@ class MM_ScavengerStats
#if defined(OMR_GC_LARGE_OBJECT_AREA)
uintptr_t _avgTenureLOABytes;
uintptr_t _avgTenureSOABytes;
uintptr_t _avgTenureSOABytesDeviation;
uintptr_t _tenureSOABytesDeviation;

This comment has been minimized.

Copy link
@amicic

amicic Jul 24, 2019

Contributor

as mentioned above we'd need _avgTenureBytesDeviation/_tenureBytesDeviation pair (outside of #if LOA) block as well. or just one pair of fields (the more generic one). and reuse them for SOA, for the case when LOA is enabled.

scavengerGCStats->_avgTenureSOABytes = scavengerGCStats->_tenureAggregateBytes - scavengerGCStats->_tenureLOABytes;
scavengerGCStats->_avgTenureLOABytes = scavengerGCStats->_tenureLOABytes;
#endif /* OMR_GC_LARGE_OBJECT_AREA */
scavengerGCStats->_avgTenureBytes = (uintptr_t)(scavengerGCStats->_flipBytes / 2);

This comment has been minimized.

Copy link
@amicic

amicic Jul 24, 2019

Contributor

add a comment here, something like (shorten and change it to your liking): Since initial tenureage is >=1, on first scavenge we don't have any tenuring. However, on the second Scavenge, there could be a large amount of tenuring, leading to unreasonable large deviation. Let's assume about half of those flipped now, will be tenured next cycle - this should lead to more realistic averages and deviations in next few cycles.

@amicic

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2019

this looks good to me. additional reviews and testing are welcome.

cedrichansen added a commit to cedrichansen/openj9 that referenced this pull request Jul 24, 2019
Parsing Command line option for tenureSOABytesDeviationBoost
- example usage: -Xgc:tenureSOABytesDeviationBoost=25 , will result in a 2.5 boost.
- Refer to eclipse/omr#4134

Signed-off-by: Cedric Hansen <cedric.hansen@ibm.com>
cedrichansen added a commit to cedrichansen/openj9 that referenced this pull request Jul 24, 2019
Parsing Command line option for tenureBytesDeviationBoost
- example usage: -Xgc:tenureBytesDeviationBoost=25 , will result in a 2.5 boost.
- Refer to eclipse/omr#4134

Signed-off-by: Cedric Hansen <cedric.hansen@ibm.com>
@charliegracie

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2019

@genie-omr build all

@charliegracie charliegracie merged commit c4a5982 into eclipse:master Jul 25, 2019

14 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/eclipse-omr/pr/aix_ppc-64 Build finished.
Details
continuous-integration/eclipse-omr/pr/linux_390-64 Build finished.
Details
continuous-integration/eclipse-omr/pr/linux_aarch64 Build finished.
Details
continuous-integration/eclipse-omr/pr/linux_arm Build finished.
Details
continuous-integration/eclipse-omr/pr/linux_ppc-64_le_gcc Build finished.
Details
continuous-integration/eclipse-omr/pr/linux_x86 Build finished.
Details
continuous-integration/eclipse-omr/pr/linux_x86-64 Build finished.
Details
continuous-integration/eclipse-omr/pr/linux_x86-64_cmprssptrs Build finished.
Details
continuous-integration/eclipse-omr/pr/osx_x86-64 Build finished.
Details
continuous-integration/eclipse-omr/pr/win_x86-64 Build finished.
Details
continuous-integration/eclipse-omr/pr/zos_390-64 Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
eclipsefdn/eca The author(s) of the pull request is covered by necessary legal agreements in order to proceed!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.