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.

Sign up for GitHub

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

Jump to bottom

Fixup continuation lists for concurrent scavenge backout case #19562

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Fixup continuation lists for concurrent scavenge backout case #19562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions runtime/gc_glue_java/MarkingSchemeRootClearer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ MM_MarkingSchemeRootClearer::scanContinuationObjects(MM_EnvironmentBase *env)
/* allow the marking scheme to handle this */
reportScanningStarted(RootScannerEntity_ContinuationObjects);
GC_Environment *gcEnv = env->getGCEnvironment();
bool const compressed = _extensions->compressObjectReferences();

MM_HeapRegionDescriptorStandard *region = NULL;
GC_HeapRegionIteratorStandard regionIterator(_extensions->heap->getHeapRegionManager());
Expand All @@ -329,6 +330,27 @@ MM_MarkingSchemeRootClearer::scanContinuationObjects(MM_EnvironmentBase *env)
while (NULL != object) {
gcEnv->_markJavaStats._continuationCandidates += 1;
omrobjectptr_t next = _extensions->accessBarrier->getContinuationLink(object);
#if defined(OMR_GC_CONCURRENT_SCAVENGER)
{
/**
* In case of CS backout make sure the list contains the new version of the objects or if there is only one version,
* due to being self-forwarded, restore the self-forwarded bit.
*/
MM_ForwardedHeader forwardHeader(object, compressed);
omrobjectptr_t forwardPtr = forwardHeader.getNonStrictForwardedObject();

if (NULL != forwardPtr) {
Assert_MM_true(_extensions->isConcurrentScavengerEnabled() && _extensions->isScavengerBackOutFlagRaised());
Assert_MM_false(_markingScheme->isMarked(object));
if (forwardHeader.isSelfForwardedPointer()) {
forwardHeader.restoreSelfForwardedPointer();
} else {
object = forwardPtr;
}
}
}
#endif /* OMR_GC_CONCURRENT_SCAVENGER */

if (_markingScheme->isMarked(object) && !VM_ContinuationHelpers::isFinished(*VM_ContinuationHelpers::getContinuationStateAddress((J9VMThread *)env->getLanguageVMThread() , object))) {
/* object was already marked. */
gcEnv->_continuationObjectBuffer->add(env, object);
Expand Down
31 changes: 14 additions & 17 deletions runtime/gc_glue_java/ScavengerBackOutScanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,22 @@ MM_ScavengerBackOutScanner::scanAllSlots(MM_EnvironmentBase *env)
/* Walk roots fixing up pointers through reverse forwarding information */
MM_RootScanner::scanAllSlots(env);

if (!_extensions->isConcurrentScavengerEnabled()) {
/* Back out Ownable Synchronizer and Continuation Processing */
MM_HeapRegionDescriptorStandard *region = NULL;
GC_HeapRegionIteratorStandard regionIterator(_extensions->heapRegionManager);
while (NULL != (region = regionIterator.nextRegion())) {
MM_HeapRegionDescriptorStandardExtension *regionExtension = MM_ConfigurationDelegate::getHeapRegionDescriptorStandardExtension(env, region);
for (uintptr_t i = 0; i < regionExtension->_maxListIndex; i++) {
/**
* For Ownable Synchronizers lists, back out all of regions (includes tenure region, which is backed up during startProcessing).
* Tenure list might have gotten new elements during main Scavenge phase and it is important to restore the list, since it will be iterated later during Marking Clearable phase.
*/
regionExtension->_ownableSynchronizerObjectLists[i].backoutList();
if ((MEMORY_TYPE_NEW == (region->getTypeFlags() & MEMORY_TYPE_NEW))) {
/**
* For Continuation lists, Consistent with startProcessing that was earlier (in this GC cycle) called only on NEW regions.
* Tenure list cannot get new elements, since they are added only during Scavenge Clearable phase that cannot be reached if there was a Scavenge abort.
*/
/* Back out Ownable Synchronizer and Continuation Processing */
MM_HeapRegionDescriptorStandard *region = NULL;
GC_HeapRegionIteratorStandard regionIterator(_extensions->heapRegionManager);

while (NULL != (region = regionIterator.nextRegion())) {
MM_HeapRegionDescriptorStandardExtension *regionExtension = MM_ConfigurationDelegate::getHeapRegionDescriptorStandardExtension(env, region);
for (uintptr_t i = 0; i < regionExtension->_maxListIndex; i++) {
if (_extensions->isConcurrentScavengerEnabled()) {
if (_scavenger->isObjectInEvacuateMemory((omrobjectptr_t )region->getLowAddress())) {
/* for concurrent scavenger case, only backout lists in Evacuate region. */
regionExtension->_continuationObjectLists[i].backoutList();
}
} else {
/* Back out all of regions (includes tenure region, which is backed up during startProcessing). */
regionExtension->_ownableSynchronizerObjectLists[i].backoutList();
regionExtension->_continuationObjectLists[i].backoutList();
Copy link
Contributor

@amicic amicic May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

i think this would be easier to read if

  1. CS check is repeated only once, at the highest possible point (still within while loop), while admittedly at the cost of repeating backout call for evac region
while 
    if CS
       if evac region then backout
    else (if not CS)
       same as we used to do before the changes
  1. do 'if evac region' based on lowAddress, rather than the first element of the list

}
}
}
Expand Down
22 changes: 12 additions & 10 deletions runtime/gc_glue_java/ScavengerRootScanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@
void
MM_ScavengerRootScanner::startContinuationProcessing(MM_EnvironmentBase *env)
{
if(J9MODRON_HANDLE_NEXT_WORK_UNIT(env)) {
if (J9MODRON_HANDLE_NEXT_WORK_UNIT(env)) {
_scavengerDelegate->setShouldScavengeContinuationObjects(false);
_scavengerDelegate->setShouldIterateContinuationObjects(false);

MM_HeapRegionDescriptorStandard *region = NULL;
GC_HeapRegionIteratorStandard regionIterator(env->getExtensions()->getHeap()->getHeapRegionManager());
while(NULL != (region = regionIterator.nextRegion())) {
while (NULL != (region = regionIterator.nextRegion())) {
MM_HeapRegionDescriptorStandardExtension *regionExtension = MM_ConfigurationDelegate::getHeapRegionDescriptorStandardExtension(env, region);
for (UDATA i = 0; i < regionExtension->_maxListIndex; i++) {
for (uintptr_t i = 0; i < regionExtension->_maxListIndex; i++) {
MM_ContinuationObjectList *list = &regionExtension->_continuationObjectLists[i];
if (!list->isEmpty()) {
_scavengerDelegate->setShouldIterateContinuationObjects(true);
Expand All @@ -66,6 +66,8 @@ MM_ScavengerRootScanner::startContinuationProcessing(MM_EnvironmentBase *env)
if (!list->wasEmpty()) {
_scavengerDelegate->setShouldScavengeContinuationObjects(true);
}
} else {
list->backupList();
Copy link
Contributor

Choose a reason for hiding this comment

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

While touching the file, fix a few coding standard problems like

  • space between if/while and '('
  • UDATA -> uintptr_t
  • * next to variable, rather next to the type

}
}
}
Expand All @@ -76,15 +78,15 @@ MM_ScavengerRootScanner::startContinuationProcessing(MM_EnvironmentBase *env)
void
MM_ScavengerRootScanner::startUnfinalizedProcessing(MM_EnvironmentBase *env)
{
if(J9MODRON_HANDLE_NEXT_WORK_UNIT(env)) {
if (J9MODRON_HANDLE_NEXT_WORK_UNIT(env)) {
_scavengerDelegate->setShouldScavengeUnfinalizedObjects(false);

MM_HeapRegionDescriptorStandard *region = NULL;
GC_HeapRegionIteratorStandard regionIterator(env->getExtensions()->getHeap()->getHeapRegionManager());
while(NULL != (region = regionIterator.nextRegion())) {
while (NULL != (region = regionIterator.nextRegion())) {
if ((MEMORY_TYPE_NEW == (region->getTypeFlags() & MEMORY_TYPE_NEW))) {
MM_HeapRegionDescriptorStandardExtension *regionExtension = MM_ConfigurationDelegate::getHeapRegionDescriptorStandardExtension(env, region);
for (UDATA i = 0; i < regionExtension->_maxListIndex; i++) {
for (uintptr_t i = 0; i < regionExtension->_maxListIndex; i++) {
MM_UnfinalizedObjectList *list = &regionExtension->_unfinalizedObjectLists[i];
list->startUnfinalizedProcessing();
if (!list->wasEmpty()) {
Expand All @@ -99,7 +101,7 @@ MM_ScavengerRootScanner::startUnfinalizedProcessing(MM_EnvironmentBase *env)
void
MM_ScavengerRootScanner::scavengeFinalizableObjects(MM_EnvironmentStandard *env)
{
GC_FinalizeListManager * const finalizeListManager = _extensions->finalizeListManager;
GC_FinalizeListManager *const finalizeListManager = _extensions->finalizeListManager;
bool const compressed = _extensions->compressObjectReferences();

/* this code must be run single-threaded and we should only be here if work is actually required */
Expand All @@ -113,7 +115,7 @@ MM_ScavengerRootScanner::scavengeFinalizableObjects(MM_EnvironmentStandard *env)
omrobjectptr_t systemObject = finalizeListManager->resetSystemFinalizableObjects();
while (NULL != systemObject) {
omrobjectptr_t next = NULL;
if(_scavenger->isObjectInEvacuateMemory(systemObject)) {
if (_scavenger->isObjectInEvacuateMemory(systemObject)) {
MM_ForwardedHeader forwardedHeader(systemObject, compressed);
if (!forwardedHeader.isForwardedPointer()) {
next = _extensions->accessBarrier->getFinalizeLink(systemObject);
Expand Down Expand Up @@ -145,7 +147,7 @@ MM_ScavengerRootScanner::scavengeFinalizableObjects(MM_EnvironmentStandard *env)
omrobjectptr_t defaultObject = finalizeListManager->resetDefaultFinalizableObjects();
while (NULL != defaultObject) {
omrobjectptr_t next = NULL;
if(_scavenger->isObjectInEvacuateMemory(defaultObject)) {
if (_scavenger->isObjectInEvacuateMemory(defaultObject)) {
MM_ForwardedHeader forwardedHeader(defaultObject, compressed);
if (!forwardedHeader.isForwardedPointer()) {
next = _extensions->accessBarrier->getFinalizeLink(defaultObject);
Expand Down Expand Up @@ -177,7 +179,7 @@ MM_ScavengerRootScanner::scavengeFinalizableObjects(MM_EnvironmentStandard *env)
omrobjectptr_t referenceObject = finalizeListManager->resetReferenceObjects();
while (NULL != referenceObject) {
omrobjectptr_t next = NULL;
if(_scavenger->isObjectInEvacuateMemory(referenceObject)) {
if (_scavenger->isObjectInEvacuateMemory(referenceObject)) {
MM_ForwardedHeader forwardedHeader(referenceObject, compressed);
if (!forwardedHeader.isForwardedPointer()) {
next = _extensions->accessBarrier->getReferenceLink(referenceObject);
Expand Down