Fixes eclipse-openj9/openj9#8020

- MarkMap init consolidated

MarkMap init logic has been merged to resolve the timing hole outlined
in eclipse-openj9/openj9#8020. MarkMap init is
dependent on Concurrent State of the global collector, with the init
logic split between `heapAddRange` and `heapReconfigured`, any changes
in states between the two may leave expanded mark map uninited. Hence,
the decision to init (`setMarkBitsInRange`) the mark map has been moved
from `heapAddRange` to `heapReconfigured` which is where we determine if
an update to the init table is required
(`tuneToHeap`/`determineInitWork`). As a result, init is no longer
affected by the change in Concurrent state and we eliminate the timing
window. This guarantee that the MarkMap will be inited, either on the
spot or afterwards by updating the init table.

- Introduced HeapReconfigReason and reworked heapReconfigred API

heapReconfigred now distinguishes different reasons for reconfiguration
(Expand, contract, etc), this functionality is required for the Mark Map
init changes, specifically for Gencon and it is used when Scavenger
resizes tenure _(PhysicalSubArenaVirtualMemorySemiSpace)_ or global
collecter preforms a resize _(PhysicalSubArenaVirtualMemoryFlat)_. At
the time, this is new information is only consumed by Concurrent Global
Collecter (ConcurrentGC.cpp), for policies making use of other
collectors the HeapReconfigReason param defaults to `NONE`.

The Reconfig reasons are dealt in the following ways by Concurrent
Global Collector:

- We should never end up in `heapReconfigured` with `RECONFIG_NONE`
reason
- `RECONFIG_CONTRACT` signifies that `heapRemoveRange` had taken place,
in which case, we just need to update init table (`tuneToHeap`) when
Concurrent is Off, otherwise just `adjustTraceTargets`
- If `heapAddRange` takes place then `heapReconfigured` is called with
`RECONFIG_EXPAND` and have have two different cases:
      1)  heapAddRange was successful (return true), heap reconfig
should be provided with `lowAddress` & `highAddress` which will be used
to init mark map
      2) heapAddRange returns false, signifies a failed `heapAddRange`,
address params are expected to be NULL, in which case mark map won't be
inited but we'll still either `tuneToHeap` or `adjustTraceTarget`.

Signed-off-by: Salman Rana <salman.rana@ibm.com>

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 = NULL, void *lowAddress = NULL, void *highAddress = NULL)
+	{
+		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 = NULL, void *lowAddress = NULL, void *highAddress = NULL);
 
 	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 = HEAP_RECONFIG_NONE, MM_MemorySubSpace *subspace = NULL, void *lowAddress = NULL, void *highAddress = NULL);
 
 	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

@@ -3267,23 +3267,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());
@@ -3329,34 +3312,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 = NULL, void *lowAddress = NULL, void *highAddress = NULL);
 
 	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 = NULL, void *lowAddress = NULL, void *highAddress = NULL);
 
 	virtual	uint32_t getGCTimePercentage(MM_EnvironmentBase *env);