From 8d2526176d85d3fb97f6042f23524cc488fc37bf Mon Sep 17 00:00:00 2001
From: Florian Schmaus <flow@cs.fau.de>
Date: Sun, 11 Apr 2021 11:46:06 +0200
Subject: [PATCH] Fix race condition in io::Stats
The workerStats std::vector was modified concurrently without being
synchronized. The Stats are constructed together with IoContext and
the worker IoContexts are constructed concurrently at the beginning of
the worker loop. And in the Stats constructor the workerStats
std::vector was modified by adding the Stats instance that is
currently being constructed to it.
Turns out, we don't need the workerStats data structure at all. We
simply provide the global and worker IoContexts to
io::Stats::printStats(). This results IMHO in a cleaner design of
printStats() since there is no longer a data structure called
workerStats, that in fact, not only holds the worker's stats. And
while we at it, we rename io::Stats::printWorkerStats() to
io::Stats::printsStats(), since it does, in fact, not only print the
worker stats, but more or less all related worker stats.
---
emper/Runtime.cpp | 2 +-
emper/io/IoContext.hpp | 2 ++
emper/io/Stats.cpp | 13 ++++++-------
emper/io/Stats.hpp | 16 ++++++++--------
4 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/emper/Runtime.cpp b/emper/Runtime.cpp
index 5c36d3b4..a49128b9 100644
--- a/emper/Runtime.cpp
+++ b/emper/Runtime.cpp
@@ -165,7 +165,7 @@ Runtime::~Runtime() {
// objects bound to the runtime's lifetime
if constexpr (emper::STATS) {
printLastRuntimeStats();
- emper::io::Stats::printWorkerStats();
+ emper::io::Stats::printStats(globalIo, ioContexts);
}
for (unsigned int i = 0; i < workerCount; ++i) {
diff --git a/emper/io/IoContext.hpp b/emper/io/IoContext.hpp
index 2f34771e..ee4d78eb 100644
--- a/emper/io/IoContext.hpp
+++ b/emper/io/IoContext.hpp
@@ -181,5 +181,7 @@ class IoContext : public Logger<LogSubsystem::IO> {
}
}
}
+
+ auto getStats() -> Stats & { return stats; };
};
} // namespace emper::io
diff --git a/emper/io/Stats.cpp b/emper/io/Stats.cpp
index ba4ffe4b..62b792c8 100644
--- a/emper/io/Stats.cpp
+++ b/emper/io/Stats.cpp
@@ -7,13 +7,12 @@
#include <string> // for operator<<, to_string
#include <utility> // for pair
+#include "io/IoContext.hpp"
#include "io/Operation.hpp" // for Operation, operator<<, Operation::RECV
#include "lib/math.hpp"
namespace emper::io {
-std::vector<Stats*> Stats::workerStats;
-
auto operator<<(std::ostream& os, const Stats::CompletionType& t) -> std::ostream& {
switch (t) {
case Stats::CompletionType::FullCompletion:
@@ -118,10 +117,9 @@ auto operator<<(std::ostream& os, const Stats& s) -> std::ostream& {
return os;
}
-void Stats::printWorkerStats() {
- auto* globalStats = workerStats.front();
- workerStats.erase(workerStats.begin());
- std::cout << *globalStats << std::endl;
+void Stats::printStats(IoContext* globalIoContext,
+ const std::vector<IoContext*>& workerIoContexts) {
+ std::cout << globalIoContext->getStats() << std::endl;
// Use a stats object to calculate the averages
Stats avgs;
@@ -130,7 +128,8 @@ void Stats::printWorkerStats() {
// calculate avgs
int i = 1;
- for (auto& stats : workerStats) {
+ for (auto* workerIoContext : workerIoContexts) {
+ auto* stats = &workerIoContext->getStats();
for (const auto& op_pair : stats->io_uring_completions) {
auto op = op_pair.first;
auto op_map = op_pair.second;
diff --git a/emper/io/Stats.hpp b/emper/io/Stats.hpp
index 776d247a..5114800e 100644
--- a/emper/io/Stats.hpp
+++ b/emper/io/Stats.hpp
@@ -24,6 +24,11 @@
#include "lib/math.hpp"
class Runtime; // lines 28-28
+namespace emper {
+namespace io {
+class IoContext;
+}
+} // namespace emper
namespace math = emper::lib::math;
@@ -52,8 +57,6 @@ class Stats {
int workerId = 0;
static_assert(sizeof(int) > sizeof(workerid_t));
- static std::vector<Stats*> workerStats;
-
enum CompletionType {
FullCompletion = 0,
ErrorCompletion,
@@ -125,13 +128,10 @@ class Stats {
unsigned reReapCount_max = 0;
boost::circular_buffer<unsigned> reReapCount_last;
- static void printWorkerStats();
+ static void printStats(IoContext* globalIoContext,
+ const std::vector<IoContext*>& workerIoContexts);
- Stats() : reReapCount_last(10) {
- if constexpr (emper::STATS) {
- workerStats.push_back(this);
- }
- }
+ Stats() : reReapCount_last(10) {}
void recordCompletion(const Future& f, const int32_t& res) {
record_completion(f.op, res, PartialCompletableFuture::DISABLE_PARTIAL_COMPLETION, f.len);
--
GitLab