Merge pull request #2328 from jonoommen/issue-2288

Refactor MM_ScavengerHotFieldStats to be dynamically allocated
eclipse-openj9 · Feb 28, 2018 · 3e8296b · 3e8296b
2 parents 22036ed + 60b782f
commit 3e8296b
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 27 deletions.
diff --git a/gc/base/EnvironmentBase.cpp b/gc/base/EnvironmentBase.cpp
@@ -100,8 +100,11 @@ MM_EnvironmentBase::initialize(MM_GCExtensionsBase *extensions)
 	}
 
 #if defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC)
-	if (!_hotFieldStats.initialize(this)) {
-		return false;
+	if (extensions->scavengerTraceHotFields) {
+		_hotFieldStats = MM_ScavengerHotFieldStats::newInstance(getExtensions());
+		if (NULL == _hotFieldStats) {
+			return false;
+		}
 	}
 #endif /* defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC) */
 
@@ -151,7 +154,10 @@ MM_EnvironmentBase::tearDown(MM_GCExtensionsBase *extensions)
 #endif /* OMR_GC_SEGREGATED_HEAP */
 
 #if defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC)
-	_hotFieldStats.tearDown(this);
+	if (NULL != _hotFieldStats) {
+		_hotFieldStats->kill(this->getExtensions());
+		_hotFieldStats = NULL;
+	}
 #endif /* defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC) */
 
 	if(NULL != _objectAllocationInterface) {

diff --git a/gc/base/EnvironmentBase.hpp b/gc/base/EnvironmentBase.hpp
@@ -171,7 +171,7 @@ class MM_EnvironmentBase : public MM_BaseVirtual
 #endif /* OMR_GC_MODRON_STANDARD || OMR_GC_REALTIME */
 #if defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC)
 	MM_ScavengerStats _scavengerStats;
-	MM_ScavengerHotFieldStats _hotFieldStats; /**< hot field statistics for this GC thread */
+	MM_ScavengerHotFieldStats *_hotFieldStats; /**< hot field statistics for this GC thread */
 #endif /* defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC) */
 #if defined(OMR_GC_CONCURRENT_SCAVENGER)
 	uint64_t _concurrentScavengerSwitchCount; /**< local counter of cycle start and cycle end transitions */
@@ -632,6 +632,9 @@ class MM_EnvironmentBase : public MM_BaseVirtual
 #if defined(OMR_GC_SEGREGATED_HEAP)
 		,_allocationTracker(NULL)
 #endif /* OMR_GC_SEGREGATED_HEAP */
+#if defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC)
+		,_hotFieldStats(NULL)
+#endif /* defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC) */
 #if defined(OMR_GC_CONCURRENT_SCAVENGER)
 		,_concurrentScavengerSwitchCount(0)
 #endif /* defined(OMR_GC_CONCURRENT_SCAVENGER) */

diff --git a/gc/base/GCExtensionsBase.cpp b/gc/base/GCExtensionsBase.cpp
@@ -212,8 +212,11 @@ MM_GCExtensionsBase::initialize(MM_EnvironmentBase* env)
 	}
 
 #if defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC)
-	if (!scavengerHotFieldStats.initialize(env)) {
-		goto failed;
+	if(scavengerTraceHotFields) {
+		scavengerHotFieldStats = MM_ScavengerHotFieldStats::newInstance(this);
+		if (NULL == scavengerHotFieldStats) {
+			goto failed;
+		}
 	}
 #endif /* defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC) */
 
@@ -244,7 +247,10 @@ void
 MM_GCExtensionsBase::tearDown(MM_EnvironmentBase* env)
 {
 #if defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC)
-	scavengerHotFieldStats.tearDown(env);
+	if (NULL != scavengerHotFieldStats) {
+		scavengerHotFieldStats->kill(this);
+		scavengerHotFieldStats = NULL;
+	}
 #endif /* defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC) */
 
 #if defined(OMR_GC_MODRON_SCAVENGER)

diff --git a/gc/base/GCExtensionsBase.hpp b/gc/base/GCExtensionsBase.hpp
@@ -407,7 +407,7 @@ class MM_GCExtensionsBase : public MM_BaseVirtual {
 	};
 	ScavengerScanOrdering scavengerScanOrdering; /**< scan ordering in Scavenger */
 	bool scavengerTraceHotFields; /**< whether tracing hot fields in Scavenger is enabled */
-	MM_ScavengerHotFieldStats scavengerHotFieldStats; /**< hot field stats accumulated over all GC threads */
+	MM_ScavengerHotFieldStats *scavengerHotFieldStats; /**< hot field stats accumulated over all GC threads */
 #if defined(OMR_GC_MODRON_SCAVENGER)
 	uintptr_t scvTenureRatioHigh;
 	uintptr_t scvTenureRatioLow;
@@ -1338,6 +1338,7 @@ class MM_GCExtensionsBase : public MM_BaseVirtual {
 #if defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC)
 		, scavengerScanOrdering(OMR_GC_SCAVENGER_SCANORDERING_HIERARCHICAL)
 		, scavengerTraceHotFields(false)
+		, scavengerHotFieldStats(NULL)
 #endif /* OMR_GC_MODRON_SCAVENGER || OMR_GC_VLHGC */
 #if defined(OMR_GC_MODRON_SCAVENGER)
 		, scvTenureRatioHigh(J9_SCV_TENURE_RATIO_HIGH)

diff --git a/gc/base/standard/Scavenger.cpp b/gc/base/standard/Scavenger.cpp
@@ -615,8 +615,8 @@ MM_Scavenger::reportGCEnd(MM_EnvironmentStandard *env)
 void
 MM_Scavenger::masterClearHotFieldStats()
 {
-	if (_extensions->scavengerTraceHotFields) {
-		_extensions->scavengerHotFieldStats.clear();
+	if (NULL != _extensions->scavengerHotFieldStats) {
+		_extensions->scavengerHotFieldStats->clear();
 	}
 }
 
@@ -627,8 +627,8 @@ MM_Scavenger::masterClearHotFieldStats()
 void
 MM_Scavenger::masterReportHotFieldStats()
 {
-	if (_extensions->scavengerTraceHotFields) {
-		_extensions->scavengerHotFieldStats.reportStats(_omrVM);
+	if (NULL != _extensions->scavengerHotFieldStats) {
+		_extensions->scavengerHotFieldStats->reportStats(_omrVM);
 	}
 }
 
@@ -638,7 +638,7 @@ MM_Scavenger::masterReportHotFieldStats()
 void
 MM_Scavenger::clearHotFieldStats(MM_EnvironmentBase *env)
 {
-	if (_extensions->scavengerTraceHotFields) {
+	if (NULL != env->_hotFieldStats) {
 		getHotFieldStats(env)->clear();
 	}
 }
@@ -649,8 +649,8 @@ MM_Scavenger::clearHotFieldStats(MM_EnvironmentBase *env)
 void
 MM_Scavenger::mergeHotFieldStats(MM_EnvironmentBase *env)
 {
-	if (_extensions->scavengerTraceHotFields) {
-		_extensions->scavengerHotFieldStats.mergeStats(getHotFieldStats(env));
+	if (NULL != _extensions->scavengerHotFieldStats) {
+		_extensions->scavengerHotFieldStats->mergeStats(getHotFieldStats(env));
 	}
 }
 
@@ -1652,7 +1652,7 @@ MM_Scavenger::scavengeObjectSlots(MM_EnvironmentStandard *env, MM_CopyScanCacheS
 			/* scan to end of array if can't split */
 			((GC_IndexableObjectScanner *)objectScanner)->scanToLimit();
 		}
-	} else if (_extensions->scavengerTraceHotFields) {
+	} else if (NULL != env->_hotFieldStats) {
 		/* maintain hotness of fields copied from this object */
 		hotFieldStats = getHotFieldStats(env);
 		hotFieldStats->_objectPtr = objectPtr;
@@ -1756,7 +1756,7 @@ MM_Scavenger::incrementalScavengeObjectSlots(MM_EnvironmentStandard *env, omrobj
 
 	/* Set up for maintaining hot field stats for scalar objects */
 	MM_ScavengerHotFieldStats *hotFieldStats = NULL;
-	if (!objectScanner->isIndexableObject() && _extensions->scavengerTraceHotFields) {
+	if (!objectScanner->isIndexableObject() && (NULL != env->_hotFieldStats)) {
 		/* maintain hotness of fields copied from this object */
 		hotFieldStats = getHotFieldStats(env);
 		hotFieldStats->_objectPtr = objectPtr;

diff --git a/gc/base/standard/Scavenger.hpp b/gc/base/standard/Scavenger.hpp
@@ -370,7 +370,7 @@ class MM_Scavenger : public MM_Collector
 	void reportScavengeStart(MM_EnvironmentStandard *env);
 	void reportScavengeEnd(MM_EnvironmentStandard *env, bool lastIncrement);
 
-	MMINLINE MM_ScavengerHotFieldStats *getHotFieldStats(MM_EnvironmentBase *env) { return &(env->_hotFieldStats); }
+	MMINLINE MM_ScavengerHotFieldStats *getHotFieldStats(MM_EnvironmentBase *env) { return env->_hotFieldStats; }
 	void masterClearHotFieldStats();
 	void masterReportHotFieldStats();
 	void clearHotFieldStats(MM_EnvironmentBase *env);

diff --git a/gc/stats/ScavengerHotFieldStats.cpp b/gc/stats/ScavengerHotFieldStats.cpp
@@ -27,12 +27,33 @@
 
 #if defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC)
 
+MM_ScavengerHotFieldStats*
+MM_ScavengerHotFieldStats::newInstance(MM_GCExtensionsBase *extensions)
+{
+	MM_ScavengerHotFieldStats *scavengerHotFieldStats = (MM_ScavengerHotFieldStats *)extensions->getForge()->allocate(sizeof(MM_ScavengerHotFieldStats), OMR::GC::AllocationCategory::FIXED, OMR_GET_CALLSITE());
+	if (NULL != scavengerHotFieldStats) {
+		new(scavengerHotFieldStats) MM_ScavengerHotFieldStats();
+		if (!scavengerHotFieldStats->initialize(extensions)) {
+			scavengerHotFieldStats->kill(extensions);
+			scavengerHotFieldStats = NULL;
+		}
+	}
+	return scavengerHotFieldStats;
+}
+
 bool
-MM_ScavengerHotFieldStats::initialize(MM_EnvironmentBase *env)
+MM_ScavengerHotFieldStats::initialize(MM_GCExtensionsBase *extensions)
 {
-	_objectModel = &(env->getExtensions()->objectModel);
+	_objectModel = &(extensions->objectModel);
 	return true;
 }
 
+void
+MM_ScavengerHotFieldStats::kill(MM_GCExtensionsBase *extensions)
+{
+	tearDown(extensions);
+	extensions->getForge()->free(this);
+}
+
 #endif /* defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC) */
 
diff --git a/gc/stats/ScavengerHotFieldStats.hpp b/gc/stats/ScavengerHotFieldStats.hpp
@@ -92,7 +92,15 @@ class MM_ScavengerHotFieldStats : public MM_Base
 	 */
 	uintptr_t _connectionHistgm[sizeof(uintptr_t)*8][SizeScavengerHotness][SizeScavengerObjectConnectorType];
 
+protected:
+	bool initialize(MM_GCExtensionsBase *extensions);	
+	void tearDown(MM_GCExtensionsBase *extensions) {} 
+
 public:
+
+	static MM_ScavengerHotFieldStats * newInstance(MM_GCExtensionsBase *extensions);	
+	void kill(MM_GCExtensionsBase *extensions); 
+
 	/**
 	 * When no information is known about the hotness of a field, then the default is hot.
 	 * This clears to this default value
@@ -117,13 +125,7 @@ class MM_ScavengerHotFieldStats : public MM_Base
 			}
 		}
 	}
-
-	MM_ScavengerHotFieldStats() { clear(); }
-
-	bool initialize(MM_EnvironmentBase *env);
-
-	void tearDown(MM_EnvironmentBase *env) {}
-
+
 	/**
 	 * Merges the given hot fields statistics into this one
 	 * @param statsToMerge the statistics to merge
@@ -295,6 +297,18 @@ class MM_ScavengerHotFieldStats : public MM_Base
 		omrtty_printf(" }\n");
 		omrtty_printf("{ Hot Field Statistics nursery-tenured: end }\n");
 	}
+
+	/**
+	 * Create a SvavengerHotFieldStats object.
+	 */
+
+	MM_ScavengerHotFieldStats() :
+		_objectModel(NULL)
+		, _hotness(0)
+	{
+		clear(); 
+	}	
 };
+
 #endif /* defined(OMR_GC_MODRON_SCAVENGER) || defined(OMR_GC_VLHGC) */
 #endif /* SCAVENGERHOTFIELDSTATS_HPP_ */