From 0e93faaaaaff38633a0dc0da04cba52c88b74ba3 Mon Sep 17 00:00:00 2001
From: Joel Rosdahl <joel@rosdahl.net>
Date: Fri, 11 Sep 2020 16:50:26 +0200
Subject: [PATCH] Add and use Statistics::increment function

---
 src/Statistics.cpp           | 14 ++++++++++++++
 src/Statistics.hpp           |  8 ++++++++
 src/ccache.cpp               |  8 ++++----
 src/compress.cpp             |  6 +++---
 src/stats.cpp                |  9 +++------
 unittest/test_Statistics.cpp | 17 +++++++++++++++++
 6 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/src/Statistics.cpp b/src/Statistics.cpp
index 25de0ec02..6b268ffb2 100644
--- a/src/Statistics.cpp
+++ b/src/Statistics.cpp
@@ -19,6 +19,7 @@
 #include "Statistics.hpp"
 
 #include "AtomicFile.hpp"
+#include "Lockfile.hpp"
 #include "Logging.hpp"
 #include "Util.hpp"
 #include "exceptions.hpp"
@@ -75,4 +76,17 @@ write(const std::string& path, const Counters& counters)
   }
 }
 
+optional<Counters>
+increment(const std::string& path, const Counters& updates)
+{
+  Lockfile lock(path);
+  if (!lock.acquired()) {
+    return nullopt;
+  }
+  auto counters = Statistics::read(path);
+  counters.increment(updates);
+  Statistics::write(path, counters);
+  return counters;
+}
+
 } // namespace Statistics
diff --git a/src/Statistics.hpp b/src/Statistics.hpp
index b517db120..0008da0f1 100644
--- a/src/Statistics.hpp
+++ b/src/Statistics.hpp
@@ -22,6 +22,8 @@
 
 #include "Counters.hpp"
 
+#include "third_party/nonstd/optional.hpp"
+
 #include <string>
 
 // Statistics fields in storage order.
@@ -71,4 +73,10 @@ Counters read(const std::string& path);
 // Write `counters` to `path`. No lock is acquired.
 void write(const std::string& path, const Counters& counters);
 
+// Acquire a lock, read counters from `path`, apply `updates`, write result to
+// `path` and release the lock. Returns the resulting counters or nullopt on
+// error (e.g. if the lock could not be acquired).
+nonstd::optional<Counters> increment(const std::string& path,
+                                     const Counters& updates);
+
 } // namespace Statistics
diff --git a/src/ccache.cpp b/src/ccache.cpp
index 2f6b2d7ce..7d3b39996 100644
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -760,10 +760,10 @@ update_manifest_file(Context& ctx)
       ctx.counter_updates.increment(Statistic::cache_size_kibibyte, size_delta);
       ctx.counter_updates.increment(Statistic::files_in_cache, files_delta);
     } else {
-      Counters counters;
-      counters.increment(Statistic::cache_size_kibibyte, size_delta);
-      counters.increment(Statistic::files_in_cache, files_delta);
-      stats_flush_to_file(ctx.config, ctx.manifest_stats_file(), counters);
+      Counters updates;
+      updates.increment(Statistic::cache_size_kibibyte, size_delta);
+      updates.increment(Statistic::files_in_cache, files_delta);
+      Statistics::increment(ctx.manifest_stats_file(), updates);
     }
   }
   MTR_END("manifest", "manifest_put");
diff --git a/src/compress.cpp b/src/compress.cpp
index 2d583631e..5bed4affd 100644
--- a/src/compress.cpp
+++ b/src/compress.cpp
@@ -204,9 +204,9 @@ recompress_file(Context& ctx,
   if (ctx.stats_file() == stats_file) {
     ctx.counter_updates.increment(Statistic::cache_size_kibibyte, size_delta);
   } else {
-    Counters counters;
-    counters.increment(Statistic::cache_size_kibibyte, size_delta);
-    stats_flush_to_file(ctx.config, stats_file, counters);
+    Counters updates;
+    updates.increment(Statistic::cache_size_kibibyte, size_delta);
+    Statistics::increment(stats_file, updates);
   }
 
   statistics.update(content_size, old_stat.size(), new_stat.size(), 0);
diff --git a/src/stats.cpp b/src/stats.cpp
index 29513dad5..392653523 100644
--- a/src/stats.cpp
+++ b/src/stats.cpp
@@ -411,10 +411,7 @@ void
 stats_add_cleanup(const std::string& dir, uint64_t count)
 {
   std::string statsfile = dir + "/stats";
-  Lockfile lock(statsfile);
-  if (lock.acquired()) {
-    Counters counters = Statistics::read(statsfile);
-    counters.increment(Statistic::cleanups_performed, count);
-    Statistics::write(statsfile, counters);
-  }
+  Counters updates;
+  updates.increment(Statistic::cleanups_performed, count);
+  Statistics::increment(statsfile, updates);
 }
diff --git a/unittest/test_Statistics.cpp b/unittest/test_Statistics.cpp
index a680ee20a..64cd1dca6 100644
--- a/unittest/test_Statistics.cpp
+++ b/unittest/test_Statistics.cpp
@@ -97,4 +97,21 @@ TEST_CASE("Write")
   }
 }
 
+TEST_CASE("Increment")
+{
+  TestContext test_context;
+
+  Util::write_file("test", "0 1 2 3 27 5\n");
+
+  Counters updates;
+  updates.set(Statistic::internal_error, 1);
+  updates.set(Statistic::cache_miss, 6);
+
+  Statistics::increment("test", updates);
+
+  Counters counters = Statistics::read("test");
+  CHECK(counters.get(Statistic::internal_error) == 4);
+  CHECK(counters.get(Statistic::cache_miss) == 33);
+}
+
 TEST_SUITE_END();
-- 
2.47.3