Refactor profiler and related classes

src/client/game.cpp

@@ -1132,9 +1132,11 @@ void Game::run()
 	FpsControl draw_times;
 	f32 dtime; // in seconds
 
-	/* Clear the profiler */
-	Profiler::GraphValues dummyvalues;
-	g_profiler->graphGet(dummyvalues);
+	// Clear the profiler
+	{
+		Profiler::GraphValues dummyvalues;
+		g_profiler->graphPop(dummyvalues);
+	}
 
 	draw_times.reset();
 
@@ -4229,7 +4231,7 @@ void Game::updateClouds(float dtime)
 inline void Game::updateProfilerGraphs(ProfilerGraph *graph)
 {
 	Profiler::GraphValues values;
-	g_profiler->graphGet(values);
+	g_profiler->graphPop(values);
 	graph->put(values);
 }
 

src/gettime.h

@@ -23,14 +23,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <string>
 #include <mutex>
 
-enum TimePrecision
-{
-	PRECISION_SECONDS,
-	PRECISION_MILLI,
-	PRECISION_MICRO,
-	PRECISION_NANO
-};
-
 inline struct tm mt_localtime()
 {
 	// initialize the time zone on first invocation

src/porting.h

@@ -33,7 +33,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "irrlichttypes.h" // u64
 #include "debug.h"
 #include "constants.h"
-#include "gettime.h"
+#include "util/timetaker.h" // TimePrecision
 
 #ifdef _MSC_VER
 	#define SWPRINTF_CHARSTRING L"%S"

src/profiler.cpp

@@ -22,40 +22,37 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 static Profiler main_profiler;
 Profiler *g_profiler = &main_profiler;
-ScopeProfiler::ScopeProfiler(
-		Profiler *profiler, const std::string &name, ScopeProfilerType type) :
-		m_profiler(profiler),
-		m_name(name), m_type(type)
+
+ScopeProfiler::ScopeProfiler(Profiler *profiler, const std::string &name,
+		ScopeProfilerType type, TimePrecision prec) :
+	m_profiler(profiler),
+	m_name(name), m_type(type), m_precision(prec)
 {
-	m_name.append(" [ms]");
-	if (m_profiler)
-		m_timer = new TimeTaker(m_name, nullptr, PRECISION_MILLI);
+	m_name.append(" [").append(TimePrecision_units[prec]).append("]");
+	m_time1 = porting::getTime(prec);
 }
 
 ScopeProfiler::~ScopeProfiler()
 {
-	if (!m_timer)
+	if (!m_profiler)
 		return;
 
-	float duration_ms = m_timer->stop(true);
-	float duration = duration_ms;
-	if (m_profiler) {
-		switch (m_type) {
-		case SPT_ADD:
-			m_profiler->add(m_name, duration);
-			break;
-		case SPT_AVG:
-			m_profiler->avg(m_name, duration);
-			break;
-		case SPT_GRAPH_ADD:
-			m_profiler->graphAdd(m_name, duration);
-			break;
-		case SPT_MAX:
-			m_profiler->max(m_name, duration);
-			break;
-		}
+	float duration = porting::getTime(m_precision) - m_time1;
+
+	switch (m_type) {
+	case SPT_ADD:
+		m_profiler->add(m_name, duration);
+		break;
+	case SPT_AVG:
+		m_profiler->avg(m_name, duration);
+		break;
+	case SPT_GRAPH_ADD:
+		m_profiler->graphAdd(m_name, duration);
+		break;
+	case SPT_MAX:
+		m_profiler->max(m_name, duration);
+		break;
 	}
-	delete m_timer;
 }
 
 Profiler::Profiler()
@@ -66,92 +63,68 @@ Profiler::Profiler()
 void Profiler::add(const std::string &name, float value)
 {
 	MutexAutoLock lock(m_mutex);
-	{
-		/* No average shall have been used; mark add/max used as -2 */
-		std::map<std::string, int>::iterator n = m_avgcounts.find(name);
-		if (n == m_avgcounts.end()) {
-			m_avgcounts[name] = -2;
-		} else {
-			if (n->second == -1)
-				n->second = -2;
-			assert(n->second == -2);
-		}
-	}
-	{
-		std::map<std::string, float>::iterator n = m_data.find(name);
-		if (n == m_data.end())
-			m_data[name] = value;
-		else
-			n->second += value;
+
+	auto it = m_data.find(name);
+	if (it == m_data.end()) {
+		// mark with special value for checking
+		m_data.emplace(name, DataPair{value, -SPT_ADD});
+	} else {
+		assert(it->second.avgcount == -SPT_ADD);
+		it->second.value += value;
 	}
 }
 
 void Profiler::max(const std::string &name, float value)
 {
 	MutexAutoLock lock(m_mutex);
-	{
-		/* No average shall have been used; mark add/max used as -2 */
-		auto n = m_avgcounts.find(name);
-		if (n == m_avgcounts.end()) {
-			m_avgcounts[name] = -2;
-		} else {
-			if (n->second == -1)
-				n->second = -2;
-			assert(n->second == -2);
-		}
-	}
-	{
-		auto n = m_data.find(name);
-		if (n == m_data.end())
-			m_data[name] = value;
-		else if (value > n->second)
-			n->second = value;
+
+	auto it = m_data.find(name);
+	if (it == m_data.end()) {
+		// mark with special value for checking
+		m_data.emplace(name, DataPair{value, -SPT_MAX});
+	} else {
+		assert(it->second.avgcount == -SPT_MAX);
+		it->second.value = std::max(value, it->second.value);
 	}
 }
 
 void Profiler::avg(const std::string &name, float value)
 {
 	MutexAutoLock lock(m_mutex);
-	int &count = m_avgcounts[name];
 
-	assert(count != -2);
-	count = MYMAX(count, 0) + 1;
-	m_data[name] += value;
+	auto it = m_data.find(name);
+	if (it == m_data.end()) {
+		m_data.emplace(name, DataPair{value, 1});
+	} else {
+		assert(it->second.avgcount >= 1);
+		it->second.value += value;
+		it->second.avgcount++;
+	}
 }
 
 void Profiler::clear()
 {
 	MutexAutoLock lock(m_mutex);
-	for (auto &it : m_data) {
-		it.second = 0;
-	}
-	m_avgcounts.clear();
+	for (auto &it : m_data)
+		it.second = DataPair();
 	m_start_time = porting::getTimeMs();
 }
 
 float Profiler::getValue(const std::string &name) const
 {
-	auto numerator = m_data.find(name);
-	if (numerator == m_data.end())
-		return 0.f;
-
-	auto denominator = m_avgcounts.find(name);
-	if (denominator != m_avgcounts.end()) {
-		if (denominator->second >= 1)
-			return numerator->second / denominator->second;
-	}
-
-	return numerator->second;
+	auto it = m_data.find(name);
+	if (it == m_data.end())
+		return 0;
+	return it->second.getValue();
 }
 
 int Profiler::getAvgCount(const std::string &name) const
 {
-	auto n = m_avgcounts.find(name);
-
-	if (n != m_avgcounts.end() && n->second >= 1)
-		return n->second;
-
-	return 1;
+	auto it = m_data.find(name);
+	if (it == m_data.end())
+		return 1;
+	int denominator = it->second.avgcount;
+	return denominator >= 1 ? denominator : 1;
 }
 
 u64 Profiler::getElapsedMs() const
@@ -204,6 +177,6 @@ void Profiler::getPage(GraphValues &o, u32 page, u32 pagecount)
 			continue;
 		}
 
-		o[i.first] = i.second / getAvgCount(i.first);
+		o[i.first] = i.second.getValue();
 	}
 }

src/profiler.h

@@ -58,54 +58,71 @@ public:
 	void getPage(GraphValues &o, u32 page, u32 pagecount);
 
 
+	void graphSet(const std::string &id, float value)
+	{
+		MutexAutoLock lock(m_mutex);
+		m_graphvalues[id] = value;
+	}
 	void graphAdd(const std::string &id, float value)
 	{
 		MutexAutoLock lock(m_mutex);
-		std::map<std::string, float>::iterator i =
-				m_graphvalues.find(id);
-		if(i == m_graphvalues.end())
-			m_graphvalues[id] = value;
+		auto it = m_graphvalues.find(id);
+		if (it == m_graphvalues.end())
+			m_graphvalues.emplace(id, value);
 		else
-			i->second += value;
+			it->second += value;
 	}
-	void graphGet(GraphValues &result)
+	void graphPop(GraphValues &result)
 	{
 		MutexAutoLock lock(m_mutex);
-		result = m_graphvalues;
-		m_graphvalues.clear();
+		assert(result.empty());
+		std::swap(result, m_graphvalues);
 	}
 
 	void remove(const std::string& name)
 	{
 		MutexAutoLock lock(m_mutex);
-		m_avgcounts.erase(name);
 		m_data.erase(name);
 	}
 
 private:
+	struct DataPair {
+		float value = 0;
+		int avgcount = 0;
+
+		inline float getValue() const {
+			return avgcount >= 1 ? (value / avgcount) : value;
+		}
+	};
+
 	std::mutex m_mutex;
-	std::map<std::string, float> m_data;
-	std::map<std::string, int> m_avgcounts;
+	std::map<std::string, DataPair> m_data;
 	std::map<std::string, float> m_graphvalues;
 	u64 m_start_time;
 };
 
-enum ScopeProfilerType{
-	SPT_ADD,
+enum ScopeProfilerType : u8
+{
+	SPT_ADD = 1,
 	SPT_AVG,
 	SPT_GRAPH_ADD,
 	SPT_MAX
 };
 
+// Note: this class should be kept lightweight.
+
 class ScopeProfiler
 {
 public:
 	ScopeProfiler(Profiler *profiler, const std::string &name,
-			ScopeProfilerType type = SPT_ADD);
+			ScopeProfilerType type = SPT_ADD,
+			TimePrecision precision = PRECISION_MILLI);
 	~ScopeProfiler();
+
 private:
 	Profiler *m_profiler = nullptr;
 	std::string m_name;
-	TimeTaker *m_timer = nullptr;
-	enum ScopeProfilerType m_type;
+	u64 m_time1;
+	ScopeProfilerType m_type;
+	TimePrecision m_precision;
 };

src/util/timetaker.cpp

@@ -23,12 +23,9 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "log.h"
 #include <ostream>
 
-TimeTaker::TimeTaker(const std::string &name, u64 *result, TimePrecision prec)
+void TimeTaker::start()
 {
-	m_name = name;
-	m_result = result;
-	m_precision = prec;
-	m_time1 = porting::getTime(prec);
+	m_time1 = porting::getTime(m_precision);
 }
 
 u64 TimeTaker::stop(bool quiet)
@@ -39,15 +36,8 @@ u64 TimeTaker::stop(bool quiet)
 			(*m_result) += dtime;
 		} else {
 			if (!quiet) {
-				static const char* const units[] = {
-					"s"  /* PRECISION_SECONDS */,
-					"ms" /* PRECISION_MILLI */,
-					"us" /* PRECISION_MICRO */,
-					"ns" /* PRECISION_NANO */,
-				};
 				infostream << m_name << " took "
-				           << dtime << units[m_precision]
-					   << std::endl;
+					<< dtime << TimePrecision_units[m_precision] << std::endl;
 			}
 		}
 		m_running = false;

src/util/timetaker.h

@@ -19,18 +19,43 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #pragma once
 
+#include <string>
 #include "irrlichttypes.h"
-#include "gettime.h"
+
+enum TimePrecision : s8
+{
+	PRECISION_SECONDS,
+	PRECISION_MILLI,
+	PRECISION_MICRO,
+	PRECISION_NANO,
+};
+
+constexpr const char *TimePrecision_units[] = {
+	"s"  /* PRECISION_SECONDS */,
+	"ms" /* PRECISION_MILLI */,
+	"us" /* PRECISION_MICRO */,
+	"ns" /* PRECISION_NANO */,
+};
 
 /*
 	TimeTaker
 */
 
+// Note: this class should be kept lightweight
+
 class TimeTaker
 {
 public:
-	TimeTaker(const std::string &name, u64 *result=nullptr,
-		TimePrecision prec=PRECISION_MILLI);
+	TimeTaker(const std::string &name, u64 *result = nullptr,
+		TimePrecision prec = PRECISION_MILLI)
+	{
+		if (result)
+			m_result = result;
+		else
+			m_name = name;
+		m_precision = prec;
+		start();
+	}
 
 	~TimeTaker()
 	{
@@ -42,9 +67,11 @@ public:
 	u64 getTimerTime();
 
 private:
+	void start();
+
 	std::string m_name;
+	u64 *m_result = nullptr;
 	u64 m_time1;
 	bool m_running = true;
 	TimePrecision m_precision;
-	u64 *m_result = nullptr;
 };