Skip to content

Commit 0a577cd

Browse files
committed
Fix PGC edenSurvivalRate overflow issue
The edenSurvivalRate is representing the number of regions out of those allocated for this Eden which survived(typically 0.0 <= the Rate <= 1.0). The edenSurvivalRate overflow (> 1.0) might cause incorrect Heap and Eden size change(unexpected spike/noise) and for some cases might cause zero free memory and trigger out of memory exception. the issue is not regression, but just exposed by recent edenResize change(it might cause indirect serious GC performance issues, but did not trigger any crashes/exceptions/assertions). There are two different overflow cases, one is major, another is minor. Major issue is incorrect edenCountBeforeCollect. During GC, the heap may expand to allow the current collection to continue. A heap resize triggers an eden size recalculation without allowing further heap expansion (heap reconfiguration). As a result, the newly calculatedneden size can differ significantly from the eden size at the start of the collection, causing the SurvivalRate calculation to become inaccurate. Therefore, set copyForwardStats->_edenEvacuateRegionCount (instead of getCurrentEdenSizeInRegions(env)) to edenCountBeforeCollect. Minor issue is _scanBytesEden double-counts. When abort is in progress, every Eden object scanned in abort-recovery contributes to _scannedBytes. But an object that was partially copied before abort (the copy succeeded, the forwarding pointer was installed) is also counted again during the abort scan of the source region. So _scanBytesEden might be double-counted, then cause edenSurvivorCount is bigger than edenCountBeforeCollect. For this case, set edenSurvivorCount = edenCountBeforeCollect. Signed-off-by: lhu <linhu@ca.ibm.com>
1 parent f9113a3 commit 0a577cd

1 file changed

Lines changed: 20 additions & 1 deletion

File tree

runtime/gc_vlhgc/SchedulingDelegate.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,15 @@ MM_SchedulingDelegate::partialGarbageCollectCompleted(MM_EnvironmentVLHGC *env,
372372
bool globalSweepHappened = _globalSweepRequired;
373373
_globalSweepRequired = false;
374374
/* copy out the Eden size of the previous interval (between the last PGC and this one) before we recalculate the next one */
375-
uintptr_t edenCountBeforeCollect = getCurrentEdenSizeInRegions(env);
375+
/* During GC, the heap may expand to allow the current collection to continue.
376+
* A heap resize triggers an eden size recalculation without allowing further
377+
* heap expansion (heap reconfiguration). As a result, the newly calculated
378+
* eden size can differ significantly from the eden size at the start of the
379+
* collection, causing the SurvivalRate calculation to become inaccurate.
380+
* Therefore, set copyForwardStats->_edenEvacuateRegionCount (instead of getCurrentEdenSizeInRegions(env)) to
381+
* edenCountBeforeCollect.
382+
*/
383+
uintptr_t edenCountBeforeCollect = copyForwardStats->_edenEvacuateRegionCount;
376384

377385
Trc_MM_SchedulingDelegate_partialGarbageCollectCompleted_stats(env->getLanguageVMThread(),
378386
copyForwardStats->_edenEvacuateRegionCount,
@@ -393,11 +401,22 @@ MM_SchedulingDelegate::partialGarbageCollectCompleted(MM_EnvironmentVLHGC *env,
393401
Assert_MM_true( (0 == copyForwardStats->_scanBytesEden) || copyForwardStats->_aborted || (0 != copyForwardStats->_nonEvacuateRegionCount));
394402
Assert_MM_true( (0 == copyForwardStats->_scanBytesNonEden) || copyForwardStats->_aborted || (0 != copyForwardStats->_nonEvacuateRegionCount));
395403
edenSurvivorCount += (copyForwardStats->_scanBytesEden + regionSize - 1) / regionSize;
404+
Assert_MM_true(copyForwardStats->_edenSurvivorRegionCount <= edenCountBeforeCollect);
405+
if (edenSurvivorCount > edenCountBeforeCollect) {
406+
/* when abort is in progress, every Eden object scanned in abort-recovery contributes to _scannedBytes.
407+
* But an object that was partially copied before abort (the copy succeeded,
408+
* the forwarding pointer was installed) is also counted again during the abort scan of the source region.
409+
* So _scanBytesEden might be double-counted, then cause edenSurvivorCount is bigger than edenCountBeforeCollect.
410+
* for this case, set edenSurvivorCount = edenCountBeforeCollect.
411+
*/
412+
edenSurvivorCount = edenCountBeforeCollect;
413+
}
396414
nonEdenSurvivorCount += (copyForwardStats->_scanBytesNonEden + regionSize - 1) / regionSize;
397415

398416
/* Eden count could be 0 in special case, after compaction if there is still no free region for scheduling eden(eden count = 0),
399417
will skip update Survival Rate */
400418
if (0 != edenCountBeforeCollect) {
419+
/* SurvivalRate should never be over 1.0 */
401420
double thisSurvivalRate = (double)edenSurvivorCount / (double)edenCountBeforeCollect;
402421
updateSurvivalRatesAfterCopyForward(thisSurvivalRate, nonEdenSurvivorCount);
403422
}

0 commit comments

Comments
 (0)