ESC-154 ESC-155 Viewer metrics fixes for min/max merged values, floating timestamps.

The min/max response time calculations needed to be sensitive to the response
counts to know if their was actual data.  Failure to do so introduced a
gratuitous min/max test against zero values which tended to corrupt the
mins.  Unit tests added to test for this condition.  Finished conversion
of times to floating point seconds.  Removed two logging events used to
debug the cross-thread messaging.  Looks like a code completion point.

3 files changed, 271 insertions, 21 deletions

diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp
index 2be3ba3280..f5e2e35e1e 100644
--- a/indra/newview/lltexturefetch.cpp
+++ b/indra/newview/lltexturefetch.cpp
@@ -2659,7 +2659,6 @@ void LLTextureFetch::commandSetRegion(const LLUUID & region_id)
 	TFReqSetRegion * req = new TFReqSetRegion(region_id);
 
 	cmdEnqueue(req);
-	LL_INFOS("Texture") << "COMMANDING SET REGION" << LL_ENDL;
 }
 
 void LLTextureFetch::commandSendMetrics(const std::string & caps_url,
@@ -2821,7 +2820,6 @@ TFReqSendMetrics::doWork(LLTextureFetch * fetcher)
 	LLViewerAssetStatsFF::merge_stats(main_stats, thread1_stats);
 
 	// *TODO:  Consider putting a report size limiter here.
-	LL_INFOS("Texture") << "PROCESSING SENDMETRICS REQUEST" << LL_ENDL;
 	if (! mCapsURL.empty())
 	{
 		LLCurlRequest::headers_t headers;
@@ -2839,7 +2837,7 @@ TFReqSendMetrics::doWork(LLTextureFetch * fetcher)
 	// In QA mode, Metrics submode, log the result for ease of testing
 	if (fetcher->isQAMode())
 	{
-		LL_INFOS("QAViewerMetrics") << thread1_stats << LL_ENDL;
+		LL_INFOS("Textures") << thread1_stats << LL_ENDL;
 	}
 
 	gViewerAssetStatsThread1->reset();
diff --git a/indra/newview/llviewerassetstats.cpp b/indra/newview/llviewerassetstats.cpp
index 3d7f9f932f..502a3aa340 100644
--- a/indra/newview/llviewerassetstats.cpp
+++ b/indra/newview/llviewerassetstats.cpp
@@ -263,19 +263,19 @@ LLViewerAssetStats::asLLSD()
 			slot[enq_tag] = LLSD(S32(stats.mRequests[i].mEnqueued.getCount()));
 			slot[deq_tag] = LLSD(S32(stats.mRequests[i].mDequeued.getCount()));
 			slot[rcnt_tag] = LLSD(S32(stats.mRequests[i].mResponse.getCount()));
-			slot[rmin_tag] = LLSD(F64(stats.mRequests[i].mResponse.getMin()));
-			slot[rmax_tag] = LLSD(F64(stats.mRequests[i].mResponse.getMax()));
-			slot[rmean_tag] = LLSD(F64(stats.mRequests[i].mResponse.getMean()));
+			slot[rmin_tag] = LLSD(F64(stats.mRequests[i].mResponse.getMin() * 1.0e-6));
+			slot[rmax_tag] = LLSD(F64(stats.mRequests[i].mResponse.getMax() * 1.0e-6));
+			slot[rmean_tag] = LLSD(F64(stats.mRequests[i].mResponse.getMean() * 1.0e-6));
 		}
 
-		reg_stat["duration"] = LLSD::Integer(stats.mTotalTime / 1000000);
+		reg_stat["duration"] = LLSD::Real(stats.mTotalTime * 1.0e-6);
 		
 		regions[it->first.asString()] = reg_stat;
 	}
 
 	LLSD ret = LLSD::emptyMap();
 	ret["regions"] = regions;
-	ret["duration"] = LLSD::Integer((now - mResetTimestamp) / 1000000);
+	ret["duration"] = LLSD::Real((now - mResetTimestamp) * 1.0e-6);
 	
 	return ret;
 }
@@ -290,12 +290,12 @@ LLViewerAssetStats::mergeRegionsLLSD(const LLSD & src, LLSD & dst)
 	static const int MOP_MEAN_REAL(3);	// Requires a 'mMergeOpArg' to weight the input terms
 
 	static const LLSD::String regions_key("regions");
+	static const LLSD::String resp_count_key("resp_count");
 	
 	static const struct
 		{
 			LLSD::String		mName;
 			int					mMergeOp;
-			LLSD::String		mMergeOpArg;
 		}
 	key_list[] =
 		{
@@ -305,12 +305,12 @@ LLViewerAssetStats::mergeRegionsLLSD(const LLSD & src, LLSD & dst)
 			// is modified or the weight will be wrong.  Key list is
 			// defined in asLLSD() and must track it.
 
-			{ "resp_mean", MOP_MEAN_REAL, "resp_count" },
-			{ "enqueued", MOP_ADD_INT, "" },
-			{ "dequeued", MOP_ADD_INT, "" },
-			{ "resp_count", MOP_ADD_INT, "" },
-			{ "resp_min", MOP_MIN_REAL, "" },
-			{ "resp_max", MOP_MAX_REAL, "" }
+			{ "resp_mean", MOP_MEAN_REAL },
+			{ "enqueued", MOP_ADD_INT },
+			{ "dequeued", MOP_ADD_INT },
+			{ "resp_min", MOP_MIN_REAL },
+			{ "resp_max", MOP_MAX_REAL },
+			{ resp_count_key, MOP_ADD_INT }			// Keep last
 		};
 
 	// Trivial checks
@@ -368,6 +368,10 @@ LLViewerAssetStats::mergeRegionsLLSD(const LLSD & src, LLSD & dst)
 					LLSD & bin_dst(reg_dst[it_sets->first]);
 					const LLSD & bin_src(reg_src[it_sets->first]);
 
+					// The "resp_count" value is needed repeatedly in operations.
+					const LLSD::Integer bin_src_count(bin_src[resp_count_key].asInteger());
+					const LLSD::Integer bin_dst_count(bin_dst[resp_count_key].asInteger());
+			
 					for (int key_index(0); key_index < LL_ARRAY_SIZE(key_list); ++key_index)
 					{
 						const LLSD::String & key_name(key_list[key_index].mName);
@@ -395,25 +399,45 @@ LLViewerAssetStats::mergeRegionsLLSD(const LLSD & src, LLSD & dst)
 							case MOP_ADD_INT:
 								// Simple counts, just add
 								dst_value = dst_value.asInteger() + src_value.asInteger();
-						
 								break;
 						
 							case MOP_MIN_REAL:
 								// Minimum
-								dst_value = llmin(dst_value.asReal(), src_value.asReal());
+								if (bin_src_count)
+								{
+									// If src has non-zero count, it's min is meaningful
+									if (bin_dst_count)
+									{
+										dst_value = llmin(dst_value.asReal(), src_value.asReal());
+									}
+									else
+									{
+										dst_value = src_value;
+									}
+								}
 								break;
 
 							case MOP_MAX_REAL:
 								// Maximum
-								dst_value = llmax(dst_value.asReal(), src_value.asReal());
+								if (bin_src_count)
+								{
+									// If src has non-zero count, it's max is meaningful
+									if (bin_dst_count)
+									{
+										dst_value = llmax(dst_value.asReal(), src_value.asReal());
+									}
+									else
+									{
+										dst_value = src_value;
+									}
+								}
 								break;
 
 							case MOP_MEAN_REAL:
 							    {
 									// Mean
-									const LLSD::String & weight_key(key_list[key_index].mMergeOpArg);
-									F64 src_weight(bin_src[weight_key].asReal());
-									F64 dst_weight(bin_dst[weight_key].asReal());
+									F64 src_weight(bin_src_count);
+									F64 dst_weight(bin_dst_count);
 									F64 tot_weight(src_weight + dst_weight);
 									if (tot_weight >= F64(0.5))
 									{
diff --git a/indra/newview/tests/llviewerassetstats_test.cpp b/indra/newview/tests/llviewerassetstats_test.cpp
index a44712e8ad..8bedd2c860 100644
--- a/indra/newview/tests/llviewerassetstats_test.cpp
+++ b/indra/newview/tests/llviewerassetstats_test.cpp
@@ -518,4 +518,232 @@ namespace tut
 			ensure_approximately_equals("weighted mean of means", dst["regions"][reg1_name]["get_other"]["resp_mean"].asReal(), 2.7901295, 20);
 		}
 	}
+
+	// Maximum merges are interesting when one side contributes nothing
+	template<> template<>
+	void tst_viewerassetstats_index_object_t::test<10>()
+	{
+		LLSD::String reg1_name = region1.asString();
+		LLSD::String reg2_name = region2.asString();
+
+		LLSD reg1_stats = LLSD::emptyMap();
+		LLSD reg2_stats = LLSD::emptyMap();
+
+		LLSD & tmp_other1 = reg1_stats["get_other"];
+		tmp_other1["enqueued"] = 4;
+		tmp_other1["dequeued"] = 4;
+		tmp_other1["resp_count"] = 7;
+		tmp_other1["resp_max"] = F64(-23.2892);
+		tmp_other1["resp_min"] = F64(-123.2892);
+		tmp_other1["resp_mean"] = F64(-58.28298);
+
+		LLSD & tmp_other2 = reg2_stats["get_other"];
+		tmp_other2["enqueued"] = 8;
+		tmp_other2["dequeued"] = 7;
+		tmp_other2["resp_count"] = 0;
+		tmp_other2["resp_max"] = F64(0);
+		tmp_other2["resp_min"] = F64(0);
+		tmp_other2["resp_mean"] = F64(0);
+		
+		{
+			LLSD src = LLSD::emptyMap();
+			LLSD dst = LLSD::emptyMap();
+
+			src["regions"][reg1_name] = reg1_stats;
+			src["duration"] = 24;
+			dst["regions"][reg1_name] = reg2_stats;
+			dst["duration"] = 36;
+
+			LLViewerAssetStats::mergeRegionsLLSD(src, dst);
+		
+			ensure_approximately_equals("dst maximum with count 0 does not contribute to merged maximum",
+										dst["regions"][reg1_name]["get_other"]["resp_max"].asReal(), F64(-23.2892), 20);
+		}
+
+		{
+			LLSD src = LLSD::emptyMap();
+			LLSD dst = LLSD::emptyMap();
+
+			src["regions"][reg1_name] = reg2_stats;
+			src["duration"] = 24;
+			dst["regions"][reg1_name] = reg1_stats;
+			dst["duration"] = 36;
+
+			LLViewerAssetStats::mergeRegionsLLSD(src, dst);
+		
+			ensure_approximately_equals("src maximum with count 0 does not contribute to merged maximum",
+										dst["regions"][reg1_name]["get_other"]["resp_max"].asReal(), F64(-23.2892), 20);
+		}
+	}
+
+    // Minimum merges are interesting when one side contributes nothing
+	template<> template<>
+	void tst_viewerassetstats_index_object_t::test<11>()
+	{
+		LLSD::String reg1_name = region1.asString();
+		LLSD::String reg2_name = region2.asString();
+
+		LLSD reg1_stats = LLSD::emptyMap();
+		LLSD reg2_stats = LLSD::emptyMap();
+
+		LLSD & tmp_other1 = reg1_stats["get_other"];
+		tmp_other1["enqueued"] = 4;
+		tmp_other1["dequeued"] = 4;
+		tmp_other1["resp_count"] = 7;
+		tmp_other1["resp_max"] = F64(123.2892);
+		tmp_other1["resp_min"] = F64(23.2892);
+		tmp_other1["resp_mean"] = F64(58.28298);
+
+		LLSD & tmp_other2 = reg2_stats["get_other"];
+		tmp_other2["enqueued"] = 8;
+		tmp_other2["dequeued"] = 7;
+		tmp_other2["resp_count"] = 0;
+		tmp_other2["resp_max"] = F64(0);
+		tmp_other2["resp_min"] = F64(0);
+		tmp_other2["resp_mean"] = F64(0);
+		
+		{
+			LLSD src = LLSD::emptyMap();
+			LLSD dst = LLSD::emptyMap();
+
+			src["regions"][reg1_name] = reg1_stats;
+			src["duration"] = 24;
+			dst["regions"][reg1_name] = reg2_stats;
+			dst["duration"] = 36;
+
+			LLViewerAssetStats::mergeRegionsLLSD(src, dst);
+		
+			ensure_approximately_equals("dst minimum with count 0 does not contribute to merged minimum",
+										dst["regions"][reg1_name]["get_other"]["resp_min"].asReal(), F64(23.2892), 20);
+		}
+
+		{
+			LLSD src = LLSD::emptyMap();
+			LLSD dst = LLSD::emptyMap();
+
+			src["regions"][reg1_name] = reg2_stats;
+			src["duration"] = 24;
+			dst["regions"][reg1_name] = reg1_stats;
+			dst["duration"] = 36;
+
+			LLViewerAssetStats::mergeRegionsLLSD(src, dst);
+		
+			ensure_approximately_equals("src minimum with count 0 does not contribute to merged minimum",
+										dst["regions"][reg1_name]["get_other"]["resp_min"].asReal(), F64(23.2892), 20);
+		}
+	}
+
+    // resp_count missing is taken as '0' for maximum calculation
+	template<> template<>
+	void tst_viewerassetstats_index_object_t::test<12>()
+	{
+		LLSD::String reg1_name = region1.asString();
+		LLSD::String reg2_name = region2.asString();
+
+		LLSD reg1_stats = LLSD::emptyMap();
+		LLSD reg2_stats = LLSD::emptyMap();
+
+		LLSD & tmp_other1 = reg1_stats["get_other"];
+		tmp_other1["enqueued"] = 4;
+		tmp_other1["dequeued"] = 4;
+		tmp_other1["resp_count"] = 7;
+		tmp_other1["resp_max"] = F64(-23.2892);
+		tmp_other1["resp_min"] = F64(-123.2892);
+		tmp_other1["resp_mean"] = F64(-58.28298);
+
+		LLSD & tmp_other2 = reg2_stats["get_other"];
+		tmp_other2["enqueued"] = 8;
+		tmp_other2["dequeued"] = 7;
+		// tmp_other2["resp_count"] = 0;
+		tmp_other2["resp_max"] = F64(0);
+		tmp_other2["resp_min"] = F64(0);
+		tmp_other2["resp_mean"] = F64(0);
+		
+		{
+			LLSD src = LLSD::emptyMap();
+			LLSD dst = LLSD::emptyMap();
+
+			src["regions"][reg1_name] = reg1_stats;
+			src["duration"] = 24;
+			dst["regions"][reg1_name] = reg2_stats;
+			dst["duration"] = 36;
+
+			LLViewerAssetStats::mergeRegionsLLSD(src, dst);
+		
+			ensure_approximately_equals("dst maximum with undefined count does not contribute to merged maximum",
+										dst["regions"][reg1_name]["get_other"]["resp_max"].asReal(), F64(-23.2892), 20);
+		}
+
+		{
+			LLSD src = LLSD::emptyMap();
+			LLSD dst = LLSD::emptyMap();
+
+			src["regions"][reg1_name] = reg2_stats;
+			src["duration"] = 24;
+			dst["regions"][reg1_name] = reg1_stats;
+			dst["duration"] = 36;
+
+			LLViewerAssetStats::mergeRegionsLLSD(src, dst);
+		
+			ensure_approximately_equals("src maximum with undefined count does not contribute to merged maximum",
+										dst["regions"][reg1_name]["get_other"]["resp_max"].asReal(), F64(-23.2892), 20);
+		}
+	}
+
+    // resp_count unspecified is taken as 0 for minimum merges
+	template<> template<>
+	void tst_viewerassetstats_index_object_t::test<13>()
+	{
+		LLSD::String reg1_name = region1.asString();
+		LLSD::String reg2_name = region2.asString();
+
+		LLSD reg1_stats = LLSD::emptyMap();
+		LLSD reg2_stats = LLSD::emptyMap();
+
+		LLSD & tmp_other1 = reg1_stats["get_other"];
+		tmp_other1["enqueued"] = 4;
+		tmp_other1["dequeued"] = 4;
+		tmp_other1["resp_count"] = 7;
+		tmp_other1["resp_max"] = F64(123.2892);
+		tmp_other1["resp_min"] = F64(23.2892);
+		tmp_other1["resp_mean"] = F64(58.28298);
+
+		LLSD & tmp_other2 = reg2_stats["get_other"];
+		tmp_other2["enqueued"] = 8;
+		tmp_other2["dequeued"] = 7;
+		// tmp_other2["resp_count"] = 0;
+		tmp_other2["resp_max"] = F64(0);
+		tmp_other2["resp_min"] = F64(0);
+		tmp_other2["resp_mean"] = F64(0);
+		
+		{
+			LLSD src = LLSD::emptyMap();
+			LLSD dst = LLSD::emptyMap();
+
+			src["regions"][reg1_name] = reg1_stats;
+			src["duration"] = 24;
+			dst["regions"][reg1_name] = reg2_stats;
+			dst["duration"] = 36;
+
+			LLViewerAssetStats::mergeRegionsLLSD(src, dst);
+		
+			ensure_approximately_equals("dst minimum with undefined count does not contribute to merged minimum",
+										dst["regions"][reg1_name]["get_other"]["resp_min"].asReal(), F64(23.2892), 20);
+		}
+
+		{
+			LLSD src = LLSD::emptyMap();
+			LLSD dst = LLSD::emptyMap();
+
+			src["regions"][reg1_name] = reg2_stats;
+			src["duration"] = 24;
+			dst["regions"][reg1_name] = reg1_stats;
+			dst["duration"] = 36;
+
+			LLViewerAssetStats::mergeRegionsLLSD(src, dst);
+		
+			ensure_approximately_equals("src minimum with undefined count does not contribute to merged minimum",
+										dst["regions"][reg1_name]["get_other"]["resp_min"].asReal(), F64(23.2892), 20);
+		}
+	}
 }