STORM-105 : takes Vadim's comments into account, clean up use of static globals and magic strings, enforce naming conventions

7 files changed, 59 insertions, 47 deletions

diff --git a/indra/llcommon/llmetricperformancetester.cpp b/indra/llcommon/llmetricperformancetester.cpp
index 2110192fbc..5fa3a5ea07 100644
--- a/indra/llcommon/llmetricperformancetester.cpp
+++ b/indra/llcommon/llmetricperformancetester.cpp
@@ -67,6 +67,7 @@ BOOL LLMetricPerformanceTesterBasic::addTester(LLMetricPerformanceTesterBasic* t
 /*static*/ 
 LLMetricPerformanceTesterBasic* LLMetricPerformanceTesterBasic::getTester(std::string name) 
 {
+	// Check for the requested metric name
 	name_tester_map_t::iterator found_it = sTesterMap.find(name) ;
 	if (found_it != sTesterMap.end())
 	{
@@ -74,6 +75,14 @@ LLMetricPerformanceTesterBasic* LLMetricPerformanceTesterBasic::getTester(std::s
 	}
 	return NULL ;
 }
+
+/*static*/ 
+// Return TRUE if this metric is requested or if the general default "catch all" metric is requested
+BOOL LLMetricPerformanceTesterBasic::isMetricLogRequested(std::string name)
+{
+	return (LLFastTimer::sMetricLog && ((LLFastTimer::sLogName == name) || (LLFastTimer::sLogName == DEFAULT_METRIC_NAME)));
+}
+
 	
 //----------------------------------------------------------------------------------------------
 // LLMetricPerformanceTesterBasic : Tester instance methods
@@ -126,13 +135,13 @@ void LLMetricPerformanceTesterBasic::analyzePerformance(std::ofstream* os, LLSD*
 {
 	resetCurrentCount() ;
 
-	std::string currentLabel = getCurrentLabelName();
-	BOOL in_base = (*base).has(currentLabel) ;
-	BOOL in_current = (*current).has(currentLabel) ;
+	std::string current_label = getCurrentLabelName();
+	BOOL in_base = (*base).has(current_label) ;
+	BOOL in_current = (*current).has(current_label) ;
 
 	while(in_base || in_current)
 	{
-		LLSD::String label = currentLabel ;		
+		LLSD::String label = current_label ;		
 
 		if(in_base && in_current)
 		{				
@@ -157,9 +166,9 @@ void LLMetricPerformanceTesterBasic::analyzePerformance(std::ofstream* os, LLSD*
 		}
 
 		incrementCurrentCount();
-		currentLabel = getCurrentLabelName();
-		in_base = (*base).has(currentLabel) ;
-		in_current = (*current).has(currentLabel) ;
+		current_label = getCurrentLabelName();
+		in_base = (*base).has(current_label) ;
+		in_current = (*current).has(current_label) ;
 	}
 }
 
diff --git a/indra/llcommon/llmetricperformancetester.h b/indra/llcommon/llmetricperformancetester.h
index 6fd1d41daa..925010ac96 100644
--- a/indra/llcommon/llmetricperformancetester.h
+++ b/indra/llcommon/llmetricperformancetester.h
@@ -27,6 +27,8 @@
 #ifndef LL_METRICPERFORMANCETESTER_H 
 #define LL_METRICPERFORMANCETESTER_H 
 
+const std::string DEFAULT_METRIC_NAME("metric");
+
 /**
  * @class LLMetricPerformanceTesterBasic
  * @brief Performance Metric Base Class
@@ -131,6 +133,13 @@ public:
 	 * @param[in] name - Name of the tester instance queried.
 	 */
 	static LLMetricPerformanceTesterBasic* getTester(std::string name) ;
+	
+	/**
+	 * @return Returns TRUE if that metric *or* the default catch all metric has been requested to be logged
+	 * @param[in] name - Name of the tester queried.
+	 */
+	static BOOL isMetricLogRequested(std::string name);
+	
 	/**
 	 * @return Returns TRUE if there's a tester defined, FALSE otherwise.
 	 */
diff --git a/indra/llimage/llimagej2c.cpp b/indra/llimage/llimagej2c.cpp
index 9173a331b3..d005aaf29f 100644
--- a/indra/llimage/llimagej2c.cpp
+++ b/indra/llimage/llimagej2c.cpp
@@ -174,12 +174,6 @@ std::string LLImageJ2C::getEngineInfo()
 	return j2cimpl_engineinfo_func();
 }
 
-//static
-bool LLImageJ2C::perfStatsEnabled() 
-{
-	return (sTesterp != NULL);
-}
-
 LLImageJ2C::LLImageJ2C() : 	LLImageFormatted(IMG_CODEC_J2C),
 							mMaxBytes(0),
 							mRawDiscardLevel(-1),
@@ -208,7 +202,8 @@ LLImageJ2C::LLImageJ2C() : 	LLImageFormatted(IMG_CODEC_J2C),
 		mDataSizes[i] = 0;
 	}
 
-	if (LLFastTimer::sMetricLog && !perfStatsEnabled() && ((LLFastTimer::sLogName == sTesterName) || (LLFastTimer::sLogName == "metric")))
+	// If that test log has ben requested but not yet created, create it
+	if (LLMetricPerformanceTesterBasic::isMetricLogRequested(sTesterName) && !LLMetricPerformanceTesterBasic::getTester(sTesterName))
 	{
 		sTesterp = new LLImageCompressionTester() ;
 		if (!sTesterp->isValid())
@@ -341,17 +336,18 @@ BOOL LLImageJ2C::decodeChannels(LLImageRaw *raw_imagep, F32 decode_time, S32 fir
 		LLImage::setLastError(mLastError);
 	}
 	
-	if (perfStatsEnabled())
+	LLImageCompressionTester* tester = (LLImageCompressionTester*)LLMetricPerformanceTesterBasic::getTester(sTesterName);
+	if (tester)
 	{
 		// Decompression stat gathering
 		// Note that we *do not* take into account the decompression failures data so we might overestimate the time spent processing
 
 		// Always add the decompression time to the stat
-		sTesterp->updateDecompressionStats(elapsed.getElapsedTimeF32()) ;
+		tester->updateDecompressionStats(elapsed.getElapsedTimeF32()) ;
 		if (res)
 		{
 			// The whole data stream is finally decompressed when res is returned as TRUE
-			sTesterp->updateDecompressionStats(this->getDataSize(), raw_imagep->getDataSize()) ;
+			tester->updateDecompressionStats(this->getDataSize(), raw_imagep->getDataSize()) ;
 		}
 	}
 
@@ -376,17 +372,18 @@ BOOL LLImageJ2C::encode(const LLImageRaw *raw_imagep, const char* comment_text,
 		LLImage::setLastError(mLastError);
 	}
 
-	if (perfStatsEnabled())
+	LLImageCompressionTester* tester = (LLImageCompressionTester*)LLMetricPerformanceTesterBasic::getTester(sTesterName);
+	if (tester)
 	{
 		// Compression stat gathering
 		// Note that we *do not* take into account the compression failures cases so we night overestimate the time spent processing
 
 		// Always add the compression time to the stat
-		sTesterp->updateCompressionStats(elapsed.getElapsedTimeF32()) ;
+		tester->updateCompressionStats(elapsed.getElapsedTimeF32()) ;
 		if (res)
 		{
 			// The whole data stream is finally compressed when res is returned as TRUE
-			sTesterp->updateCompressionStats(this->getDataSize(), raw_imagep->getDataSize()) ;
+			tester->updateCompressionStats(this->getDataSize(), raw_imagep->getDataSize()) ;
 		}
 	}
 
diff --git a/indra/llimage/llimagej2c.h b/indra/llimage/llimagej2c.h
index 7333f0370f..cc3dabd7d8 100644
--- a/indra/llimage/llimagej2c.h
+++ b/indra/llimage/llimagej2c.h
@@ -97,7 +97,6 @@ protected:
 
     // Image compression/decompression tester
 	static LLImageCompressionTester* sTesterp;
-	static bool perfStatsEnabled();
 };
 
 // Derive from this class to implement JPEG2000 decoding
diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index 5b69fd80af..bf0f948a6d 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -1638,14 +1638,14 @@ bool LLAppViewer::cleanup()
 	{
 		llinfos << "Analyzing performance" << llendl;
 		
-		std::string baselineName = LLFastTimer::sLogName + "_baseline.slp";
-		std::string currentName  = LLFastTimer::sLogName + ".slp"; 
-		std::string reportName   = LLFastTimer::sLogName + "_report.csv";
+		std::string baseline_name = LLFastTimer::sLogName + "_baseline.slp";
+		std::string current_name  = LLFastTimer::sLogName + ".slp"; 
+		std::string report_name   = LLFastTimer::sLogName + "_report.csv";
 
 		LLFastTimerView::doAnalysis(
-			gDirUtilp->getExpandedFilename(LL_PATH_LOGS, baselineName),
-			gDirUtilp->getExpandedFilename(LL_PATH_LOGS, currentName),
-			gDirUtilp->getExpandedFilename(LL_PATH_LOGS, reportName));
+			gDirUtilp->getExpandedFilename(LL_PATH_LOGS, baseline_name),
+			gDirUtilp->getExpandedFilename(LL_PATH_LOGS, current_name),
+			gDirUtilp->getExpandedFilename(LL_PATH_LOGS, report_name));
 	}
 	LLMetricPerformanceTesterBasic::cleanClass() ;
 
@@ -2117,8 +2117,8 @@ bool LLAppViewer::initConfiguration()
 		llinfos << "'--logmetrics' argument : " << test_name << llendl;
 		if (test_name == "")
 		{
-			llwarns << "No '--logmetrics' argument given, will output all metrics." << llendl;
-			LLFastTimer::sLogName = std::string("metric");
+			llwarns << "No '--logmetrics' argument given, will output all metrics to " << DEFAULT_METRIC_NAME << llendl;
+			LLFastTimer::sLogName = DEFAULT_METRIC_NAME;
 		}
 		else
 		{
diff --git a/indra/newview/llviewertexture.cpp b/indra/newview/llviewertexture.cpp
index 2b27f308df..6160510c0e 100644
--- a/indra/newview/llviewertexture.cpp
+++ b/indra/newview/llviewertexture.cpp
@@ -288,12 +288,6 @@ LLViewerFetchedTexture* LLViewerTextureManager::getFetchedTextureFromUrl(const s
 	return gTextureList.getImageFromUrl(url, usemipmaps, boost_priority, texture_type, internal_format, primary_format, force_id) ;
 }
 
-//static
-bool LLViewerTextureManager::perfStatsEnabled() 
-{
-	return (sTesterp != NULL);
-}
-
 LLViewerFetchedTexture* LLViewerTextureManager::getFetchedTextureFromHost(const LLUUID& image_id, LLHost host) 
 {
 	return gTextureList.getImageFromHost(image_id, host) ;
@@ -348,7 +342,7 @@ void LLViewerTextureManager::init()
 
 	LLViewerTexture::initClass() ;
 
-	if (LLFastTimer::sMetricLog && !perfStatsEnabled() && ((LLFastTimer::sLogName == sTesterName) || (LLFastTimer::sLogName == "metric")))
+	if (LLMetricPerformanceTesterBasic::isMetricLogRequested(sTesterName) && !LLMetricPerformanceTesterBasic::getTester(sTesterName))
 	{
 		sTesterp = new LLTexturePipelineTester() ;
 		if (!sTesterp->isValid())
@@ -420,9 +414,10 @@ void LLViewerTexture::updateClass(const F32 velocity, const F32 angular_velocity
 {
 	sCurrentTime = gFrameTimeSeconds ;
 
-	if (LLViewerTextureManager::perfStatsEnabled())
+	LLTexturePipelineTester* tester = (LLTexturePipelineTester*)LLMetricPerformanceTesterBasic::getTester(sTesterName);
+	if (tester)
 	{
-		LLViewerTextureManager::sTesterp->update() ;
+		tester->update() ;
 	}
 	LLViewerMediaTexture::updateClass() ;
 
@@ -615,9 +610,10 @@ bool LLViewerTexture::bindDefaultImage(S32 stage)
 	//check if there is cached raw image and switch to it if possible
 	switchToCachedImage() ;
 
-	if (LLViewerTextureManager::perfStatsEnabled())
+	LLTexturePipelineTester* tester = (LLTexturePipelineTester*)LLMetricPerformanceTesterBasic::getTester(sTesterName);
+	if (tester)
 	{
-		LLViewerTextureManager::sTesterp->updateGrayTextureBinding() ;
+		tester->updateGrayTextureBinding() ;
 	}
 	return res;
 }
@@ -1078,9 +1074,10 @@ BOOL LLViewerTexture::isLargeImage()
 //virtual 
 void LLViewerTexture::updateBindStatsForTester()
 {
-	if (LLViewerTextureManager::perfStatsEnabled())
+	LLTexturePipelineTester* tester = (LLTexturePipelineTester*)LLMetricPerformanceTesterBasic::getTester(sTesterName);
+	if (tester)
 	{
-		LLViewerTextureManager::sTesterp->updateTextureBindingStats(this) ;
+		tester->updateTextureBindingStats(this) ;
 	}
 }
 
@@ -1861,10 +1858,11 @@ bool LLViewerFetchedTexture::updateFetch()
 		// We may have data ready regardless of whether or not we are finished (e.g. waiting on write)
 		if (mRawImage.notNull())
 		{
-			if (LLViewerTextureManager::perfStatsEnabled())
+			LLTexturePipelineTester* tester = (LLTexturePipelineTester*)LLMetricPerformanceTesterBasic::getTester(sTesterName);
+			if (tester)
 			{
 				mIsFetched = TRUE ;
-				LLViewerTextureManager::sTesterp->updateTextureLoadingStats(this, mRawImage, LLAppViewer::getTextureFetch()->isFromLocalCache(mID)) ;
+				tester->updateTextureLoadingStats(this, mRawImage, LLAppViewer::getTextureFetch()->isFromLocalCache(mID)) ;
 			}
 			mRawDiscardLevel = fetch_discard;
 			if ((mRawImage->getDataSize() > 0 && mRawDiscardLevel >= 0) &&
@@ -3088,9 +3086,10 @@ void LLViewerLODTexture::scaleDown()
 	{		
 		switchToCachedImage() ;	
 
-		if (LLViewerTextureManager::perfStatsEnabled())
+		LLTexturePipelineTester* tester = (LLTexturePipelineTester*)LLMetricPerformanceTesterBasic::getTester(sTesterName);
+		if (tester)
 		{
-			LLViewerTextureManager::sTesterp->setStablizingTime() ;
+			tester->setStablizingTime() ;
 		}
 	}
 }
diff --git a/indra/newview/llviewertexture.h b/indra/newview/llviewertexture.h
index 88d449e061..b5636bbdc7 100644
--- a/indra/newview/llviewertexture.h
+++ b/indra/newview/llviewertexture.h
@@ -676,7 +676,6 @@ private:
 public:
     //texture pipeline tester
 	static LLTexturePipelineTester* sTesterp ;
-	static bool perfStatsEnabled();
 
 	//returns NULL if tex is not a LLViewerFetchedTexture nor derived from LLViewerFetchedTexture.
 	static LLViewerFetchedTexture*    staticCastToFetchedTexture(LLTexture* tex, BOOL report_error = FALSE) ;