From 4a0fa579890a408c23e0c3eb1298aacdf74b816d Mon Sep 17 00:00:00 2001
From: Xiaoge Su <magichp@gmail.com>
Date: Thu, 12 Nov 2020 15:38:51 -0800
Subject: [PATCH] Add bytes_per_second unit in histograms

---
 fdbserver/storageserver.actor.cpp |  2 +-
 flow/Histogram.cpp                | 30 +++++++++++++++++++++++-------
 flow/Histogram.h                  |  8 +++++++-
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp
index 6c34fc7746..99776171fe 100644
--- a/fdbserver/storageserver.actor.cpp
+++ b/fdbserver/storageserver.actor.cpp
@@ -311,7 +311,7 @@ public:
 		    bytes(Histogram::getHistogram(STORAGESERVER_HISTOGRAM_GROUP, FETCH_KEYS_BYTES_HISTOGRAM,
 		                                  Histogram::Unit::bytes)),
 		    bandwidth(Histogram::getHistogram(STORAGESERVER_HISTOGRAM_GROUP, FETCH_KEYS_BYTES_PER_SECOND_HISTOGRAM,
-		                                      Histogram::Unit::bytes)) {}
+		                                      Histogram::Unit::bytes_per_second)) {}
 	} fetchKeysHistograms;
 
 	class CurrentRunningFetchKeys {
diff --git a/flow/Histogram.cpp b/flow/Histogram.cpp
index e5281093c9..4de14970ef 100644
--- a/flow/Histogram.cpp
+++ b/flow/Histogram.cpp
@@ -41,6 +41,8 @@ thread_local ISimulator::ProcessInfo* ISimulator::currentProcess = nullptr;
 // we have a simulated contex here; we'd just use the current context regardless.
 static HistogramRegistry* globalHistograms = nullptr;
 
+#pragma region HistogramRegistry
+
 HistogramRegistry& GetHistogramRegistry() {
 	ISimulator::ProcessInfo* info = g_simulator.getCurrentProcess();
 
@@ -89,6 +91,16 @@ void HistogramRegistry::logReport() {
 	}
 }
 
+#pragma endregion // HistogramRegistry
+
+#pragma region Histogram
+
+const std::unordered_map<Histogram::Unit, std::string> Histogram::UnitToStringMapper = {
+	{ Histogram::Unit::microseconds, "microseconds" },
+	{ Histogram::Unit::bytes, "bytes" },
+	{ Histogram::Unit::bytes_per_second, "bytes_per_second" }
+};
+
 void Histogram::writeToLog() {
 	bool active = false;
 	for (uint32_t i = 0; i < 32; i++) {
@@ -102,17 +114,19 @@ void Histogram::writeToLog() {
 	}
 
 	TraceEvent e(SevInfo, "Histogram");
-	e.detail("Group", group).detail("Op", op);
+	e.detail("Group", group).detail("Op", op).detail("Unit", UnitToStringMapper.at(unit));
+
 	for (uint32_t i = 0; i < 32; i++) {
+		uint32_t value = ((uint32_t)1) << (i + 1);
+
 		if (buckets[i]) {
 			switch (unit) {
-			case Unit::microseconds: {
-				uint32_t usec = ((uint32_t)1) << (i + 1);
-				e.detail(format("LessThan%u.%03u", usec / 1000, usec % 1000), buckets[i]);
+			case Unit::microseconds:
+				e.detail(format("LessThan%u.%03u", value / 1000, value % 1000), buckets[i]);
 				break;
-			}
 			case Unit::bytes:
-				e.detail(format("LessThan%u", ((uint32_t)1) << (i + 1)), buckets[i]);
+			case Unit::bytes_per_second:
+				e.detail(format("LessThan%u", value), buckets[i]);
 				break;
 			default:
 				ASSERT(false);
@@ -121,6 +135,8 @@ void Histogram::writeToLog() {
 	}
 }
 
+#pragma endregion // Histogram
+
 TEST_CASE("/flow/histogram/smoke_test") {
 
 	{
@@ -168,4 +184,4 @@ TEST_CASE("/flow/histogram/smoke_test") {
 	GetHistogramRegistry().logReport();
 
 	return Void();
-}
\ No newline at end of file
+}
diff --git a/flow/Histogram.h b/flow/Histogram.h
index fd765f4d86..7cef45b49f 100644
--- a/flow/Histogram.h
+++ b/flow/Histogram.h
@@ -26,6 +26,7 @@
 
 #include <string>
 #include <map>
+#include <unordered_map>
 
 #ifdef _WIN32
 #include <intrin.h>
@@ -57,11 +58,16 @@ HistogramRegistry& GetHistogramRegistry();
  */
 class Histogram sealed : public ReferenceCounted<Histogram> {
 public:
-	enum class Unit { microseconds, bytes };
+	enum class Unit { microseconds, bytes, bytes_per_second };
 
 private:
+	static const std::unordered_map<Unit, std::string> UnitToStringMapper;
+
 	Histogram(std::string group, std::string op, Unit unit, HistogramRegistry& registry)
 	  : group(group), op(op), unit(unit), registry(registry), ReferenceCounted<Histogram>() {
+
+		ASSERT(UnitToStringMapper.find(unit) != UnitToStringMapper.end());
+
 		clear();
 	}