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 <[email protected]>
eclipse · Sep 6, 2022 · caed11e · caed11e
1 parent 16e09fb
commit caed11e
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 47 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
 			}
 		}
 	}