diff --git a/src/metric.cpp b/src/metric.cpp index eb455f2..ea934b0 100644 --- a/src/metric.cpp +++ b/src/metric.cpp @@ -105,8 +105,6 @@ template <> struct hash { namespace metric { -using AtomicWord = std::atomic; - // DESIGN: Store doubles in atomic for lock-free operations // - Preserves full IEEE 754 double precision (no truncation) // - Allows atomic load/store without locks @@ -154,9 +152,10 @@ struct Counter::State { friend struct Metric; }; -// Gauge: Global, can increase/decrease, multiple writers (uses atomic CAS) +// Gauge: Global, can increase/decrease, multiple writers (uses atomic CAS). +// TODO slow under contention. struct Gauge::State { - AtomicWord value; // Stores double as uint64_t bits, lock-free + std::atomic value; // Stores double as uint64_t bits, lock-free friend struct Metric; }; @@ -166,8 +165,10 @@ struct Histogram::State { thresholds; // Bucket boundaries (sorted, deduplicated, includes +Inf) std::vector counts; // Count per bucket - single writer, malloc // provides 16-byte alignment - AtomicWord sum; // Sum of observations (double stored as uint64_t bits) - AtomicWord observations; // Total observation count (uint64_t) + // TODO this should just be a double like in counter + std::atomic + sum; // Sum of observations (double stored as uint64_t bits) + std::atomic observations; // Total observation count (uint64_t) friend struct Metric; }; @@ -190,7 +191,10 @@ struct Metric { // Thread registration happens lazily when metrics are created } ~ThreadInit() { - // Clean up this thread's storage from all families + // TODO we need to accumulate this threads counts into a global. Otherwise + // it can go backwards when we destroy a thread. + + // Clean up this thread's storage from all families std::unique_lock _{mutex}; auto thread_id = std::this_thread::get_id(); @@ -375,6 +379,8 @@ update_histogram_buckets(const std::vector &thresholds, : "x"(updated_counts) // Input: xmm register : "memory" // Memory clobber ); + // We don't actually need it to be atomic across 128-bits, but that's + // sufficient to guarantee each 64 bit half is atomic. } // Handle remainder with atomic stores