From caed11ef7dfefddc939e277b43dd3092539a87a8 Mon Sep 17 00:00:00 2001 From: Lin Hu Date: Tue, 23 Aug 2022 10:12:08 -0400 Subject: [PATCH] Uncomment the code have 2 dependency with downstream projects Remove the temporary comments after the dependent downstream project has merged. Update Method fixupForwardedSlot return true or false for the condition to update heap object reference slot(we won't need to update the reference slot if the reference is not fixed up) in order to avoid potential reference slot overwriting(concurrent marking) during Concurrent scavenger back out case. Share method MM_Scavenger.shouldRememberSlot, remove duplicated logic for checking if should remember the slot. Signed-off-by: Lin Hu --- example/glue/ScavengerDelegate.cpp | 12 ------------ example/glue/ScavengerDelegate.hpp | 2 -- gc/base/MarkingScheme.cpp | 4 +++- gc/base/MarkingScheme.hpp | 7 ++++--- gc/base/standard/HeapWalker.cpp | 18 ++++++------------ gc/base/standard/HeapWalker.hpp | 9 +++------ gc/base/standard/Scavenger.cpp | 14 +++----------- 7 files changed, 19 insertions(+), 47 deletions(-) diff --git a/example/glue/ScavengerDelegate.cpp b/example/glue/ScavengerDelegate.cpp index 0c6dffaa442..3cdbd550c10 100644 --- a/example/glue/ScavengerDelegate.cpp +++ b/example/glue/ScavengerDelegate.cpp @@ -110,18 +110,6 @@ MM_ScavengerDelegate::getObjectScanner(MM_EnvironmentStandard *env, omrobjectptr return objectScanner; } -/* temporary API for backward dependency, will be removed after related changes are merged. */ -GC_ObjectScanner * -MM_ScavengerDelegate::getObjectScanner(MM_EnvironmentStandard *env, omrobjectptr_t objectPtr, void *allocSpace, uintptr_t flags) -{ -#if defined(OMR_GC_MODRON_SCAVENGER_STRICT) - Assert_MM_true((GC_ObjectScanner::scanHeap == flags) ^ (GC_ObjectScanner::scanRoots == flags)); -#endif /* defined(OMR_GC_MODRON_SCAVENGER_STRICT) */ - GC_ObjectScanner *objectScanner = NULL; - objectScanner = GC_MixedObjectScanner::newInstance(env, objectPtr, allocSpace, flags); - return objectScanner; -} - void MM_ScavengerDelegate::flushReferenceObjects(MM_EnvironmentStandard *env) { diff --git a/example/glue/ScavengerDelegate.hpp b/example/glue/ScavengerDelegate.hpp index 072a7cd8ac6..3595ff36ef6 100644 --- a/example/glue/ScavengerDelegate.hpp +++ b/example/glue/ScavengerDelegate.hpp @@ -138,8 +138,6 @@ class MM_ScavengerDelegate : public MM_BaseVirtual { */ GC_ObjectScanner *getObjectScanner(MM_EnvironmentStandard *env, omrobjectptr_t objectPtr, void *allocSpace, uintptr_t flags, MM_ScavengeScanReason reason, bool *shouldRemember); - /* temporary API for backward dependency, will be removed after related changes are merged. */ - GC_ObjectScanner *getObjectScanner(MM_EnvironmentStandard *env, omrobjectptr_t objectPtr, void *allocSpace, uintptr_t flags); /** * Scavenger calls this method when required to force GC threads to flush any locally-held references into * associated global buffers. diff --git a/gc/base/MarkingScheme.cpp b/gc/base/MarkingScheme.cpp index 94566a88884..394d0a5701a 100644 --- a/gc/base/MarkingScheme.cpp +++ b/gc/base/MarkingScheme.cpp @@ -410,7 +410,7 @@ MM_MarkingScheme::createWorkPackets(MM_EnvironmentBase *env) return workPackets; } -void +bool MM_MarkingScheme::fixupForwardedSlot(omrobjectptr_t *slotPtr) { #if defined(OMR_GC_CONCURRENT_SCAVENGER) bool const compressed = _extensions->compressObjectReferences(); @@ -423,10 +423,12 @@ MM_MarkingScheme::fixupForwardedSlot(omrobjectptr_t *slotPtr) { forwardHeader.restoreSelfForwardedPointer(); } else { *slotPtr = forwardPtr; + return true; } } } #endif /* OMR_GC_CONCURRENT_SCAVENGER */ + return false; } uintptr_t diff --git a/gc/base/MarkingScheme.hpp b/gc/base/MarkingScheme.hpp index 3c2c67002db..68c31f32c4e 100644 --- a/gc/base/MarkingScheme.hpp +++ b/gc/base/MarkingScheme.hpp @@ -291,12 +291,13 @@ class MM_MarkingScheme : public MM_BaseVirtual MMINLINE void fixupForwardedSlot(GC_SlotObject *slotObject) { if (_extensions->isConcurrentScavengerEnabled() && _extensions->isScavengerBackOutFlagRaised()) { omrobjectptr_t slot = slotObject->readReferenceFromSlot(); - fixupForwardedSlot(&slot); - slotObject->writeReferenceToSlot(slot); + if (fixupForwardedSlot(&slot)) { + slotObject->writeReferenceToSlot(slot); + } } } - void fixupForwardedSlot(omrobjectptr_t *slotPtr); + bool fixupForwardedSlot(omrobjectptr_t *slotPtr); virtual uintptr_t setupIndexableScanner(MM_EnvironmentBase *env, omrobjectptr_t objectPtr, MM_MarkingSchemeScanReason reason, uintptr_t *sizeToDo, uintptr_t *sizeInElementsToDo, fomrobject_t **basePtr, uintptr_t *flags); /** diff --git a/gc/base/standard/HeapWalker.cpp b/gc/base/standard/HeapWalker.cpp index 475e4a42c0c..3dcda501b5f 100644 --- a/gc/base/standard/HeapWalker.cpp +++ b/gc/base/standard/HeapWalker.cpp @@ -71,9 +71,7 @@ heapWalkerObjectFieldSlotDo(OMR_VM *omrVM, omrobjectptr_t object, GC_SlotObject * walk through slots of mixed object and apply the user function. */ static void -// comment just for back dependency -//heapWalkerObjectSlotsDo(OMR_VMThread *omrVMThread, omrobjectptr_t object, MM_HeapWalkerSlotFunc oSlotIterator, void *localUserData, MM_HeapWalkerDelegate *delegate) -heapWalkerObjectSlotsDo(OMR_VMThread *omrVMThread, omrobjectptr_t object, MM_HeapWalkerSlotFunc oSlotIterator, void *localUserData) +heapWalkerObjectSlotsDo(OMR_VMThread *omrVMThread, omrobjectptr_t object, MM_HeapWalkerSlotFunc oSlotIterator, void *localUserData, MM_HeapWalkerDelegate *delegate) { OMR_VM *omrVM = omrVMThread->_vm; GC_ObjectIterator objectIterator(omrVM, object); @@ -82,8 +80,7 @@ heapWalkerObjectSlotsDo(OMR_VMThread *omrVMThread, omrobjectptr_t object, MM_Hea while ((slotObject = objectIterator.nextSlot()) != NULL) { heapWalkerObjectFieldSlotDo(omrVM, object, slotObject, oSlotIterator, localUserData); } -// comment just for back dependency -// delegate->objectSlotsDo(omrVMThread, object, oSlotIterator, localUserData); + delegate->objectSlotsDo(omrVMThread, object, oSlotIterator, localUserData); } void @@ -111,9 +108,7 @@ heapWalkerObjectSlotsDo(OMR_VMThread *omrVMThread, MM_HeapRegionDescriptor *regi (*oSlotIterator)(omrVM, &indirectObject, localUserData, 0); } -// comment just for back dependency -// heapWalkerObjectSlotsDo(omrVMThread, object, oSlotIterator, localUserData, slotObjectDoUserData->heapWalker->getHeapWalkerDelegate()); - heapWalkerObjectSlotsDo(omrVMThread, object, oSlotIterator, localUserData); + heapWalkerObjectSlotsDo(omrVMThread, object, oSlotIterator, localUserData, slotObjectDoUserData->heapWalker->getHeapWalkerDelegate()); } MM_HeapWalker * @@ -132,10 +127,9 @@ MM_HeapWalker::newInstance(MM_EnvironmentBase *env) bool MM_HeapWalker::initialize(MM_EnvironmentBase *env) { -// comment just for back dependency -// if (!_delegate.initialize(env, this)) { -// return false; -// } + if (!_delegate.initialize(env, this)) { + return false; + } return true; } diff --git a/gc/base/standard/HeapWalker.hpp b/gc/base/standard/HeapWalker.hpp index 4a72d0c6fd9..bfcc06b284c 100644 --- a/gc/base/standard/HeapWalker.hpp +++ b/gc/base/standard/HeapWalker.hpp @@ -28,8 +28,7 @@ #include "objectdescription.h" #include "BaseVirtual.hpp" -// comment just for back dependency -//#include "HeapWalkerDelegate.hpp" +#include "HeapWalkerDelegate.hpp" class MM_EnvironmentBase; class MM_Heap; @@ -44,8 +43,7 @@ class MM_HeapWalker : public MM_BaseVirtual { private: protected: -// comment just for back dependency -//MM_HeapWalkerDelegate _delegate; +MM_HeapWalkerDelegate _delegate; #if defined(OMR_GC_MODRON_SCAVENGER) void rememberedObjectSlotsDo(MM_EnvironmentBase *env, MM_HeapWalkerSlotFunc function, void *userData, uintptr_t walkFlags, bool parallel); #endif /* OMR_GC_MODRON_SCAVENGER */ @@ -59,8 +57,7 @@ class MM_HeapWalker : public MM_BaseVirtual virtual void kill(MM_EnvironmentBase *env); void heapWalkerSlotCallback(MM_EnvironmentBase *env, omrobjectptr_t *objectSlotPtr, MM_HeapWalkerSlotFunc function, void * userData); -// comment just for back dependency -// MM_HeapWalkerDelegate *getHeapWalkerDelegate() { return &_delegate; } + MM_HeapWalkerDelegate *getHeapWalkerDelegate() { return &_delegate; } /** * constructor of Heap Walker */ diff --git a/gc/base/standard/Scavenger.cpp b/gc/base/standard/Scavenger.cpp index bcc32b1063c..e4bbe536c5f 100644 --- a/gc/base/standard/Scavenger.cpp +++ b/gc/base/standard/Scavenger.cpp @@ -1872,9 +1872,7 @@ MM_Scavenger::copyHotField(MM_EnvironmentStandard *env, omrobjectptr_t destinati GC_ObjectScanner * MM_Scavenger::getObjectScanner(MM_EnvironmentStandard *env, omrobjectptr_t objectptr, void *objectScannerState, uintptr_t flags, MM_ScavengeScanReason reason, bool *shouldRemember) { -// for temporary backdependence, need to be updated after related changes are merged. -// return _delegate.getObjectScanner(env, objectptr, (void*) objectScannerState, flags, reason, shouldRemember); - return _delegate.getObjectScanner(env, objectptr, (void*) objectScannerState, flags); + return _delegate.getObjectScanner(env, objectptr, (void*) objectScannerState, flags, reason, shouldRemember); } uintptr_t @@ -2803,14 +2801,8 @@ MM_Scavenger::shouldRememberObject(MM_EnvironmentStandard *env, omrobjectptr_t o GC_SlotObject *slotPtr; while (NULL != (slotPtr = objectScanner->getNextSlot())) { omrobjectptr_t slotObjectPtr = slotPtr->readReferenceFromSlot(); - if (NULL != slotObjectPtr) { - if (isObjectInNewSpace(slotObjectPtr)) { - Assert_MM_true(!isObjectInEvacuateMemory(slotObjectPtr)); - return true; - } else if (IS_CONCURRENT_ENABLED && isBackOutFlagRaised() && isObjectInEvacuateMemory(slotObjectPtr)) { - /* Could happen if we aborted before completing RS scan */ - return true; - } + if (shouldRememberSlot(&slotObjectPtr)) { + return true; } } }