This is an automated email from the ASF dual-hosted git repository. wwbmmm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push: new 1ff5f3fc Support generics for MultiDimension APIs (#3026) 1ff5f3fc is described below commit 1ff5f3fc5a210da66a8855aa6a355e598005cbd9 Author: Bright Chen <chenguangmin...@foxmail.com> AuthorDate: Wed Jul 23 10:14:05 2025 +0800 Support generics for MultiDimension APIs (#3026) * Support generics for MVariable api * Update document --- docs/cn/mbvar_c++.md | 87 ++++++++++- src/brpc/builtin/prometheus_metrics_service.cpp | 2 +- src/butil/containers/flat_map.h | 4 +- src/butil/containers/flat_map_inl.h | 1 - src/butil/strings/string_piece.h | 16 +++ src/bvar/multi_dimension.h | 113 +++++++++++---- src/bvar/multi_dimension_inl.h | 184 +++++++++++------------- src/bvar/mvariable.cpp | 45 +++--- src/bvar/mvariable.h | 33 +++-- src/bvar/variable.cpp | 2 +- test/CMakeLists.txt | 10 +- test/bvar_mvariable_unittest.cpp | 165 +++++++++++++++------ 12 files changed, 432 insertions(+), 230 deletions(-) diff --git a/docs/cn/mbvar_c++.md b/docs/cn/mbvar_c++.md index b7a0d417..8a81e714 100644 --- a/docs/cn/mbvar_c++.md +++ b/docs/cn/mbvar_c++.md @@ -331,11 +331,13 @@ size_t mbvar_list_exposed(std::vector<std::string>* names) { 多维度统计的实现,主要提供bvar的获取、列举等功能。 +为了兼容旧的用法,KeyType默认类型是std::list<std::string>。KeyType必须是(STL或者自定义)容器,value_type必须是std::string。 + ## constructor 有三个构造函数: ```c++ -template <typename T> +template <typename T, typename KeyType = std::list<std::string>> class MultiDimension : public MVariable { public: // 不建议使用 @@ -398,7 +400,7 @@ bvar::MultiDimension<bvar::Adder<int> > g_request_count("foo_bar", "request_coun ## stats ```c++ -template <typename T> +template <typename T, typename KeyType = std::list<std::string>> class MultiDimension : public MVariable { public: ... @@ -410,7 +412,15 @@ public: ``` ### get_stats -根据指定label获取对应的单维度统计项bvar +根据指定label获取对应的单维度统计项bvar。 + +get_stats除了支持(默认)std::list<std::string>参数类型,也支持自定义参数类型,满足以下条件: +1. (STL或者自定义)容器。 +2. K::value_type支持通过operator std::string()转换为std::string和通过operator butil::StringPiece()转换为butil::StringPiece。 +3. K::value_type支持和std::string进行比较。 + +推荐使用不需要分配内存的容器(例如,std::array、absl::InlinedVector)和不需要拷贝字符串的数据结构(例如,const char*、std::string_view、butil::StringPieces),可以提高性能。 + ```c++ #include <bvar/bvar.h> #include <bvar/multi_dimension.h> @@ -420,7 +430,8 @@ namespace bar { // 定义一个全局的多维度mbvar变量 bvar::MultiDimension<bvar::Adder<int> > g_request_count("request_count", {"idc", "method", "status"}); -int get_request_count(const std::list<std::string>& request_label) { +template <typename K> +int get_request_count(const K& request_label) { // 获取request_label对应的单维度bvar指针,比如:request_label = {"tc", "get", "200"} bvar::Adder<int> *request_adder = g_request_count.get_stats(request_label); // 判断指针非空 @@ -433,6 +444,70 @@ int get_request_count(const std::list<std::string>& request_label) { return request_adder->get_value(); } +std::list<std::string> request_label_list = {"tc", "get", "200"}; +int request_count = get_request_count(request_label_list); + +std::vector<std::string_view> request_label_list = {"tc", "get", "200"}; +int request_count = get_request_count(request_label_list); + +std::vector<butil::StringPiece> request_label_list = {"tc", "get", "200"}; +int request_count = get_request_count(request_label_list); + +class MyStringView { +public: + MyStringView() : _ptr(NULL), _len(0) {} + MyStringView(const char* str) + : _ptr(str), + _len(str == NULL ? 0 : strlen(str)) {} +#if __cplusplus >= 201703L + MyStringView(const std::string_view& str) + : _ptr(str.data()), _len(str.size()) {} +#endif // __cplusplus >= 201703L + MyStringView(const std::string& str) + : _ptr(str.data()), _len(str.size()) {} + MyStringView(const char* offset, size_t len) + : _ptr(offset), _len(len) {} + + const char* data() const { return _ptr; } + size_t size() const { return _len; } + + // Converts to `std::basic_string`. + explicit operator std::string() const { + if (NULL == _ptr) { + return {}; + } + return {_ptr, size()}; + } + + // Converts to butil::StringPiece. + explicit operator butil::StringPiece() const { + if (NULL == _ptr) { + return {}; + } + return {_ptr, size()}; + } + +private: + const char* _ptr; + size_t _len; +}; + +bool operator==(const MyStringView& x, const std::string& y) { + if (x.size() != y.size()) { + return false; + } + return butil::StringPiece::wordmemcmp(x.data(), y.data(), x.size()) == 0; +} +bool operator==(const std::string& x, const MyStringView& y) { + if (x.size() != y.size()) { + return false; + } + return butil::StringPiece::wordmemcmp(x.data(), y.data(), x.size()) == 0; +} + +std::vector<MyStringView> request_label_list = {"tc", "get", "200"}; +int request_count = get_request_count(request_label_list); + } // namespace bar } // namespace foo ``` @@ -462,7 +537,7 @@ public: size_t count_labels() const; }; -template <typename T> +template <typename T, typename KeyType = std::list<std::string>> class MultiDimension : public MVariable { public: ... @@ -536,7 +611,7 @@ size_t count_stats() { ## list ```c++ -template <typename T> +template <typename T, typename KeyType = std::list<std::string>> class MultiDimension : public MVariable { public: ... diff --git a/src/brpc/builtin/prometheus_metrics_service.cpp b/src/brpc/builtin/prometheus_metrics_service.cpp index 4c5dd590..4efc24b9 100644 --- a/src/brpc/builtin/prometheus_metrics_service.cpp +++ b/src/brpc/builtin/prometheus_metrics_service.cpp @@ -232,7 +232,7 @@ int DumpPrometheusMetricsToIOBuf(butil::IOBuf* output) { if (bvar::FLAGS_bvar_max_dump_multi_dimension_metric_number > 0) { PrometheusMetricsDumper dumper_md(&os, g_server_info_prefix); - const int ndump_md = bvar::MVariable::dump_exposed(&dumper_md, NULL); + const int ndump_md = bvar::MVariableBase::dump_exposed(&dumper_md, NULL); if (ndump_md < 0) { return -1; } diff --git a/src/butil/containers/flat_map.h b/src/butil/containers/flat_map.h index 9ae72db9..54981b81 100644 --- a/src/butil/containers/flat_map.h +++ b/src/butil/containers/flat_map.h @@ -199,12 +199,12 @@ public: // Remove |key| and all associated value // Returns: 1 on erased, 0 otherwise. template <typename K2, bool Multi = _Multi> - typename std::enable_if<!Multi, size_t >::type + typename std::enable_if<!Multi, size_t>::type erase(const K2& key, mapped_type* old_value = NULL); // For `_Multi=true'. // Returns: num of value on erased, 0 otherwise. template <typename K2, bool Multi = _Multi> - typename std::enable_if<Multi, size_t >::type + typename std::enable_if<Multi, size_t>::type erase(const K2& key, std::vector<mapped_type>* old_values = NULL); // Remove all items. Allocated spaces are NOT returned by system. diff --git a/src/butil/containers/flat_map_inl.h b/src/butil/containers/flat_map_inl.h index 3b82b9ec..13444b2e 100644 --- a/src/butil/containers/flat_map_inl.h +++ b/src/butil/containers/flat_map_inl.h @@ -592,7 +592,6 @@ typename std::enable_if<!Multi, _T&>::type FlatMap<_K, _T, _H, _E, _S, _A, _M>::operator[](const key_type& key) { const size_t index = flatmap_mod(_hashfn(key), _nbucket); Bucket& first_node = _buckets[index]; - // LOG(INFO) << "index=" << index; if (!first_node.is_valid()) { ++_size; if (_S) { diff --git a/src/butil/strings/string_piece.h b/src/butil/strings/string_piece.h index 0489b3cd..dbfd69e6 100644 --- a/src/butil/strings/string_piece.h +++ b/src/butil/strings/string_piece.h @@ -32,6 +32,9 @@ #include <iosfwd> #include <string> +#if __cplusplus >= 201703L +#include <string_view> +#endif // __cplusplus >= 201703L #include "butil/base_export.h" #include "butil/basictypes.h" @@ -182,6 +185,11 @@ template <typename STRING_TYPE> class BasicStringPiece { BasicStringPiece(const value_type* str) : ptr_(str), length_((str == NULL) ? 0 : STRING_TYPE::traits_type::length(str)) {} +#if __cplusplus >= 201703L + BasicStringPiece( + const std::basic_string_view<value_type, typename STRING_TYPE::traits_type>& str) + : ptr_(str.data()), length_(str.size()) {} +#endif // __cplusplus >= 201703L BasicStringPiece(const STRING_TYPE& str) : ptr_(str.data()), length_(str.size()) {} BasicStringPiece(const value_type* offset, size_type len) @@ -370,6 +378,14 @@ template <typename STRING_TYPE> class BasicStringPiece { return internal::substr(*this, pos, n); } + // Converts to `std::basic_string`. + explicit operator STRING_TYPE() const { + if (NULL == data()) { + return {}; + } + return STRING_TYPE(data(), size()); + } + protected: const value_type* ptr_; size_type length_; diff --git a/src/bvar/multi_dimension.h b/src/bvar/multi_dimension.h index cc249c5f..8eb55dc7 100644 --- a/src/bvar/multi_dimension.h +++ b/src/bvar/multi_dimension.h @@ -25,39 +25,54 @@ #include "butil/scoped_lock.h" // BAIDU_SCOPE_LOCK #include "butil/containers/doubly_buffered_data.h" // DBD #include "butil/containers/flat_map.h" // butil::FlatMap +#include "butil/strings/string_piece.h" #include "bvar/mvariable.h" namespace bvar { -template <typename T> -class MultiDimension : public MVariable { +// KeyType requirements: +// 1. KeyType must be a container type with iterator, e.g. std::vector, std::list, std::set. +// 2. KeyType::value_type must be std::string. +// 3. KeyType::size() returns the number of labels. +// 4. KeyType::push_back() adds a label to the end of the container. +template <typename T, typename KeyType = std::list<std::string>> +class MultiDimension : public MVariable<KeyType> { public: - enum STATS_OP { READ_ONLY, READ_OR_INSERT, }; - typedef MVariable Base; - typedef std::list<std::string> key_type; + typedef KeyType key_type; typedef T value_type; typedef T* value_ptr_type; + typedef MVariable<key_type> Base; struct KeyHash { - size_t operator() (const key_type& key) const { + template <typename K> + size_t operator() (const K& key) const { size_t hash_value = 0; - for (auto &k : key) { - hash_value += std::hash<std::string>()(k); + for (auto& k : key) { + hash_value += BUTIL_HASH_NAMESPACE::hash<butil::StringPiece>()( + butil::StringPiece(k)); } return hash_value; } }; + + struct KeyEqualTo { + template <typename K> + bool operator()(const key_type& k1, const K& k2) const { + return k1.size() == k2.size() && + std::equal(k1.cbegin(), k1.cend(), k2.cbegin()); + } + }; typedef value_ptr_type op_value_type; - typedef typename butil::FlatMap<key_type, op_value_type, KeyHash> MetricMap; + typedef butil::FlatMap<key_type, op_value_type, KeyHash, KeyEqualTo> MetricMap; typedef typename MetricMap::const_iterator MetricMapConstIterator; - typedef typename butil::DoublyBufferedData<MetricMap> MetricMapDBD; + typedef butil::DoublyBufferedData<MetricMap> MetricMapDBD; typedef typename MetricMapDBD::ScopedPtr MetricMapScopedPtr; explicit MultiDimension(const key_type& labels); @@ -69,28 +84,39 @@ public: const butil::StringPiece& name, const key_type& labels); - ~MultiDimension(); + ~MultiDimension() override; // Implement this method to print the variable into ostream. - void describe(std::ostream& os); + void describe(std::ostream& os) override; - // Dump real bvar pointer - size_t dump(Dumper* dumper, const DumpOptions* options); + // Dump real bvar pointer + size_t dump(Dumper* dumper, const DumpOptions* options) override { + return dump_impl(dumper, options); + } // Get real bvar pointer object // Return real bvar pointer on success, NULL otherwise. - T* get_stats(const key_type& labels_value) { + // K requirements: + // 1. K must be a container type with iterator, + // e.g. std::vector, std::list, std::set, std::array. + // 2. K::value_type must be able to convert to std::string and butil::StringPiece + // through operator std::string() function and operator butil::StringPiece() function. + // 3. K::value_type must be able to compare with std::string. + template <typename K = key_type> + T* get_stats(const K& labels_value) { return get_stats_impl(labels_value, READ_OR_INSERT); } // Remove stat so those not count and dump - void delete_stats(const key_type& labels_value); + template <typename K = key_type> + void delete_stats(const K& labels_value); // Remove all stat void clear_stats(); // True if bvar pointer exists - bool has_stats(const key_type& labels_value); + template <typename K = key_type> + bool has_stats(const K& labels_value); // Get number of stats size_t count_stats(); @@ -102,33 +128,61 @@ public: // Get real bvar pointer object // Return real bvar pointer if labels_name exist, NULL otherwise. // CAUTION!!! Just For Debug!!! - T* get_stats_read_only(const key_type& labels_value) { + template <typename K = key_type> + T* get_stats_read_only(const K& labels_value) { return get_stats_impl(labels_value); } // Get real bvar pointer object // Return real bvar pointer if labels_name exist, otherwise(not exist) create bvar pointer. // CAUTION!!! Just For Debug!!! - T* get_stats_read_or_insert(const key_type& labels_value, bool* do_write = NULL) { + template <typename K = key_type> + T* get_stats_read_or_insert(const K& labels_value, bool* do_write = NULL) { return get_stats_impl(labels_value, READ_OR_INSERT, do_write); } #endif private: - T* get_stats_impl(const key_type& labels_value); + template <typename K> + T* get_stats_impl(const K& labels_value); + + template <typename K> + T* get_stats_impl(const K& labels_value, STATS_OP stats_op, bool* do_write = NULL); + + template <typename K> + static typename std::enable_if<butil::is_same<K, key_type>::value>::type + insert_metrics_map(MetricMap& bg, const K& labels_value, op_value_type metric) { + bg.insert(labels_value, metric); + } + + template <typename K> + static typename std::enable_if<!butil::is_same<K, key_type>::value>::type + insert_metrics_map(MetricMap& bg, const K& labels_value, op_value_type metric) { + key_type labels_value_str; + for (auto& label : labels_value) { + // key_type::value_type must be able to convert to std::string. + labels_value_str.push_back(std::string(label)); + } + bg.insert(labels_value_str, metric); + } - T* get_stats_impl(const key_type& labels_value, STATS_OP stats_op, bool* do_write = NULL); + template <typename U = T> + typename std::enable_if<!butil::is_same<LatencyRecorder, U>::value, size_t>::type + dump_impl(Dumper* dumper, const DumpOptions* options); - void make_dump_key(std::ostream& os, - const key_type& labels_value, - const std::string& suffix = "", - const int quantile = 0); + template <typename U = T> + typename std::enable_if<butil::is_same<LatencyRecorder, U>::value, size_t>::type + dump_impl(Dumper* dumper, const DumpOptions* options); - void make_labels_kvpair_string(std::ostream& os, - const key_type& labels_value, - const int quantile); + void make_dump_key(std::ostream& os, const key_type& labels_value, + const std::string& suffix = "", int quantile = 0); - bool is_valid_lables_value(const key_type& labels_value) const; + void make_labels_kvpair_string( + std::ostream& os, const key_type& labels_value, int quantile); + + + template <typename K> + bool is_valid_lables_value(const K& labels_value) const; // Remove all stats so those not count and dump void delete_stats(); @@ -139,7 +193,6 @@ private: static size_t init_flatmap(MetricMap& bg); -private: size_t _max_stats_count; MetricMapDBD _metric_map; }; diff --git a/src/bvar/multi_dimension_inl.h b/src/bvar/multi_dimension_inl.h index b7ef2b2d..15378248 100644 --- a/src/bvar/multi_dimension_inl.h +++ b/src/bvar/multi_dimension_inl.h @@ -34,51 +34,43 @@ static const std::string ALLOW_UNUSED METRIC_TYPE_SUMMARY = "summary"; static const std::string ALLOW_UNUSED METRIC_TYPE_HISTOGRAM = "histogram"; static const std::string ALLOW_UNUSED METRIC_TYPE_GAUGE = "gauge"; -template <typename T> -inline -MultiDimension<T>::MultiDimension(const key_type& labels) +template <typename T, typename KeyType> +MultiDimension<T, KeyType>::MultiDimension(const key_type& labels) : Base(labels) - , _max_stats_count(FLAGS_max_multi_dimension_stats_count) -{ + , _max_stats_count(FLAGS_max_multi_dimension_stats_count) { _metric_map.Modify(init_flatmap); } -template <typename T> -inline -MultiDimension<T>::MultiDimension(const butil::StringPiece& name, - const key_type& labels) - : MultiDimension(labels) -{ +template <typename T, typename KeyType> +MultiDimension<T, KeyType>::MultiDimension(const butil::StringPiece& name, + const key_type& labels) + : MultiDimension(labels) { this->expose(name); } -template <typename T> -inline -MultiDimension<T>::MultiDimension(const butil::StringPiece& prefix, - const butil::StringPiece& name, - const key_type& labels) - : MultiDimension(labels) -{ +template <typename T, typename KeyType> +MultiDimension<T, KeyType>::MultiDimension(const butil::StringPiece& prefix, + const butil::StringPiece& name, + const key_type& labels) + : MultiDimension(labels) { this->expose_as(prefix, name); } -template <typename T> -MultiDimension<T>::~MultiDimension() { - hide(); +template <typename T, typename KeyType> +MultiDimension<T, KeyType>::~MultiDimension() { + this->hide(); delete_stats(); } -template <typename T> -inline -size_t MultiDimension<T>::init_flatmap(MetricMap& bg) { +template <typename T, typename KeyType> +size_t MultiDimension<T, KeyType>::init_flatmap(MetricMap& bg) { // size = 1 << 13 CHECK_EQ(0, bg.init(8192, 80)); return (size_t)1; } -template <typename T> -inline -size_t MultiDimension<T>::count_stats() { +template <typename T, typename KeyType> +size_t MultiDimension<T, KeyType>::count_stats() { MetricMapScopedPtr metric_map_ptr; if (_metric_map.Read(&metric_map_ptr) != 0) { LOG(ERROR) << "Fail to read dbd"; @@ -87,21 +79,17 @@ size_t MultiDimension<T>::count_stats() { return metric_map_ptr->size(); } -template <typename T> -inline -void MultiDimension<T>::delete_stats(const key_type& labels_value) { +template <typename T, typename KeyType> +template <typename K> +void MultiDimension<T, KeyType>::delete_stats(const K& labels_value) { if (is_valid_lables_value(labels_value)) { - // Because there are two copies(foreground and background) in DBD, we need to use an empty tmp_metric, - // get the deleted value of second copy into tmp_metric, which can prevent the bvar object from being deleted twice. + // Because there are two copies(foreground and background) in DBD, + // we need to use an empty tmp_metric, get the deleted value of + // second copy into tmp_metric, which can prevent the bvar object + // from being deleted twice. op_value_type tmp_metric = NULL; auto erase_fn = [&labels_value, &tmp_metric](MetricMap& bg) { - auto it = bg.seek(labels_value); - if (it != NULL) { - tmp_metric = *it; - bg.erase(labels_value); - return 1; - } - return 0; + return bg.erase(labels_value, &tmp_metric); }; _metric_map.Modify(erase_fn); if (tmp_metric) { @@ -110,9 +98,8 @@ void MultiDimension<T>::delete_stats(const key_type& labels_value) { } } -template <typename T> -inline -void MultiDimension<T>::delete_stats() { +template <typename T, typename KeyType> +void MultiDimension<T, KeyType>::delete_stats() { // Because there are two copies(foreground and background) in DBD, we need to use an empty tmp_map, // swap two copies with empty, and get the value of second copy into tmp_map, // then traversal tmp_map and delete bvar object, @@ -133,9 +120,8 @@ void MultiDimension<T>::delete_stats() { } } -template <typename T> -inline -void MultiDimension<T>::list_stats(std::vector<key_type>* names) { +template <typename T, typename KeyType> +void MultiDimension<T, KeyType>::list_stats(std::vector<key_type>* names) { if (names == NULL) { return; } @@ -151,9 +137,9 @@ void MultiDimension<T>::list_stats(std::vector<key_type>* names) { } } -template <typename T> -inline -T* MultiDimension<T>::get_stats_impl(const key_type& labels_value) { +template <typename T, typename KeyType> +template <typename K> +T* MultiDimension<T, KeyType>::get_stats_impl(const K& labels_value) { if (!is_valid_lables_value(labels_value)) { return nullptr; } @@ -170,28 +156,30 @@ T* MultiDimension<T>::get_stats_impl(const key_type& labels_value) { return (*it); } -template <typename T> -inline -T* MultiDimension<T>::get_stats_impl(const key_type& labels_value, STATS_OP stats_op, bool* do_write) { +template <typename T, typename KeyType> +template <typename K> +T* MultiDimension<T, KeyType>::get_stats_impl( + const K& labels_value, STATS_OP stats_op, bool* do_write) { if (!is_valid_lables_value(labels_value)) { return nullptr; } { MetricMapScopedPtr metric_map_ptr; - if (_metric_map.Read(&metric_map_ptr) != 0) { + if (0 != _metric_map.Read(&metric_map_ptr)) { LOG(ERROR) << "Fail to read dbd"; return nullptr; } auto it = metric_map_ptr->seek(labels_value); - if (it != NULL) { + if (NULL != it) { return (*it); } else if (READ_ONLY == stats_op) { return nullptr; } if (metric_map_ptr->size() > _max_stats_count) { - LOG(ERROR) << "Too many stats seen, overflow detected, max stats count=" << _max_stats_count; + LOG(ERROR) << "Too many stats seen, overflow detected, max stats count=" + << _max_stats_count; return nullptr; } } @@ -209,37 +197,35 @@ T* MultiDimension<T>::get_stats_impl(const key_type& labels_value, STATS_OP stat if (do_write) { *do_write = true; } - if (NULL != cache_metric) { - bg.insert(labels_value, cache_metric); - } else { - T* add_metric = new T(); - bg.insert(labels_value, add_metric); - cache_metric = add_metric; + + if (NULL == cache_metric) { + cache_metric = new T(); } + insert_metrics_map(bg, labels_value, cache_metric); return 1; }; _metric_map.Modify(insert_fn); return cache_metric; } -template <typename T> -inline -void MultiDimension<T>::clear_stats() { +template <typename T, typename KeyType> +void MultiDimension<T, KeyType>::clear_stats() { delete_stats(); } -template <typename T> -inline -bool MultiDimension<T>::has_stats(const key_type& labels_value) { +template <typename T, typename KeyType> +template <typename K> +bool MultiDimension<T, KeyType>::has_stats(const K& labels_value) { return get_stats_impl(labels_value) != nullptr; } -template <typename T> -inline -size_t MultiDimension<T>::dump(Dumper* dumper, const DumpOptions* options) { +template <typename T, typename KeyType> +template <typename U> +typename std::enable_if<!butil::is_same<LatencyRecorder, U>::value, size_t>::type +MultiDimension<T, KeyType>::dump_impl(Dumper* dumper, const DumpOptions* options) { std::vector<key_type> label_names; list_stats(&label_names); - if (label_names.empty() || !dumper->dump_comment(name(), METRIC_TYPE_GAUGE)) { + if (label_names.empty() || !dumper->dump_comment(this->name(), METRIC_TYPE_GAUGE)) { return 0; } size_t n = 0; @@ -260,9 +246,10 @@ size_t MultiDimension<T>::dump(Dumper* dumper, const DumpOptions* options) { return n; } -template <> -inline -size_t MultiDimension<bvar::LatencyRecorder>::dump(Dumper* dumper, const DumpOptions*) { +template <typename T, typename KeyType> +template <typename U> +typename std::enable_if<butil::is_same<LatencyRecorder, U>::value, size_t>::type +MultiDimension<T, KeyType>::dump_impl(Dumper* dumper, const DumpOptions*) { std::vector<key_type> label_names; list_stats(&label_names); if (label_names.empty()) { @@ -272,7 +259,7 @@ size_t MultiDimension<bvar::LatencyRecorder>::dump(Dumper* dumper, const DumpOpt // To meet prometheus specification, we must guarantee no second TYPE line for one metric name // latency comment - dumper->dump_comment(name() + "_latency", METRIC_TYPE_GAUGE); + dumper->dump_comment(this->name() + "_latency", METRIC_TYPE_GAUGE); for (auto &label_name : label_names) { bvar::LatencyRecorder* bvar = get_stats_impl(label_name); if (!bvar) { @@ -310,7 +297,7 @@ size_t MultiDimension<bvar::LatencyRecorder>::dump(Dumper* dumper, const DumpOpt } // max_latency comment - dumper->dump_comment(name() + "_max_latency", METRIC_TYPE_GAUGE); + dumper->dump_comment(this->name() + "_max_latency", METRIC_TYPE_GAUGE); for (auto &label_name : label_names) { bvar::LatencyRecorder* bvar = get_stats_impl(label_name); if (!bvar) { @@ -324,7 +311,7 @@ size_t MultiDimension<bvar::LatencyRecorder>::dump(Dumper* dumper, const DumpOpt } // qps comment - dumper->dump_comment(name() + "_qps", METRIC_TYPE_GAUGE); + dumper->dump_comment(this->name() + "_qps", METRIC_TYPE_GAUGE); for (auto &label_name : label_names) { bvar::LatencyRecorder* bvar = get_stats_impl(label_name); if (!bvar) { @@ -338,7 +325,7 @@ size_t MultiDimension<bvar::LatencyRecorder>::dump(Dumper* dumper, const DumpOpt } // count comment - dumper->dump_comment(name() + "_count", METRIC_TYPE_COUNTER); + dumper->dump_comment(this->name() + "_count", METRIC_TYPE_COUNTER); for (auto &label_name : label_names) { bvar::LatencyRecorder* bvar = get_stats_impl(label_name); if (!bvar) { @@ -353,29 +340,24 @@ size_t MultiDimension<bvar::LatencyRecorder>::dump(Dumper* dumper, const DumpOpt return n; } -template <typename T> -inline -void MultiDimension<T>::make_dump_key(std::ostream& os, - const key_type& labels_value, - const std::string& suffix, - const int quantile) { - os << name(); +template <typename T, typename KeyType> +void MultiDimension<T, KeyType>::make_dump_key(std::ostream& os, const key_type& labels_value, + const std::string& suffix, int quantile) { + os << this->name(); if (!suffix.empty()) { os << suffix; } make_labels_kvpair_string(os, labels_value, quantile); } -template <typename T> -inline -void MultiDimension<T>::make_labels_kvpair_string(std::ostream& os, - const key_type& labels_value, - const int quantile) { +template <typename T, typename KeyType> +void MultiDimension<T, KeyType>::make_labels_kvpair_string( + std::ostream& os, const key_type& labels_value, int quantile) { os << "{"; - auto label_key = _labels.cbegin(); + auto label_key = this->_labels.cbegin(); auto label_value = labels_value.cbegin(); char comma[2] = {'\0', '\0'}; - for (; label_key != _labels.cend() && label_value != labels_value.cend(); + for (; label_key != this->_labels.cend() && label_value != labels_value.cend(); label_key++, label_value++) { os << comma << label_key->c_str() << "=\"" << label_value->c_str() << "\""; comma[0] = ','; @@ -386,22 +368,22 @@ void MultiDimension<T>::make_labels_kvpair_string(std::ostream& os, os << "}"; } -template <typename T> -inline -bool MultiDimension<T>::is_valid_lables_value(const key_type& labels_value) const { - if (count_labels() != labels_value.size()) { - LOG(ERROR) << "Invalid labels count"; +template <typename T, typename KeyType> +template <typename K> +bool MultiDimension<T, KeyType>::is_valid_lables_value(const K& labels_value) const { + if (this->count_labels() != labels_value.size()) { + LOG(ERROR) << "Invalid labels count" << this->count_labels() + << " != " << labels_value.size(); return false; } return true; } -template <typename T> -inline -void MultiDimension<T>::describe(std::ostream& os) { - os << "{\"name\" : \"" << _name << "\", \"labels\" : ["; +template <typename T, typename KeyType> +void MultiDimension<T, KeyType>::describe(std::ostream& os) { + os << "{\"name\" : \"" << this->name() << "\", \"labels\" : ["; char comma[3] = {'\0', ' ', '\0'}; - for (auto &label : _labels) { + for (auto& label : this->_labels) { os << comma << "\"" << label << "\""; comma[0] = ','; } diff --git a/src/bvar/mvariable.cpp b/src/bvar/mvariable.cpp index 26bb97c4..5503748b 100644 --- a/src/bvar/mvariable.cpp +++ b/src/bvar/mvariable.cpp @@ -30,8 +30,6 @@ namespace bvar { -constexpr uint64_t MAX_LABELS_COUNT = 10; - DECLARE_bool(bvar_abort_on_same_name); extern bool s_bvar_may_abort; @@ -75,7 +73,7 @@ class MVarEntry { public: MVarEntry() : var(NULL) {} - MVariable* var; + MVariableBase* var; }; typedef butil::FlatMap<std::string, MVarEntry> MVarMap; @@ -107,27 +105,18 @@ inline MVarMapWithLock& get_mvar_map() { return *s_mvar_map; } -MVariable::MVariable(const std::list<std::string>& labels) { - _labels.assign(labels.begin(), labels.end()); - size_t n = labels.size(); - if (n > MAX_LABELS_COUNT) { - LOG(ERROR) << "Too many labels: " << n << " seen, overflow detected, max labels count: " << MAX_LABELS_COUNT; - _labels.resize(MAX_LABELS_COUNT); - } -} - -MVariable::~MVariable() { - CHECK(!hide()) << "Subclass of MVariable MUST call hide() manually in their" - " dtors to avoid displaying a variable that is just destructing"; +MVariableBase::~MVariableBase() { + CHECK(!hide()) << "Subclass of MVariableBase MUST call hide() manually in their " + "dtors to avoid displaying a variable that is just destructing"; } -std::string MVariable::get_description() { +std::string MVariableBase::get_description() { std::ostringstream os; describe(os); return os.str(); } -int MVariable::describe_exposed(const std::string& name, +int MVariableBase::describe_exposed(const std::string& name, std::ostream& os) { MVarMapWithLock& m = get_mvar_map(); BAIDU_SCOPED_LOCK(m.mutex); @@ -139,7 +128,7 @@ int MVariable::describe_exposed(const std::string& name, return 0; } -std::string MVariable::describe_exposed(const std::string& name) { +std::string MVariableBase::describe_exposed(const std::string& name) { std::ostringstream oss; if (describe_exposed(name, oss) == 0) { return oss.str(); @@ -147,8 +136,8 @@ std::string MVariable::describe_exposed(const std::string& name) { return std::string(); } -int MVariable::expose_impl(const butil::StringPiece& prefix, - const butil::StringPiece& name) { +int MVariableBase::expose_impl(const butil::StringPiece& prefix, + const butil::StringPiece& name) { if (name.empty()) { LOG(ERROR) << "Parameter[name] is empty"; return -1; @@ -205,7 +194,7 @@ int MVariable::expose_impl(const butil::StringPiece& prefix, return 0; } -bool MVariable::hide() { +bool MVariableBase::hide() { if (_name.empty()) { return false; } @@ -223,20 +212,20 @@ bool MVariable::hide() { } #ifdef UNIT_TEST -void MVariable::hide_all() { +void MVariableBase::hide_all() { MVarMapWithLock& m = get_mvar_map(); BAIDU_SCOPED_LOCK(m.mutex); m.clear(); } #endif // end UNIT_TEST -size_t MVariable::count_exposed() { +size_t MVariableBase::count_exposed() { MVarMapWithLock& m = get_mvar_map(); BAIDU_SCOPED_LOCK(m.mutex); return m.size(); } -void MVariable::list_exposed(std::vector<std::string>* names) { +void MVariableBase::list_exposed(std::vector<std::string>* names) { if (names == NULL) { return; } @@ -251,7 +240,7 @@ void MVariable::list_exposed(std::vector<std::string>* names) { } } -size_t MVariable::dump_exposed(Dumper* dumper, const DumpOptions* options) { +size_t MVariableBase::dump_exposed(Dumper* dumper, const DumpOptions* options) { if (NULL == dumper) { LOG(ERROR) << "Parameter[dumper] is NULL"; return -1; @@ -271,10 +260,8 @@ size_t MVariable::dump_exposed(Dumper* dumper, const DumpOptions* options) { n += entry->var->dump(dumper, &opt); } if (n > static_cast<size_t>(FLAGS_bvar_max_dump_multi_dimension_metric_number)) { - LOG(WARNING) << "truncated because of \ - exceed max dump multi dimension label number[" - << FLAGS_bvar_max_dump_multi_dimension_metric_number - << "]"; + LOG(WARNING) << "truncated because of exceed max dump multi dimension label number[" + << FLAGS_bvar_max_dump_multi_dimension_metric_number << "]"; break; } } diff --git a/src/bvar/mvariable.h b/src/bvar/mvariable.h index 06f8a5d1..22719554 100644 --- a/src/bvar/mvariable.h +++ b/src/bvar/mvariable.h @@ -32,11 +32,11 @@ namespace bvar { class Dumper; struct DumpOptions; -class MVariable { +class MVariableBase { public: - explicit MVariable(const std::list<std::string>& labels); + MVariableBase() = default; - virtual ~MVariable(); + virtual ~MVariableBase(); // Implement this method to print the mvariable info into ostream. virtual void describe(std::ostream&) = 0; @@ -46,12 +46,6 @@ public: // Get mvariable name const std::string& name() const { return _name; } - - // Get mvariable labels - const std::list<std::string>& labels() const { return _labels; } - - // Get number of mvariable labels - size_t count_labels() const { return _labels.size(); } // Expose this mvariable globally so that it's counted in following // functions: @@ -113,11 +107,28 @@ protected: protected: std::string _name; - std::list<std::string> _labels; // mbvar uses bvar, bvar uses TLS, thus copying/assignment need to copy TLS stuff as well, // which is heavy. We disable copying/assignment now. - DISALLOW_COPY_AND_ASSIGN(MVariable); + DISALLOW_COPY_AND_ASSIGN(MVariableBase); +}; + +template <typename KeyType> +class MVariable : public MVariableBase { +public: + explicit MVariable(const KeyType& labels) : _labels(labels.cbegin(), labels.cend()) { + static_assert(std::is_same<typename KeyType::value_type, std::string>::value, + "value_type of KeyType must be std::string"); + } + + // Get mvariable labels + const KeyType& labels() const { return _labels; } + + // Get number of mvariable labels + size_t count_labels() const { return _labels.size(); } + +protected: + KeyType _labels; }; } // namespace bvar diff --git a/src/bvar/variable.cpp b/src/bvar/variable.cpp index 0b1e20d9..fe76b347 100644 --- a/src/bvar/variable.cpp +++ b/src/bvar/variable.cpp @@ -833,7 +833,7 @@ static void* dumping_thread(void*) { } else if ("prometheus" == mbvar_format) { dumper = new PrometheusFileDumper(mbvar_filename, mbvar_prefix); } - int nline = MVariable::dump_exposed(dumper, &options); + int nline = MVariableBase::dump_exposed(dumper, &options); if (nline < 0) { LOG(ERROR) << "Fail to dump mvars into " << filename; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 9f635e67..a478e8cc 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -188,10 +188,12 @@ endif() # bthread_* functions are used in logging.cc, and they need to be marked as # weak symbols explicitly in Darwin system. if(CMAKE_SYSTEM_NAME STREQUAL "Darwin") - set(DYNAMIC_LIB ${DYNAMIC_LIB} - "-Wl,-U,_bthread_getspecific" - "-Wl,-U,_bthread_setspecific" - "-Wl,-U,_bthread_key_create") + set(DYNAMIC_LIB ${DYNAMIC_LIB} "-Wl,-U,_bthread_getspecific" + "-Wl,-U,_bthread_setspecific" + "-Wl,-U,_bthread_key_create" + "-Wl,-U,_bthread_self" + "-Wl,-U,_bthread_timed_connect" + ) endif() add_library(BUTIL_DEBUG_LIB OBJECT ${BUTIL_SOURCES}) diff --git a/test/bvar_mvariable_unittest.cpp b/test/bvar_mvariable_unittest.cpp index 5ab171c4..d72a2e5d 100644 --- a/test/bvar_mvariable_unittest.cpp +++ b/test/bvar_mvariable_unittest.cpp @@ -17,12 +17,16 @@ // Date 2021/11/17 14:57:49 +// #if __cplusplus >= 201703L +#include <string_view> +// #endif // __cplusplus >= 201703L #include <pthread.h> // pthread_* #include <cstddef> #include <memory> #include <iostream> #include <set> #include <string> +#include <array> #include <gflags/gflags.h> #include <gtest/gtest.h> #include "butil/time.h" @@ -30,21 +34,8 @@ #include "bvar/bvar.h" #include "bvar/multi_dimension.h" -static const int num_thread = 24; - -static const int idc_count = 20; -static const int method_count = 20; -static const int status_count = 50; -static const int labels_count = idc_count * method_count * status_count; - static const std::list<std::string> labels = {"idc", "method", "status"}; -struct thread_perf_data { - bvar::MVariable* mbvar; - bvar::Variable* rbvar; - bvar::Variable* wbvar; -}; - class MVariableTest : public testing::Test { protected: void SetUp() {} @@ -66,9 +57,9 @@ TEST_F(MVariableTest, expose) { std::vector<std::string> list_exposed_vars; std::list<std::string> labels_value1 {"bj", "get", "200"}; bvar::MultiDimension<bvar::Adder<int> > my_madder1(labels); - ASSERT_EQ(0UL, bvar::MVariable::count_exposed()); + ASSERT_EQ(0UL, bvar::MVariableBase::count_exposed()); my_madder1.expose("request_count_madder"); - ASSERT_EQ(1UL, bvar::MVariable::count_exposed()); + ASSERT_EQ(1UL, bvar::MVariableBase::count_exposed()); bvar::Adder<int>* my_adder1 = my_madder1.get_stats(labels_value1); ASSERT_TRUE(my_adder1); ASSERT_STREQ("request_count_madder", my_madder1.name().c_str()); @@ -102,54 +93,140 @@ TEST_F(MVariableTest, expose) { list_exposed_vars.push_back("request_count_madder"); ASSERT_EQ(1UL, my_madder1.count_stats()); - ASSERT_EQ(1UL, bvar::MVariable::count_exposed()); + ASSERT_EQ(1UL, bvar::MVariableBase::count_exposed()); std::list<std::string> labels2 {"user", "url", "cost"}; bvar::MultiDimension<bvar::Adder<int> > my_madder2("client_url", labels2); - ASSERT_EQ(2UL, bvar::MVariable::count_exposed()); + ASSERT_EQ(2UL, bvar::MVariableBase::count_exposed()); list_exposed_vars.push_back("client_url"); std::list<std::string> labels3 {"product", "system", "module"}; bvar::MultiDimension<bvar::Adder<int> > my_madder3("request_from", labels3); list_exposed_vars.push_back("request_from"); - ASSERT_EQ(3UL, bvar::MVariable::count_exposed()); + ASSERT_EQ(3UL, bvar::MVariableBase::count_exposed()); std::vector<std::string> exposed_vars; - bvar::MVariable::list_exposed(&exposed_vars); + bvar::MVariableBase::list_exposed(&exposed_vars); ASSERT_EQ(3, exposed_vars.size()); my_madder3.hide(); - ASSERT_EQ(2UL, bvar::MVariable::count_exposed()); + ASSERT_EQ(2UL, bvar::MVariableBase::count_exposed()); list_exposed_vars.pop_back(); exposed_vars.clear(); - bvar::MVariable::list_exposed(&exposed_vars); + bvar::MVariableBase::list_exposed(&exposed_vars); ASSERT_EQ(2, exposed_vars.size()); } -TEST_F(MVariableTest, labels) { - std::list<std::string> labels_value1 {"bj", "get", "200"}; - bvar::MultiDimension<bvar::Adder<int> > my_madder1("request_count_madder", labels); - - ASSERT_EQ(labels.size(), my_madder1.count_labels()); - ASSERT_STREQ("request_count_madder", my_madder1.name().c_str()); +class MyStringView { +public: + MyStringView() : _ptr(NULL), _len(0) {} + MyStringView(const char* str) + : _ptr(str), + _len(str == NULL ? 0 : strlen(str)) {} +#if __cplusplus >= 201703L + MyStringView(const std::string_view& str) + : _ptr(str.data()), _len(str.size()) {} +#endif // __cplusplus >= 201703L + MyStringView(const std::string& str) + : _ptr(str.data()), _len(str.size()) {} + MyStringView(const char* offset, size_t len) + : _ptr(offset), _len(len) {} + + const char* data() const { return _ptr; } + size_t size() const { return _len; } + + // Converts to `std::basic_string`. + explicit operator std::string() const { + if (NULL == _ptr) { + return {}; + } + return {_ptr, size()}; + } - ASSERT_EQ(labels, my_madder1.labels()); - - std::list<std::string> labels_too_long; - std::list<std::string> labels_max; - int labels_too_long_count = 15; - for (int i = 0; i < labels_too_long_count; ++i) { - std::ostringstream os; - os << "label" << i; - labels_too_long.push_back(os.str()); - if (i < 10) { - labels_max.push_back(os.str()); + // Converts to butil::StringPiece. + explicit operator butil::StringPiece() const { + if (NULL == _ptr) { + return {}; } + return {_ptr, size()}; } - ASSERT_EQ(labels_too_long_count, labels_too_long.size()); - bvar::MultiDimension<bvar::Adder<int> > my_madder2("request_labels_too_long", labels_too_long); - ASSERT_EQ(10, my_madder2.count_labels()); - ASSERT_EQ(labels_max, my_madder2.labels()); + +private: + const char* _ptr; + size_t _len; +}; + +bool operator==(const MyStringView& x, const std::string& y) { + if (x.size() != y.size()) { + return false; + } + + return butil::StringPiece::wordmemcmp(x.data(), y.data(), x.size()) == 0; +} + +bool operator==(const std::string& x, const MyStringView& y) { + if (x.size() != y.size()) { + return false; + } + + return butil::StringPiece::wordmemcmp(x.data(), y.data(), x.size()) == 0; +} + +static int g_exposed_count = 0; + +template <typename KeyType, typename ValueType> +static void TestLabels() { + std::string mbvar_name = butil::string_printf("my_madder_%d", g_exposed_count); + KeyType labels{"idc", "method", "status"}; + bvar::MultiDimension<bvar::Adder<int>, KeyType> my_madder(mbvar_name, labels); + ASSERT_EQ(labels.size(), my_madder.count_labels()); + ASSERT_STREQ(mbvar_name.c_str(), my_madder.name().c_str()); + ASSERT_EQ(labels, my_madder.labels()); + + using ItemType = typename ValueType::value_type; + ValueType labels_value{ItemType("cv"), ItemType("post"), ItemType("200")}; + bvar::Adder<int>* adder = my_madder.get_stats(labels_value); + ASSERT_NE(nullptr, adder); + ASSERT_TRUE(my_madder.has_stats(labels_value)); + ASSERT_EQ((size_t)1, my_madder.count_stats()); + { + // Compatible with old API. + bvar::Adder<int>* temp = my_madder.get_stats({"cv", "post", "200"}); + ASSERT_EQ(adder, temp); + } + *adder << g_exposed_count; + ASSERT_EQ(g_exposed_count, adder->get_value()); + my_madder.delete_stats(labels_value); + ASSERT_FALSE(my_madder.has_stats(labels_value)); + ASSERT_EQ((size_t)0, my_madder.count_stats()); +} + +TEST_F(MVariableTest, labels) { + TestLabels<std::list<std::string>, std::list<std::string>>(); + TestLabels<std::list<std::string>, std::vector<std::string>>(); + TestLabels<std::list<std::string>, std::array<std::string, 3>>(); + + TestLabels<std::vector<std::string>, std::list<std::string>>(); + TestLabels<std::vector<std::string>, std::vector<std::string>>(); + TestLabels<std::vector<std::string>, std::array<std::string, 3>>(); + +#if __cplusplus >= 201703L + TestLabels<std::list<std::string>, std::list<std::string_view>>(); + TestLabels<std::list<std::string>, std::vector<std::string_view>>(); + TestLabels<std::list<std::string>, std::array<std::string_view, 3>>(); +#endif // __cplusplus >= 201703L + + TestLabels<std::vector<std::string>, std::list<butil::StringPiece>>(); + TestLabels<std::vector<std::string>, std::vector<butil::StringPiece>>(); + TestLabels<std::vector<std::string>, std::array<butil::StringPiece, 3>>(); + + TestLabels<std::list<std::string>, std::list<MyStringView>>(); + TestLabels<std::list<std::string>, std::vector<MyStringView>>(); + TestLabels<std::list<std::string>, std::array<MyStringView, 3>>(); + + TestLabels<std::vector<std::string>, std::list<MyStringView>>(); + TestLabels<std::vector<std::string>, std::vector<MyStringView>>(); + TestLabels<std::vector<std::string>, std::array<MyStringView, 3>>(); } TEST_F(MVariableTest, dump) { @@ -229,9 +306,9 @@ TEST_F(MVariableTest, test_describe_exposed) { std::string bvar_name("request_count_describe"); bvar::MultiDimension<bvar::Adder<int> > my_madder1(bvar_name, labels); - std::string describe_str = bvar::MVariable::describe_exposed(bvar_name); + std::string describe_str = bvar::MVariableBase::describe_exposed(bvar_name); std::ostringstream describe_oss; - ASSERT_EQ(0, bvar::MVariable::describe_exposed(bvar_name, describe_oss)); + ASSERT_EQ(0, bvar::MVariableBase::describe_exposed(bvar_name, describe_oss)); ASSERT_STREQ(describe_str.c_str(), describe_oss.str().c_str()); } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org