Merge pull request #4716 from RSalman/cgc-fix

Heap Resize: Concurrent KO/Uninitialized Mark Map Bug fix

gc/base/Collector.hpp

@@ -30,6 +30,7 @@
 
 #include "BaseVirtual.hpp"
 #include "EnvironmentBase.hpp"
+#include "ModronAssertions.h"
 
 class MM_AllocateDescription;
 class MM_AllocationContext;
@@ -194,7 +195,18 @@ class MM_Collector : public MM_BaseVirtual
 	 * moved from one subspace to another.
 	 * @param env[in] The thread which performed the change in heap geometry 
 	 */
-	virtual void heapReconfigured(MM_EnvironmentBase* env) = 0;
+	virtual void heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress)
+	{
+		heapReconfigured(env); /* Required temporarily to not break dependency with heapReconfigure API changes */
+	};
+
+	/* Required temporarily to not break dependency with heapReconfigure API changes */
+	virtual void heapReconfigured(MM_EnvironmentBase *env) 
+	{
+		/* ALl collectors except Scavenger & Realtime should have their own implementation */
+		MM_GCPolicy gc_policy = env->getExtensions()->configurationOptions._gcPolicy;
+		Assert_MM_true(gc_policy == OMR_GC_POLICY_METRONOME || gc_policy == OMR_GC_POLICY_GENCON);
+	};
 
 	/**
 	 * Post collection broadcast event, indicating that the collection has been completed.

gc/base/Heap.cpp

@@ -433,12 +433,12 @@ MM_Heap::heapRemoveRange(MM_EnvironmentBase* env, MM_MemorySubSpace* subspace, u
  * The heap has had its memory shuffled between memory subspaces and/or memory pools.
  */
 void
-MM_Heap::heapReconfigured(MM_EnvironmentBase* env)
+MM_Heap::heapReconfigured(MM_EnvironmentBase* env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress)
 {
 	MM_GlobalCollector* globalCollector = env->getExtensions()->getGlobalCollector();
 
 	if (NULL != globalCollector) {
-		globalCollector->heapReconfigured(env);
+		globalCollector->heapReconfigured(env, reason, subspace, lowAddress, highAddress);
 	}
 }
 

gc/base/Heap.hpp

@@ -148,7 +148,7 @@ class MM_Heap : public MM_BaseVirtual
 	 * moved from one subspace to another.
 	 * @param env[in] The thread which performed the change in heap geometry 
 	 */
-	virtual void heapReconfigured(MM_EnvironmentBase *env);
+	virtual void heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress);
 
 	virtual bool objectIsInGap(void *object);
 

gc/base/MemorySpace.cpp

@@ -515,9 +515,9 @@ MM_MemorySpace::heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subs
  * 
  */
 void
-MM_MemorySpace::heapReconfigured(MM_EnvironmentBase *env)
+MM_MemorySpace::heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress)
 {
-	_heap->heapReconfigured(env);
+	_heap->heapReconfigured(env, reason, subspace, lowAddress, highAddress);
 }
 
 /**

gc/base/MemorySpace.hpp

@@ -181,7 +181,7 @@ class MM_MemorySpace : public MM_BaseVirtual
 	 * moved from one subspace to another.
 	 * @param env[in] The thread which performed the change in heap geometry 
 	 */
-	void heapReconfigured(MM_EnvironmentBase *env);
+	void heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress);
 
 	static MM_MemorySpace *getMemorySpace(void *memorySpace) { return (MM_MemorySpace *)memorySpace; }
 

gc/base/MemorySubSpace.cpp

@@ -1380,16 +1380,16 @@ MM_MemorySubSpace::heapRemoveRange(MM_EnvironmentBase* env, MM_MemorySubSpace* s
  * 
  */
 void
-MM_MemorySubSpace::heapReconfigured(MM_EnvironmentBase* env)
+MM_MemorySubSpace::heapReconfigured(MM_EnvironmentBase* env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress)
 {
 	if (!_usesGlobalCollector && (NULL != _collector)) {
-		_collector->heapReconfigured(env);
+		_collector->heapReconfigured(env, reason, subspace, lowAddress, highAddress);
 	}
 
 	if (_parent) {
-		_parent->heapReconfigured(env);
+		_parent->heapReconfigured(env, reason, subspace, lowAddress, highAddress);
 	} else if (_memorySpace) {
-		_memorySpace->heapReconfigured(env);
+		_memorySpace->heapReconfigured(env, reason, subspace, lowAddress, highAddress);
 	}
 }
 

gc/base/MemorySubSpace.hpp

@@ -404,7 +404,7 @@ friend class GC_MemorySubSpaceRegionIterator;
 	 * moved from one subspace to another.
 	 * @param env[in] The thread which performed the change in heap geometry 
 	 */
-	virtual void heapReconfigured(MM_EnvironmentBase *env);
+	virtual void heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason = HEAP_RECONFIG_NONE, MM_MemorySubSpace *subspace = NULL, void *lowAddress = NULL, void *highAddress = NULL);
 
 	virtual void *findFreeEntryEndingAtAddr(MM_EnvironmentBase *env, void *addr);
 	virtual void *findFreeEntryTopStartingAtAddr(MM_EnvironmentBase *env, void *addr);

gc/base/PhysicalSubArenaVirtualMemoryFlat.cpp

@@ -94,7 +94,7 @@ MM_PhysicalSubArenaVirtualMemoryFlat::tearDown(MM_EnvironmentBase *env)
 	}
 	if (NULL != _subSpace) {
 		_subSpace->heapRemoveRange(env, _subSpace, ((uintptr_t)_highAddress) - ((uintptr_t)_lowAddress), _lowAddress, _highAddress, lowValidAddress, highValidAddress);
-		_subSpace->heapReconfigured(env);
+		_subSpace->heapReconfigured(env, HEAP_RECONFIG_CONTRACT);
 	}
 	MM_PhysicalSubArenaVirtualMemory::tearDown(env);
 }
@@ -113,9 +113,17 @@ MM_PhysicalSubArenaVirtualMemoryFlat::inflate(MM_EnvironmentBase *env)
 		if(NULL != _region) {
 			Assert_MM_true((_lowAddress == _region->getLowAddress()) && (_highAddress == _region->getHighAddress()));
 			/* Inflation successful - inform the owning memorySubSpace */
-			//TODO: Like the semi space arena, this should dispatch directory to the child subspace
-			result = _subSpace->expanded(env, this, _region->getSize(), _region->getLowAddress(), _region->getHighAddress(), false);
-			_subSpace->heapReconfigured(env);
+
+			void *lowAddress = _region->getLowAddress();
+			void *highAddress = _region->getHighAddress();
+
+			MM_MemorySubSpace *genericSubSpace = ((MM_MemorySubSpaceFlat *)_subSpace)->getChildSubSpace();
+			result = genericSubSpace->expanded(env, this, _region->getSize(), lowAddress, highAddress, false);
+			if (result) {
+				_subSpace->heapReconfigured(env, HEAP_RECONFIG_EXPAND, genericSubSpace, lowAddress, highAddress);
+			} else {
+				_subSpace->heapReconfigured(env, HEAP_RECONFIG_EXPAND);
+			}
 		}
 	}
 	return result;
@@ -211,11 +219,12 @@ MM_PhysicalSubArenaVirtualMemoryFlat::expandNoCheck(MM_EnvironmentBase *env, uin
 		getHeapRegionManager()->resizeAuxillaryRegion(env, _region, _lowAddress, _highAddress);
 		Assert_MM_true(NULL != _region);
 
-		if(result){
+		if(result) {
 			genericSubSpace->addExistingMemory(env, this, expandSize, lowExpandAddress, highExpandAddress, true);
+			_subSpace->heapReconfigured(env, HEAP_RECONFIG_EXPAND, genericSubSpace, lowExpandAddress, highExpandAddress);
+		} else {
+			_subSpace->heapReconfigured(env, HEAP_RECONFIG_EXPAND);
 		}
-
-		_subSpace->heapReconfigured(env);
 	}
 
 	Assert_MM_true(_lowAddress == _region->getLowAddress());
@@ -305,10 +314,9 @@ MM_PhysicalSubArenaVirtualMemoryFlat::contract(MM_EnvironmentBase *env, uintptr_
 	_highAddress = (void *)contractBase;
 	getHeapRegionManager()->resizeAuxillaryRegion(env, _region, _lowAddress, _highAddress);
 	Assert_MM_true(NULL != _region);
-
 	/* Broadcast that heap has been removed */
 	genericSubSpace->heapRemoveRange(env, _subSpace, contractSize, (void *)contractBase, (void *)contractTop, lowValidAddress, highValidAddress);
-	genericSubSpace->heapReconfigured(env);
+	genericSubSpace->heapReconfigured(env, HEAP_RECONFIG_CONTRACT);
 
 	/* Execute any pending counter balances to the contract that have been enqueued */
 	_subSpace->triggerEnqueuedCounterBalancing(env);

gc/base/segregated/MemorySubSpaceSegregated.cpp

@@ -310,9 +310,9 @@ MM_MemorySubSpaceSegregated::heapRemoveRange(MM_EnvironmentBase *env, MM_MemoryS
 }
 
 void
-MM_MemorySubSpaceSegregated::heapReconfigured(MM_EnvironmentBase *env)
+MM_MemorySubSpaceSegregated::heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress)
 {
-	MM_MemorySubSpaceUniSpace::heapReconfigured(env);
+	MM_MemorySubSpaceUniSpace::heapReconfigured(env, reason, subspace, lowAddress, highAddress);
 	if (_regionExpansionBase != _regionExpansionTop) {
 		expandRegionPool();
 	}

gc/base/segregated/MemorySubSpaceSegregated.hpp

@@ -114,7 +114,7 @@ class MM_MemorySubSpaceSegregated : public MM_MemorySubSpaceUniSpace
 
 	virtual bool heapAddRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspace, uintptr_t size, void *lowAddress, void *highAddress);
 	virtual bool heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspace, uintptr_t size, void *lowAddress, void *highAddress, void *lowValidAddress, void *highValidAddress);
-	virtual void heapReconfigured(MM_EnvironmentBase *env);
+	virtual void heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace = NULL, void *lowAddress = NULL, void *highAddress = NULL);
 
 	virtual MM_MemoryPool *getMemoryPool();
 

gc/base/segregated/SegregatedGC.cpp

@@ -126,11 +126,6 @@ MM_SegregatedGC::heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpace *sub
 	return _markingScheme->heapRemoveRange(env, subspace, size, lowAddress, highAddress, lowValidAddress, highValidAddress);
 }
 
-void MM_SegregatedGC::heapReconfigured(MM_EnvironmentBase* env)
-{
-	/* OMRTODO implement proper heap resizing in segregated heaps */
-}
-
 bool
 MM_SegregatedGC::collectorStartup(MM_GCExtensionsBase* extensions)
 {

gc/base/segregated/SegregatedGC.hpp

@@ -96,7 +96,6 @@ class MM_SegregatedGC : public MM_GlobalCollector
 
 	virtual bool heapAddRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspace, uintptr_t size, void *lowAddress, void *highAddress);
 	virtual bool heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspace,uintptr_t size, void *lowAddress, void *highAddress, void *lowValidAddress, void *highValidAddress);
-	virtual void heapReconfigured(MM_EnvironmentBase* env);
 
 	virtual bool isMarked(void *objectPtr) { return _markingScheme->isMarked(static_cast<omrobjectptr_t>(objectPtr)); }
 

gc/base/standard/ConcurrentCardTable.cpp

@@ -408,18 +408,6 @@ MM_ConcurrentCardTable::heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpa
 	return result;
 }
 
-/**
- * Re-size all structures which are dependent on the current size of the heap.
- * No new memory has been added to a heap reconfiguration.  This call typically is the result
- * of having segment range changes (memory redistributed between segments) or the meaning of
- * memory changed.
- *
- */
-void
-MM_ConcurrentCardTable::heapReconfigured(MM_EnvironmentBase *env)
-{
-}
-
 /**
  *					Object creation and destruction
  * 					===============================

gc/base/standard/ConcurrentCardTable.hpp

@@ -320,12 +320,6 @@ class MM_ConcurrentCardTable : public MM_CardTable
 	 * @return true if contraction is successful
 	 */
 	bool heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspace, uintptr_t size, void *lowAddress, void *highAddress, void *lowValidAddress, void *highValidAddress);
-	/**
-	 * Called when the heap geometry has been reconfigured but no memory was added or removed from the heap (happens during tilt).
-	 * @param[in] env The thread which caused the heap geometry change (typically the master GC thread)
-	 * @note This implementation does nothing.
-	 */
-	void heapReconfigured(MM_EnvironmentBase *env);
 
 	/**
 	 * @return A pointer to the mutable card table stats structure

gc/base/standard/ConcurrentGC.cpp

@@ -3268,23 +3268,6 @@ MM_ConcurrentGC::heapAddRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspa
 	/* Expand any superclass structures including mark bits*/
 	bool result = MM_ParallelGlobalGC::heapAddRange(env, subspace, size, lowAddress, highAddress);
 
-	if (result) {
-		/* If we are within a concurrent cycle we need to initialize the mark bits
-		 * for new region of heap now
-		 */
-		if (CONCURRENT_OFF < _stats.getExecutionMode()) {
-			/* If subspace is concurrently collectible then clear bits otherwise
-			 * set the bits on to stop tracing INTO this area during concurrent
-			 * mark cycle.
-			 */
-			if (subspace->isConcurrentCollectable()) {
-				_markingScheme->setMarkBitsInRange(env, lowAddress, highAddress, true);
-			} else {
-				_markingScheme->setMarkBitsInRange(env, lowAddress, highAddress, false);
-			}
-		}
-	}
-
 	_heapAlloc = _extensions->heap->getHeapTop();
 
 	Trc_MM_ConcurrentGC_heapAddRange_Exit(env->getLanguageVMThread());
@@ -3330,34 +3313,63 @@ MM_ConcurrentGC::heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpace *sub
  * @see MM_ParallelGlobalGC::heapReconfigured()
  */
 void
-MM_ConcurrentGC::heapReconfigured(MM_EnvironmentBase *env)
+MM_ConcurrentGC::heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress)
 {
-	/* If called outside a global collection for a heap expand/contract..
+	Assert_MM_true(reason != HEAP_RECONFIG_NONE);
+
+	/* _rebuildInitWorkForAdd/_rebuildInitWorkForRemove are not sufficient for determining on how to proceed with headReconfigured
+	 * We end up here in the following scenarios:
+	 *  1) During heap expand - after heapAddRange
+	 *  2) During heap contact - after heapRemoveRange
+	 *  3) After Scavenger Tilt (no resize)
+	 *
+	 *  It is necessary that _rebuildInitWorkForAdd is set when we're here during an expand (after heapAddRange), or
+	 *  _rebuildInitWorkForRemove in the case of contract. However, it is not a sufficient check to ensure the reason we're
+	 *  here. For instance, when Concurent is on, _rebuildInitWorkForAdd will be set but not cleared.
+	 *  As a result, we can have multiple calls of expands interleaved with contracts, resulting in both flags being set.
+	 *  Similarly, we can end up here after scavenger tilt with any of the flags set.
 	 */
-	if (!_stwCollectionInProgress && (_rebuildInitWorkForAdd || _rebuildInitWorkForRemove)) {
-		/* ... and a concurrent cycle has not yet started then we
-		 *  tune to heap here to reflect new heap size
-		 *  Note: CMVC 153167 : Under gencon, there is a timing hole where
-		 *  if we are in the middle of initializing the heap ranges while a
-		 *  scavenge occurs, and if the scavenge causes the heap to contract,
-		 *  we will try to memset ranges that are now contracted (decommitted memory)
-		 *  when we resume the init work.
+
+	if ((lowAddress != NULL) && (highAddress != NULL)) {
+		Assert_MM_true(_rebuildInitWorkForAdd && (reason == HEAP_RECONFIG_EXPAND));
+		/* If we are within a concurrent cycle we need to initialize the mark bits
+		 * for new region of heap now
 		 */
-		if (_stats.getExecutionMode() < CONCURRENT_INIT_COMPLETE) {
-			tuneToHeap(env);
-		} else {
-			/* Heap expand/contract is during a concurrent cycle..we need to adjust the trace target so
-			 * that the trace rate is adjusted correctly on  subsequent allocates.
+		if (CONCURRENT_OFF < _stats.getExecutionMode()) {
+			/* If subspace is concurrently collectible then clear bits otherwise
+			 * set the bits on to stop tracing INTO this area during concurrent
+			 * mark cycle.
 			 */
-			adjustTraceTarget();
+			_markingScheme->setMarkBitsInRange(env, lowAddress, highAddress, subspace->isConcurrentCollectable());
 		}
 	}
 
-	/* Expand any superclass structures */
-	MM_ParallelGlobalGC::heapReconfigured(env);
+	/* If called outside a global collection for a heap expand/contract.. */
+	if((reason == HEAP_RECONFIG_CONTRACT) || (reason == HEAP_RECONFIG_EXPAND)) {
+		Assert_MM_true(_rebuildInitWorkForAdd || _rebuildInitWorkForRemove);
+		if (!_stwCollectionInProgress) {
+			/* ... and a concurrent cycle has not yet started then we
+			 *  tune to heap here to reflect new heap size
+			 *  Note: CMVC 153167 : Under gencon, there is a timing hole where
+			 *  if we are in the middle of initializing the heap ranges while a
+			 *  scavenge occurs, and if the scavenge causes the heap to contract,
+			 *  we will try to memset ranges that are now contracted (decommitted memory)
+			 *  when we resume the init work.
+			 */
+			if (_stats.getExecutionMode() < CONCURRENT_INIT_COMPLETE) {
+				tuneToHeap(env);
+			} else {
+				/* Heap expand/contract is during a concurrent cycle..we need to adjust the trace target so
+				 * that the trace rate is adjusted correctly on  subsequent allocates.
+				 */
+				adjustTraceTarget();
+			}
+		}
+	}
 
-	/* ...and then expand the card table */
-	((MM_ConcurrentCardTable *)_cardTable)->heapReconfigured(env);
+
+	/* Expand any superclass structures */
+	MM_ParallelGlobalGC::heapReconfigured(env, reason, subspace, lowAddress, highAddress);
 }
 
 /**

gc/base/standard/ConcurrentGC.hpp

@@ -426,7 +426,7 @@ class MM_ConcurrentGC : public MM_ParallelGlobalGC
 
 	virtual bool heapAddRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspace, uintptr_t size, void *lowAddress, void *highAddress);
 	virtual bool heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspace, uintptr_t size, void *lowAddress, void *highAddress, void *lowValidAddress, void *highValidAddress);
-	virtual void heapReconfigured(MM_EnvironmentBase *env);
+	virtual void heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress);
 
 	void finalCleanCards(MM_EnvironmentBase *env);
 

gc/base/standard/ParallelGlobalGC.cpp

@@ -1345,10 +1345,9 @@ MM_ParallelGlobalGC::heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpace
  * @see MM_GlobalCollector::heapReconfigured()
  */
 void
-MM_ParallelGlobalGC::heapReconfigured(MM_EnvironmentBase *env)
+MM_ParallelGlobalGC::heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress)
 {
 	_sweepScheme->heapReconfigured(env);
-
 }
 
 bool

gc/base/standard/ParallelGlobalGC.hpp

@@ -289,7 +289,7 @@ class MM_ParallelGlobalGC : public MM_GlobalCollector
 
 	virtual bool heapAddRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspace, uintptr_t size, void *lowAddress, void *highAddress);
 	virtual bool heapRemoveRange(MM_EnvironmentBase *env, MM_MemorySubSpace *subspace, uintptr_t size, void *lowAddress, void *highAddress, void *lowValidAddress, void *highValidAddress);
-	virtual void heapReconfigured(MM_EnvironmentBase *env);
+	virtual void heapReconfigured(MM_EnvironmentBase *env, HeapReconfigReason reason, MM_MemorySubSpace *subspace, void *lowAddress, void *highAddress);
 
 	virtual	uint32_t getGCTimePercentage(MM_EnvironmentBase *env);