From c574aef4437253dcf0e6c1fed4c9cab1033b33f9 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Sun, 11 Jun 2017 19:43:02 +0200 Subject: [PATCH] Make the histograms more flexible. --- metrics.cpp | 98 +++++++++++++++++++++++++++++++----------- metrics.h | 47 ++++++++++++++------ x264_encoder.cpp | 14 ++---- x264_encoder.h | 6 +-- x264_speed_control.cpp | 9 ++-- x264_speed_control.h | 6 +-- 6 files changed, 120 insertions(+), 60 deletions(-) diff --git a/metrics.cpp b/metrics.cpp index 3b3d1eb..3468ed7 100644 --- a/metrics.cpp +++ b/metrics.cpp @@ -2,6 +2,7 @@ #include +#include #include #include @@ -57,17 +58,18 @@ void Metrics::add(const string &name, const vector> &labels types[name] = type; } -void Metrics::add_histogram(const string &name, const vector> &labels, atomic *first_bucket_location, atomic *sum_location, size_t num_elements) +void Metrics::add(const string &name, const vector> &labels, Histogram *location) { - Histogram histogram; - histogram.name = name; - histogram.labels = labels; - histogram.first_bucket_location = first_bucket_location; - histogram.sum_location = sum_location; - histogram.num_elements = num_elements; + Metric metric; + metric.data_type = DATA_TYPE_HISTOGRAM; + metric.name = name; + metric.labels = labels; + metric.location_histogram = location; lock_guard lock(mu); - histograms.push_back(histogram); + metrics.push_back(metric); + assert(types.count(name) == 0 || types[name] == TYPE_HISTOGRAM); + types[name] = TYPE_HISTOGRAM; } string Metrics::serialize() const @@ -80,6 +82,8 @@ string Metrics::serialize() const for (const auto &name_and_type : types) { if (name_and_type.second == TYPE_GAUGE) { ss << "# TYPE nageru_" << name_and_type.first << " gauge\n"; + } else if (name_and_type.second == TYPE_HISTOGRAM) { + ss << "# TYPE nageru_" << name_and_type.first << " histogram\n"; } } for (const Metric &metric : metrics) { @@ -87,28 +91,72 @@ string Metrics::serialize() const if (metric.data_type == DATA_TYPE_INT64) { ss << name << " " << metric.location_int64->load() << "\n"; - } else { + } else if (metric.data_type == DATA_TYPE_DOUBLE) { ss << name << " " << metric.location_double->load() << "\n"; + } else { + ss << metric.location_histogram->serialize(metric.name, metric.labels); } } - for (const Histogram &histogram : histograms) { - ss << "# TYPE nageru_" << histogram.name << " histogram\n"; - - int64_t count = 0; - for (size_t i = 0; i < histogram.num_elements; ++i) { - char buf[16]; - snprintf(buf, sizeof(buf), "%lu", i); - vector> labels = histogram.labels; - labels.emplace_back("le", buf); - - int64_t val = histogram.first_bucket_location[i].load(); - count += val; - ss << serialize_name(histogram.name + "_bucket", labels) << " " << count << "\n"; - } - ss << serialize_name(histogram.name + "_sum", histogram.labels) << " " << histogram.sum_location->load() << "\n"; - ss << serialize_name(histogram.name + "_count", histogram.labels) << " " << count << "\n"; + return ss.str(); +} + +void Histogram::init(const vector &bucket_vals) +{ + this->num_buckets = bucket_vals.size(); + buckets.reset(new Bucket[num_buckets]); + for (size_t i = 0; i < num_buckets; ++i) { + buckets[i].val = bucket_vals[i]; + } +} + +void Histogram::init_uniform(size_t num_buckets) +{ + this->num_buckets = num_buckets; + buckets.reset(new Bucket[num_buckets]); + for (size_t i = 0; i < num_buckets; ++i) { + buckets[i].val = i; } +} + +void Histogram::count_event(double val) +{ + Bucket ref_bucket; + ref_bucket.val = val; + auto it = lower_bound(buckets.get(), buckets.get() + num_buckets, ref_bucket, + [](const Bucket &a, const Bucket &b) { return a.val < b.val; }); + if (it == buckets.get() + num_buckets) { + ++count_after_last_bucket; + } else { + ++it->count; + } + // Non-atomic add, but that's fine, since there are no concurrent writers. + sum = sum + val; +} + +string Histogram::serialize(const string &name, const vector> &labels) const +{ + stringstream ss; + ss.imbue(locale("C")); + ss.precision(20); + + int64_t count = 0; + for (size_t bucket_idx = 0; bucket_idx < num_buckets; ++bucket_idx) { + stringstream le_ss; + le_ss.imbue(locale("C")); + le_ss.precision(20); + le_ss << buckets[bucket_idx].val; + vector> bucket_labels = labels; + bucket_labels.emplace_back("le", le_ss.str()); + + count += buckets[bucket_idx].count.load(); + ss << serialize_name(name + "_bucket", bucket_labels) << " " << count << "\n"; + } + + count += count_after_last_bucket.load(); + + ss << serialize_name(name + "_sum", labels) << " " << sum.load() << "\n"; + ss << serialize_name(name + "_count", labels) << " " << count << "\n"; return ss.str(); } diff --git a/metrics.h b/metrics.h index c1239a6..a570723 100644 --- a/metrics.h +++ b/metrics.h @@ -7,16 +7,20 @@ // which makes it quite unwieldy. Thus, we'll package our own for the time being. #include +#include +#include #include #include -#include #include +class Histogram; + class Metrics { public: enum Type { TYPE_COUNTER, TYPE_GAUGE, + TYPE_HISTOGRAM, // Internal use only. }; void add(const std::string &name, std::atomic *location, Type type = TYPE_COUNTER) @@ -29,11 +33,14 @@ public: add(name, {}, location, type); } + void add(const std::string &name, Histogram *location) + { + add(name, {}, location); + } + void add(const std::string &name, const std::vector> &labels, std::atomic *location, Type type = TYPE_COUNTER); void add(const std::string &name, const std::vector> &labels, std::atomic *location, Type type = TYPE_COUNTER); - - // Only integer histogram, ie. keys are 0..(N-1). - void add_histogram(const std::string &name, const std::vector> &labels, std::atomic *first_bucket_location, std::atomic *sum_location, size_t num_elements); + void add(const std::string &name, const std::vector> &labels, Histogram *location); std::string serialize() const; @@ -41,6 +48,7 @@ private: enum DataType { DATA_TYPE_INT64, DATA_TYPE_DOUBLE, + DATA_TYPE_HISTOGRAM, }; struct Metric { @@ -50,24 +58,37 @@ private: union { std::atomic *location_int64; std::atomic *location_double; + Histogram *location_histogram; }; }; - // TODO: This needs to be more general. - struct Histogram { - std::string name; - std::vector> labels; - std::atomic *first_bucket_location; - std::atomic *sum_location; - size_t num_elements; - }; - mutable std::mutex mu; std::map types; std::vector metrics; std::vector histograms; }; +class Histogram { +public: + void init(const std::vector &bucket_vals); + void init_uniform(size_t num_buckets); // Sets up buckets 0..(N-1). + void count_event(double val); + std::string serialize(const std::string &name, const std::vector> &labels) const; + +private: + // Bucket counts number of events where val[i - 1] < x <= val[i]. + // The end histogram ends up being made into a cumulative one, + // but that's not how we store it here. + struct Bucket { + double val; + std::atomic count{0}; + }; + std::unique_ptr buckets; + size_t num_buckets; + std::atomic sum{0.0}; + std::atomic count_after_last_bucket{0}; +}; + extern Metrics global_metrics; #endif // !defined(_METRICS_H) diff --git a/x264_encoder.cpp b/x264_encoder.cpp index 9d108a0..08f6c91 100644 --- a/x264_encoder.cpp +++ b/x264_encoder.cpp @@ -65,7 +65,9 @@ X264Encoder::X264Encoder(AVOutputFormat *oformat) global_metrics.add("x264_output_frames", {{ "type", "i" }}, &metric_x264_output_frames_i); global_metrics.add("x264_output_frames", {{ "type", "p" }}, &metric_x264_output_frames_p); global_metrics.add("x264_output_frames", {{ "type", "b" }}, &metric_x264_output_frames_b); - global_metrics.add_histogram("x264_crf", {}, metric_x264_crf, &metric_x264_crf_sum, crf_buckets); + + metric_x264_crf.init_uniform(50); + global_metrics.add("x264_crf", &metric_x264_crf); } X264Encoder::~X264Encoder() @@ -364,15 +366,7 @@ void X264Encoder::encode_frame(X264Encoder::QueuedFrame qf) ++metric_x264_output_frames_p; } - if (pic.prop.f_crf_avg <= 0.0) { - ++metric_x264_crf[0]; - } else if (pic.prop.f_crf_avg <= crf_buckets - 1) { - ++metric_x264_crf[int(floor(pic.prop.f_crf_avg))]; - } else { - // Just clamp; this isn't ideal, but at least the total count will be right. - ++metric_x264_crf[crf_buckets - 1]; - } - metric_x264_crf_sum = metric_x264_crf_sum + pic.prop.f_crf_avg; + metric_x264_crf.count_event(pic.prop.f_crf_avg); if (frames_being_encoded.count(pic.i_pts)) { ReceivedTimestamps received_ts = frames_being_encoded[pic.i_pts]; diff --git a/x264_encoder.h b/x264_encoder.h index 19ce1df..b8a3562 100644 --- a/x264_encoder.h +++ b/x264_encoder.h @@ -33,6 +33,7 @@ extern "C" { #include #include "defs.h" +#include "metrics.h" #include "print_latency.h" #include "x264_dynamic.h" @@ -122,10 +123,7 @@ private: std::atomic metric_x264_output_frames_i{0}; std::atomic metric_x264_output_frames_p{0}; std::atomic metric_x264_output_frames_b{0}; - - static constexpr size_t crf_buckets = 50; - std::atomic metric_x264_crf[crf_buckets]{{0}}; - std::atomic metric_x264_crf_sum{0.0}; + Histogram metric_x264_crf; }; #endif // !defined(_X264ENCODE_H) diff --git a/x264_speed_control.cpp b/x264_speed_control.cpp index c69d31e..530feb7 100644 --- a/x264_speed_control.cpp +++ b/x264_speed_control.cpp @@ -16,6 +16,8 @@ using namespace std; using namespace std::chrono; +#define SC_PRESETS 26 + X264SpeedControl::X264SpeedControl(x264_t *x264, float f_speed, int i_buffer_size, float f_buffer_init) : dyn(load_x264_for_bit_depth(global_flags.x264_bit_depth)), x264(x264), f_speed(f_speed) @@ -40,7 +42,8 @@ X264SpeedControl::X264SpeedControl(x264_t *x264, float f_speed, int i_buffer_siz metric_x264_speedcontrol_buffer_available_seconds = buffer_fill * 1e-6; metric_x264_speedcontrol_buffer_size_seconds = buffer_size * 1e-6; - global_metrics.add_histogram("x264_speedcontrol_preset_used_frames", {}, metric_x264_speedcontrol_preset_used_frames, &metric_x264_speedcontrol_preset_used_frames_sum, SC_PRESETS); + metric_x264_speedcontrol_preset_used_frames.init_uniform(SC_PRESETS); + global_metrics.add("x264_speedcontrol_preset_used_frames", &metric_x264_speedcontrol_preset_used_frames); global_metrics.add("x264_speedcontrol_buffer_available_seconds", &metric_x264_speedcontrol_buffer_available_seconds, Metrics::TYPE_GAUGE); global_metrics.add("x264_speedcontrol_buffer_size_seconds", &metric_x264_speedcontrol_buffer_size_seconds, Metrics::TYPE_GAUGE); global_metrics.add("x264_speedcontrol_idle_frames", &metric_x264_speedcontrol_idle_frames); @@ -337,7 +340,5 @@ void X264SpeedControl::apply_preset(int new_preset) dyn.x264_encoder_reconfig(x264, &p); preset = new_preset; - ++metric_x264_speedcontrol_preset_used_frames[new_preset]; - // Non-atomic add, but that's fine, since there are no concurrent writers. - metric_x264_speedcontrol_preset_used_frames_sum = metric_x264_speedcontrol_preset_used_frames_sum + new_preset; + metric_x264_speedcontrol_preset_used_frames.count_event(new_preset); } diff --git a/x264_speed_control.h b/x264_speed_control.h index 2a20414..bd58dcb 100644 --- a/x264_speed_control.h +++ b/x264_speed_control.h @@ -52,10 +52,9 @@ extern "C" { #include } +#include "metrics.h" #include "x264_dynamic.h" -#define SC_PRESETS 26 - class X264SpeedControl { public: // x264: Encoding object we are using; must be opened. Assumed to be @@ -132,8 +131,7 @@ private: std::function override_func = nullptr; // Metrics. - std::atomic metric_x264_speedcontrol_preset_used_frames[SC_PRESETS]{{0}}; - std::atomic metric_x264_speedcontrol_preset_used_frames_sum{0.0}; + Histogram metric_x264_speedcontrol_preset_used_frames; std::atomic metric_x264_speedcontrol_buffer_available_seconds{0.0}; std::atomic metric_x264_speedcontrol_buffer_size_seconds{0.0}; std::atomic metric_x264_speedcontrol_idle_frames{0}; -- 2.39.2