From 5713fcd77089b8a5aa9e6a80461285fdcf7e5218 Mon Sep 17 00:00:00 2001
From: Zygo Blaxell <bees@furryterror.org>
Date: Sun, 22 Jan 2017 21:50:22 -0500
Subject: [PATCH] bees: clean up statistics class

Some whitespace fixes.  Remove some duplicate code.  Don't lock
two BeesStats objects in the - operator method.

Get the locking for T& at(const K&) right to avoid locking a mutex
recursively.  Make the non-const version of the function private.

Signed-off-by: Zygo Blaxell <bees@furryterror.org>
---
 src/bees.cc | 18 ++++++++++--------
 src/bees.h  |  2 +-
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/bees.cc b/src/bees.cc
index fd7d1de..1efaf78 100644
--- a/src/bees.cc
+++ b/src/bees.cc
@@ -86,11 +86,10 @@ thread_local string BeesNote::tl_name;
 BeesNote::~BeesNote()
 {
 	tl_next = m_prev;
+	unique_lock<mutex> lock(s_mutex);
 	if (tl_next) {
-		unique_lock<mutex> lock(s_mutex);
 		s_status[gettid()] = tl_next;
 	} else {
-		unique_lock<mutex> lock(s_mutex);
 		s_status.erase(gettid());
 	}
 }
@@ -203,11 +202,10 @@ template <class T>
 T&
 BeesStatTmpl<T>::at(string idx)
 {
-	unique_lock<mutex> lock(m_mutex);
-    if (!m_stats_map.count(idx)) {
+	if (!m_stats_map.count(idx)) {
 		m_stats_map[idx] = 0;
 	}
-    return m_stats_map[idx];
+	return m_stats_map[idx];
 }
 
 template <class T>
@@ -215,7 +213,8 @@ T
 BeesStatTmpl<T>::at(string idx) const
 {
 	unique_lock<mutex> lock(m_mutex);
-    return m_stats_map.at(idx);
+	auto rv = m_stats_map.at(idx);
+	return rv;
 }
 
 template <class T>
@@ -223,7 +222,7 @@ void
 BeesStatTmpl<T>::add_count(string idx, size_t amount)
 {
 	unique_lock<mutex> lock(m_mutex);
-    if (!m_stats_map.count(idx)) {
+	if (!m_stats_map.count(idx)) {
 		m_stats_map[idx] = 0;
 	}
 	m_stats_map.at(idx) += amount;
@@ -255,14 +254,17 @@ BeesStats
 BeesStats::operator-(const BeesStats &that) const
 {
 	if (&that == this) return BeesStats();
+
 	unique_lock<mutex> this_lock(m_mutex);
 	BeesStats this_copy;
 	this_copy.m_stats_map = m_stats_map;
+	this_lock.unlock();
+
 	unique_lock<mutex> that_lock(that.m_mutex);
 	BeesStats that_copy;
 	that_copy.m_stats_map = that.m_stats_map;
-	this_lock.unlock();
 	that_lock.unlock();
+
 	for (auto i : that.m_stats_map) {
 		if (i.second != 0) {
 			this_copy.at(i.first) -= i.second;
diff --git a/src/bees.h b/src/bees.h
index 0c68fca..8a17960 100644
--- a/src/bees.h
+++ b/src/bees.h
@@ -155,12 +155,12 @@ class BeesStatTmpl {
 	map<string, T>	m_stats_map;
 	mutable mutex	m_mutex;
 
+	T& at(string idx);
 public:
 	BeesStatTmpl() = default;
 	BeesStatTmpl(const BeesStatTmpl &that);
 	BeesStatTmpl &operator=(const BeesStatTmpl &that);
 	void add_count(string idx, size_t amount = 1);
-	T& at(string idx);
 	T at(string idx) const;
 
 friend ostream& operator<< <>(ostream &os, const BeesStatTmpl<T> &bs);