From: Joel Rosdahl <joel@rosdahl.net>
Date: Fri, 11 Sep 2020 11:09:36 +0000 (+0200)
Subject: Extract stats_{read,write} and Statistic enum to Statistics namespace
X-Git-Tag: v4.0~104
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=98db303ddcba36fa2dc9a8ce9e6441b53e2fbf9a;p=thirdparty%2Fccache.git

Extract stats_{read,write} and Statistic enum to Statistics namespace
---

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 92fcbc67b..88736d8cb 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -25,6 +25,7 @@ set(
   ResultRetriever.cpp
   SignalHandler.cpp
   Stat.cpp
+  Statistics.cpp
   TemporaryFile.cpp
   ThreadPool.cpp
   Util.cpp
diff --git a/src/Counters.cpp b/src/Counters.cpp
index 56603a164..986aacfab 100644
--- a/src/Counters.cpp
+++ b/src/Counters.cpp
@@ -18,7 +18,7 @@
 
 #include "Counters.hpp"
 
-#include "stats.hpp"
+#include "Statistics.hpp"
 
 #include <algorithm>
 
diff --git a/src/Result.cpp b/src/Result.cpp
index 96174f3ed..cbad746e1 100644
--- a/src/Result.cpp
+++ b/src/Result.cpp
@@ -27,9 +27,9 @@
 #include "File.hpp"
 #include "Logging.hpp"
 #include "Stat.hpp"
+#include "Statistics.hpp"
 #include "Util.hpp"
 #include "exceptions.hpp"
-#include "stats.hpp"
 
 #include <algorithm>
 
diff --git a/src/Statistics.cpp b/src/Statistics.cpp
new file mode 100644
index 000000000..25de0ec02
--- /dev/null
+++ b/src/Statistics.cpp
@@ -0,0 +1,78 @@
+// Copyright (C) 2020 Joel Rosdahl and other contributors
+//
+// See doc/AUTHORS.adoc for a complete list of contributors.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// this program; if not, write to the Free Software Foundation, Inc., 51
+// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+#include "Statistics.hpp"
+
+#include "AtomicFile.hpp"
+#include "Logging.hpp"
+#include "Util.hpp"
+#include "exceptions.hpp"
+
+using Logging::log;
+using nonstd::nullopt;
+using nonstd::optional;
+
+namespace Statistics {
+
+Counters
+read(const std::string& path)
+{
+  Counters counters;
+
+  std::string data;
+  try {
+    data = Util::read_file(path);
+  } catch (const Error&) {
+    // Ignore.
+    return counters;
+  }
+
+  size_t i = 0;
+  const char* str = data.c_str();
+  while (true) {
+    char* end;
+    const uint64_t value = std::strtoull(str, &end, 10);
+    if (end == str) {
+      break;
+    }
+    counters.set_raw(i, value);
+    ++i;
+    str = end;
+  }
+
+  return counters;
+}
+
+void
+write(const std::string& path, const Counters& counters)
+{
+  AtomicFile file(path, AtomicFile::Mode::text);
+  for (size_t i = 0; i < counters.size(); ++i) {
+    file.write(fmt::format("{}\n", counters.get_raw(i)));
+  }
+  try {
+    file.commit();
+  } catch (const Error& e) {
+    // Make failure to write a stats file a soft error since it's not important
+    // enough to fail whole the process and also because it is called in the
+    // Context destructor.
+    log("Error: {}", e.what());
+  }
+}
+
+} // namespace Statistics
diff --git a/src/Statistics.hpp b/src/Statistics.hpp
new file mode 100644
index 000000000..b517db120
--- /dev/null
+++ b/src/Statistics.hpp
@@ -0,0 +1,74 @@
+// Copyright (C) 2020 Joel Rosdahl and other contributors
+//
+// See doc/AUTHORS.adoc for a complete list of contributors.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// this program; if not, write to the Free Software Foundation, Inc., 51
+// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+#pragma once
+
+#include "system.hpp"
+
+#include "Counters.hpp"
+
+#include <string>
+
+// Statistics fields in storage order.
+enum class Statistic {
+  none = 0,
+  compiler_produced_stdout = 1,
+  compile_failed = 2,
+  internal_error = 3,
+  cache_miss = 4,
+  preprocessor_error = 5,
+  could_not_find_compiler = 6,
+  missing_cache_file = 7,
+  preprocessed_cache_hit = 8,
+  bad_compiler_arguments = 9,
+  called_for_link = 10,
+  files_in_cache = 11,
+  cache_size_kibibyte = 12,
+  obsolete_max_files = 13,
+  obsolete_max_size = 14,
+  unsupported_source_language = 15,
+  bad_output_file = 16,
+  no_input_file = 17,
+  multiple_source_files = 18,
+  autoconf_test = 19,
+  unsupported_compiler_option = 20,
+  output_to_stdout = 21,
+  direct_cache_hit = 22,
+  compiler_produced_no_output = 23,
+  compiler_produced_empty_output = 24,
+  error_hashing_extra_file = 25,
+  compiler_check_failed = 26,
+  could_not_use_precompiled_header = 27,
+  called_for_preprocessing = 28,
+  cleanups_performed = 29,
+  unsupported_code_directive = 30,
+  stats_zeroed_timestamp = 31,
+  could_not_use_modules = 32,
+
+  END
+};
+
+namespace Statistics {
+
+// Read counters from `path`. No lock is acquired.
+Counters read(const std::string& path);
+
+// Write `counters` to `path`. No lock is acquired.
+void write(const std::string& path, const Counters& counters);
+
+} // namespace Statistics
diff --git a/src/Util.hpp b/src/Util.hpp
index 834f9f74e..1c156b8e9 100644
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -32,6 +32,8 @@
 #include <utility>
 #include <vector>
 
+class Context;
+
 namespace Util {
 
 using DataReceiver = std::function<void(const void* data, size_t size)>;
diff --git a/src/argprocessing.hpp b/src/argprocessing.hpp
index cd36c7d0b..b8336fb08 100644
--- a/src/argprocessing.hpp
+++ b/src/argprocessing.hpp
@@ -19,7 +19,7 @@
 #pragma once
 
 #include "Args.hpp"
-#include "stats.hpp"
+#include "Statistics.hpp"
 
 #include "third_party/nonstd/optional.hpp"
 
diff --git a/src/compress.cpp b/src/compress.cpp
index 961faab76..2d583631e 100644
--- a/src/compress.cpp
+++ b/src/compress.cpp
@@ -26,6 +26,7 @@
 #include "Logging.hpp"
 #include "Manifest.hpp"
 #include "Result.hpp"
+#include "Statistics.hpp"
 #include "StdMakeUnique.hpp"
 #include "ThreadPool.hpp"
 #include "ZstdCompressor.hpp"
diff --git a/src/exceptions.hpp b/src/exceptions.hpp
index a6a6e670b..1c731414b 100644
--- a/src/exceptions.hpp
+++ b/src/exceptions.hpp
@@ -21,7 +21,7 @@
 #include "system.hpp"
 
 #include "FormatNonstdStringView.hpp"
-#include "stats.hpp"
+#include "Statistics.hpp"
 
 #include "third_party/fmt/core.h"
 #include "third_party/nonstd/optional.hpp"
diff --git a/src/hashutil.cpp b/src/hashutil.cpp
index 67f6a5b2f..1ecc3ab40 100644
--- a/src/hashutil.cpp
+++ b/src/hashutil.cpp
@@ -27,7 +27,6 @@
 #include "ccache.hpp"
 #include "execute.hpp"
 #include "macroskip.hpp"
-#include "stats.hpp"
 
 #ifdef INODE_CACHE_SUPPORTED
 #  include "InodeCache.hpp"
diff --git a/src/stats.cpp b/src/stats.cpp
index 5e39f10bb..29513dad5 100644
--- a/src/stats.cpp
+++ b/src/stats.cpp
@@ -27,10 +27,12 @@
 #include "Counters.hpp"
 #include "Lockfile.hpp"
 #include "Logging.hpp"
+#include "Statistics.hpp"
 #include "cleanup.hpp"
 #include "hashutil.hpp"
 
 #include "third_party/fmt/core.h"
+#include "third_party/nonstd/optional.hpp"
 
 #include <cmath>
 
@@ -149,42 +151,6 @@ format_timestamp(uint64_t timestamp)
   }
 }
 
-// Parse a stats file from a buffer, adding to the counters.
-static void
-parse_stats(Counters& counters, const std::string& buf)
-{
-  size_t i = 0;
-  const char* p = buf.c_str();
-  while (true) {
-    char* p2;
-    unsigned long long val = std::strtoull(p, &p2, 10);
-    if (p2 == p) {
-      break;
-    }
-    counters.increment(static_cast<Statistic>(i), val);
-    i++;
-    p = p2;
-  }
-}
-
-// Write out a stats file.
-void
-stats_write(const std::string& path, const Counters& counters)
-{
-  AtomicFile file(path, AtomicFile::Mode::text);
-  for (size_t i = 0; i < counters.size(); ++i) {
-    file.write(fmt::format("{}\n", counters.get_raw(i)));
-  }
-  try {
-    file.commit();
-  } catch (const Error& e) {
-    // Make failure to write a stats file a soft error since it's not important
-    // enough to fail whole the process AND because it is called in the Context
-    // destructor.
-    log("Error: {}", e.what());
-  }
-}
-
 static double
 stats_hit_rate(const Counters& counters)
 {
@@ -214,30 +180,15 @@ stats_collect(const Config& config, Counters& counters, time_t* last_updated)
     }
 
     counters.set(Statistic::stats_zeroed_timestamp, 0); // Don't add
-    stats_read(fname, counters);
+    counters.increment(Statistics::read(fname));
     zero_timestamp =
       std::max(counters.get(Statistic::stats_zeroed_timestamp), zero_timestamp);
-    auto st = Stat::stat(fname);
-    if (st && st.mtime() > *last_updated) {
-      *last_updated = st.mtime();
-    }
+    *last_updated = std::max(*last_updated, Stat::stat(fname).mtime());
   }
 
   counters.set(Statistic::stats_zeroed_timestamp, zero_timestamp);
 }
 
-// Read in the stats from one directory and add to the counters.
-void
-stats_read(const std::string& path, Counters& counters)
-{
-  try {
-    std::string data = Util::read_file(path);
-    parse_stats(counters, data);
-  } catch (Error&) {
-    // Ignore.
-  }
-}
-
 // Write counter updates in updates to sfile.
 void
 stats_flush_to_file(const Config& config,
@@ -274,9 +225,9 @@ stats_flush_to_file(const Config& config,
       return;
     }
 
-    stats_read(sfile, counters);
+    counters = Statistics::read(sfile);
     counters.increment(updates);
-    stats_write(sfile, counters);
+    Statistics::write(sfile, counters);
   }
 
   std::string subdir(Util::dir_name(sfile));
@@ -407,7 +358,6 @@ stats_zero(const Context& ctx)
   time_t timestamp = time(nullptr);
 
   for (int dir = 0; dir <= 0xF; dir++) {
-    Counters counters;
     auto fname = fmt::format("{}/{:x}/stats", ctx.config.cache_dir(), dir);
     if (!Stat::stat(fname)) {
       // No point in trying to reset the stats file if it doesn't exist.
@@ -415,14 +365,14 @@ stats_zero(const Context& ctx)
     }
     Lockfile lock(fname);
     if (lock.acquired()) {
-      stats_read(fname, counters);
+      Counters counters = Statistics::read(fname);
       for (size_t i = 0; k_statistics_fields[i].message; i++) {
         if (!(k_statistics_fields[i].flags & FLAG_NOZERO)) {
           counters.set(k_statistics_fields[i].statistic, 0);
         }
       }
       counters.set(Statistic::stats_zeroed_timestamp, timestamp);
-      stats_write(fname, counters);
+      Statistics::write(fname, counters);
     }
   }
 }
@@ -436,9 +386,8 @@ stats_get_obsolete_limits(const std::string& dir,
   assert(maxfiles);
   assert(maxsize);
 
-  Counters counters;
   std::string sname = dir + "/stats";
-  stats_read(sname, counters);
+  Counters counters = Statistics::read(sname);
   *maxfiles = counters.get(Statistic::obsolete_max_files);
   *maxsize = counters.get(Statistic::obsolete_max_size) * 1024;
 }
@@ -447,14 +396,13 @@ stats_get_obsolete_limits(const std::string& dir,
 void
 stats_set_sizes(const std::string& dir, uint64_t num_files, uint64_t total_size)
 {
-  Counters counters;
   std::string statsfile = dir + "/stats";
   Lockfile lock(statsfile);
   if (lock.acquired()) {
-    stats_read(statsfile, counters);
+    Counters counters = Statistics::read(statsfile);
     counters.set(Statistic::files_in_cache, num_files);
     counters.set(Statistic::cache_size_kibibyte, total_size / 1024);
-    stats_write(statsfile, counters);
+    Statistics::write(statsfile, counters);
   }
 }
 
@@ -462,12 +410,11 @@ stats_set_sizes(const std::string& dir, uint64_t num_files, uint64_t total_size)
 void
 stats_add_cleanup(const std::string& dir, uint64_t count)
 {
-  Counters counters;
   std::string statsfile = dir + "/stats";
   Lockfile lock(statsfile);
   if (lock.acquired()) {
-    stats_read(statsfile, counters);
+    Counters counters = Statistics::read(statsfile);
     counters.increment(Statistic::cleanups_performed, count);
-    stats_write(statsfile, counters);
+    Statistics::write(statsfile, counters);
   }
 }
diff --git a/src/stats.hpp b/src/stats.hpp
index 67948db52..d8c1b813d 100644
--- a/src/stats.hpp
+++ b/src/stats.hpp
@@ -27,45 +27,6 @@
 class Config;
 class Context;
 
-// Statistics fields in storage order.
-enum class Statistic {
-  none = 0,
-  compiler_produced_stdout = 1,
-  compile_failed = 2,
-  internal_error = 3,
-  cache_miss = 4,
-  preprocessor_error = 5,
-  could_not_find_compiler = 6,
-  missing_cache_file = 7,
-  preprocessed_cache_hit = 8,
-  bad_compiler_arguments = 9,
-  called_for_link = 10,
-  files_in_cache = 11,
-  cache_size_kibibyte = 12,
-  obsolete_max_files = 13,
-  obsolete_max_size = 14,
-  unsupported_source_language = 15,
-  bad_output_file = 16,
-  no_input_file = 17,
-  multiple_source_files = 18,
-  autoconf_test = 19,
-  unsupported_compiler_option = 20,
-  output_to_stdout = 21,
-  direct_cache_hit = 22,
-  compiler_produced_no_output = 23,
-  compiler_produced_empty_output = 24,
-  error_hashing_extra_file = 25,
-  compiler_check_failed = 26,
-  could_not_use_precompiled_header = 27,
-  called_for_preprocessing = 28,
-  cleanups_performed = 29,
-  unsupported_code_directive = 30,
-  stats_zeroed_timestamp = 31,
-  could_not_use_modules = 32,
-
-  END
-};
-
 void stats_flush(Context& ctx);
 void stats_flush_to_file(const Config& config,
                          const std::string& sfile,
@@ -81,5 +42,3 @@ void stats_set_sizes(const std::string& dir,
                      uint64_t num_files,
                      uint64_t total_size);
 void stats_add_cleanup(const std::string& dir, uint64_t count);
-void stats_read(const std::string& path, Counters& counters);
-void stats_write(const std::string& path, const Counters& counters);
diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt
index 6ae54ffa2..aa1fa8f52 100644
--- a/unittest/CMakeLists.txt
+++ b/unittest/CMakeLists.txt
@@ -13,6 +13,7 @@ set(
   test_Lockfile.cpp
   test_NullCompression.cpp
   test_Stat.cpp
+  test_Statistics.cpp
   test_Util.cpp
   test_ZstdCompression.cpp
   test_argprocessing.cpp
diff --git a/unittest/test_Counters.cpp b/unittest/test_Counters.cpp
index 951856ad2..d2382af3d 100644
--- a/unittest/test_Counters.cpp
+++ b/unittest/test_Counters.cpp
@@ -17,7 +17,7 @@
 // Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
 #include "../src/Counters.hpp"
-#include "../src/stats.hpp"
+#include "../src/Statistics.hpp"
 #include "TestUtil.hpp"
 
 #include "third_party/doctest.h"
diff --git a/unittest/test_Statistics.cpp b/unittest/test_Statistics.cpp
new file mode 100644
index 000000000..a680ee20a
--- /dev/null
+++ b/unittest/test_Statistics.cpp
@@ -0,0 +1,100 @@
+// Copyright (C) 2011-2020 Joel Rosdahl and other contributors
+//
+// See doc/AUTHORS.adoc for a complete list of contributors.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// this program; if not, write to the Free Software Foundation, Inc., 51
+// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+#include "../src/Statistics.hpp"
+#include "../src/Util.hpp"
+#include "TestUtil.hpp"
+
+#include "third_party/doctest.h"
+
+using TestUtil::TestContext;
+
+TEST_SUITE_BEGIN("Statistics");
+
+TEST_CASE("Read nonexistent")
+{
+  TestContext test_context;
+
+  Counters counters = Statistics::read("test");
+
+  REQUIRE(counters.size() == static_cast<size_t>(Statistic::END));
+  CHECK(counters.get(Statistic::cache_miss) == 0);
+}
+
+TEST_CASE("Read bad")
+{
+  TestContext test_context;
+
+  Util::write_file("test", "bad 1 2 3 4 5\n");
+  Counters counters = Statistics::read("test");
+
+  REQUIRE(counters.size() == static_cast<size_t>(Statistic::END));
+  CHECK(counters.get(Statistic::cache_miss) == 0);
+}
+
+TEST_CASE("Read existing")
+{
+  TestContext test_context;
+
+  Util::write_file("test", "0 1 2 3 27 5\n");
+  Counters counters = Statistics::read("test");
+
+  REQUIRE(counters.size() == static_cast<size_t>(Statistic::END));
+  CHECK(counters.get(Statistic::cache_miss) == 27);
+  CHECK(counters.get(Statistic::could_not_use_modules) == 0);
+}
+
+TEST_CASE("Read future counters")
+{
+  TestContext test_context;
+
+  std::string content;
+  size_t count = static_cast<size_t>(Statistic::END) + 1;
+  for (size_t i = 0; i < count; ++i) {
+    content += fmt::format("{}\n", i);
+  }
+
+  Util::write_file("test", content);
+  Counters counters = Statistics::read("test");
+
+  REQUIRE(counters.size() == count);
+  for (size_t i = 0; i < count; ++i) {
+    CHECK(counters.get_raw(i) == i);
+  }
+}
+
+TEST_CASE("Write")
+{
+  TestContext test_context;
+
+  Counters counters;
+  size_t count = static_cast<size_t>(Statistic::END) + 1;
+  for (size_t i = 0; i < count; ++i) {
+    counters.set_raw(i, i);
+  }
+
+  Statistics::write("test", counters);
+  counters = Statistics::read("test");
+
+  REQUIRE(counters.size() == count);
+  for (size_t i = 0; i < count; ++i) {
+    CHECK(counters.get_raw(i) == i);
+  }
+}
+
+TEST_SUITE_END();
diff --git a/unittest/test_argprocessing.cpp b/unittest/test_argprocessing.cpp
index 547671463..30ef057ed 100644
--- a/unittest/test_argprocessing.cpp
+++ b/unittest/test_argprocessing.cpp
@@ -19,8 +19,8 @@
 #include "../src/Args.hpp"
 #include "../src/Config.hpp"
 #include "../src/Context.hpp"
+#include "../src/Statistics.hpp"
 #include "../src/Util.hpp"
-#include "../src/stats.hpp"
 #include "TestUtil.hpp"
 #include "argprocessing.hpp"