From: Radoslaw Zarzynski Date: Sat, 3 Feb 2018 18:13:13 +0000 (+0100) Subject: core: make SubsystemMap more statical and optimize should_gather(). X-Git-Tag: v13.0.2~150^2~5 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d96bf57879431e8083618c894b9f5ca33afaa17c;p=ceph.git core: make SubsystemMap more statical and optimize should_gather(). Signed-off-by: Radoslaw Zarzynski --- diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 683ac3bc3a7..e399af3dce2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -451,7 +451,6 @@ set(libcommon_files common/types.cc common/iso_8601.cc log/Log.cc - log/SubsystemMap.cc mon/MonCap.cc mon/MonClient.cc mon/MonMap.cc diff --git a/src/common/config.cc b/src/common/config.cc index 86e92b36dce..e16db0fe322 100644 --- a/src/common/config.cc +++ b/src/common/config.cc @@ -78,8 +78,6 @@ md_config_t::md_config_t(bool is_daemon) : cluster(""), lock("md_config_t", true, false) { - init_subsys(); - // Load the compile-time list of Option into // a map so that we can resolve keys quickly. for (const auto &i : ceph_options) { @@ -176,17 +174,6 @@ void md_config_t::validate_schema() } } -void md_config_t::init_subsys() -{ -#define SUBSYS(name, log, gather) \ - subsys.add(ceph_subsys_##name, STRINGIFY(name), log, gather); -#define DEFAULT_SUBSYS(log, gather) \ - subsys.add(ceph_subsys_, "none", log, gather); -#include "common/subsys.h" -#undef SUBSYS -#undef DEFAULT_SUBSYS -} - md_config_t::~md_config_t() { } @@ -830,7 +817,7 @@ int md_config_t::set_val(const std::string &key, const char *val, // subsystems? if (strncmp(k.c_str(), "debug_", 6) == 0) { for (size_t o = 0; o < subsys.get_num(); o++) { - std::string as_option = "debug_" + subsys.get_name(o); + std::string as_option = std::string("debug_") + subsys.get_name(o); if (k == as_option) { int log, gather; int r = sscanf(v.c_str(), "%d/%d", &log, &gather); @@ -956,7 +943,7 @@ int md_config_t::_get_val(const std::string &key, char **buf, int len) const string k(ConfFile::normalize_key_name(key)); // subsys? for (size_t o = 0; o < subsys.get_num(); o++) { - std::string as_option = "debug_" + subsys.get_name(o); + std::string as_option = std::string("debug_") + subsys.get_name(o); if (k == as_option) { if (len == -1) { *buf = (char*)malloc(20); @@ -984,7 +971,7 @@ void md_config_t::get_all_keys(std::vector *keys) const { } } for (size_t i = 0; i < subsys.get_num(); ++i) { - keys->push_back("debug_" + subsys.get_name(i)); + keys->push_back(std::string("debug_") + subsys.get_name(i)); } } diff --git a/src/common/config.h b/src/common/config.h index 4a8475c177c..36970cf3046 100644 --- a/src/common/config.h +++ b/src/common/config.h @@ -230,8 +230,6 @@ private: void update_legacy_val(const Option &opt, md_config_t::member_ptr_t member); - void init_subsys(); - bool expand_meta(std::string &val, std::ostream *oss) const; diff --git a/src/common/subsys_types.h b/src/common/subsys_types.h index a7fffdaab5b..a87ea1a0f3a 100644 --- a/src/common/subsys_types.h +++ b/src/common/subsys_types.h @@ -15,6 +15,8 @@ #ifndef CEPH_SUBSYS_TYPES_H #define CEPH_SUBSYS_TYPES_H +#include + enum ceph_subsys_id_t { ceph_subsys_, // default #define SUBSYS(name, log, gather) \ @@ -30,5 +32,25 @@ constexpr static std::size_t ceph_subsys_get_num() { return static_cast(ceph_subsys_max); } +struct ceph_subsys_item_t { + const char* name; + uint8_t log_level; + uint8_t gather_level; +}; + +constexpr static std::array +ceph_subsys_get_as_array() { +#define SUBSYS(name, log, gather) \ + ceph_subsys_item_t{ #name, log, gather }, +#define DEFAULT_SUBSYS(log, gather) \ + ceph_subsys_item_t{ "none", log, gather }, + + return { +#include "common/subsys.h" + }; +#undef SUBSYS +#undef DEFAULT_SUBSYS +} + #endif // CEPH_SUBSYS_TYPES_H diff --git a/src/log/Log.cc b/src/log/Log.cc index 10b76c32a54..70c69881060 100644 --- a/src/log/Log.cc +++ b/src/log/Log.cc @@ -407,10 +407,8 @@ void Log::dump_recent() char buf[4096]; _log_message("--- logging levels ---", true); - for (vector::iterator p = m_subs->m_subsys.begin(); - p != m_subs->m_subsys.end(); - ++p) { - snprintf(buf, sizeof(buf), " %2d/%2d %s", p->log_level, p->gather_level, p->name.c_str()); + for (const auto& p : m_subs->m_subsys) { + snprintf(buf, sizeof(buf), " %2d/%2d %s", p.log_level, p.gather_level, p.name); _log_message(buf, true); } diff --git a/src/log/SubsystemMap.cc b/src/log/SubsystemMap.cc deleted file mode 100644 index a3d31aaa10f..00000000000 --- a/src/log/SubsystemMap.cc +++ /dev/null @@ -1,31 +0,0 @@ - -#include "SubsystemMap.h" - -namespace ceph { -namespace logging { - -void SubsystemMap::add(unsigned subsys, std::string name, int log, int gather) -{ - if (subsys >= m_subsys.size()) - m_subsys.resize(subsys + 1); - m_subsys[subsys].name = name; - m_subsys[subsys].log_level = log; - m_subsys[subsys].gather_level = gather; - if (name.length() > m_max_name_len) - m_max_name_len = name.length(); -} - -void SubsystemMap::set_log_level(unsigned subsys, int log) -{ - assert(subsys < m_subsys.size()); - m_subsys[subsys].log_level = log; -} - -void SubsystemMap::set_gather_level(unsigned subsys, int gather) -{ - assert(subsys < m_subsys.size()); - m_subsys[subsys].gather_level = gather; -} - -} -} diff --git a/src/log/SubsystemMap.h b/src/log/SubsystemMap.h index db03760b84a..c7740ccb0bd 100644 --- a/src/log/SubsystemMap.h +++ b/src/log/SubsystemMap.h @@ -6,6 +6,7 @@ #include #include +#include #include "common/subsys_types.h" @@ -14,62 +15,89 @@ namespace ceph { namespace logging { -struct Subsystem { - int log_level, gather_level; - std::string name; - - Subsystem() : log_level(0), gather_level(0) {} -}; - class SubsystemMap { - std::vector m_subsys; - unsigned m_max_name_len; + // Access to the current gathering levels must be *FAST* as they are + // read over and over from all places in the code (via should_gather() + // by i.e. dout). + std::array m_gather_levels; + + // The rest. Should be as small as possible to not unnecessarily + // enlarge md_config_t and spread it other elements across cache + // lines. Access can be slow. + std::vector m_subsys; + + // TODO(rzarzynski): knock this out with constexpr. + unsigned m_max_name_len = 0; friend class Log; public: - SubsystemMap() : m_max_name_len(0) {} + SubsystemMap() : m_max_name_len(0) { + constexpr auto s = ceph_subsys_get_as_array(); + m_subsys.reserve(s.size()); + + std::size_t i = 0; + for (const ceph_subsys_item_t& item : s) { + m_subsys.emplace_back(item); + m_gather_levels[i++] = std::max(item.log_level, item.gather_level); + + m_max_name_len = std::max(m_max_name_len, + static_cast(strlen(item.name))); + } + } - size_t get_num() const { - return m_subsys.size(); + constexpr static size_t get_num() { + return ceph_subsys_get_num(); } int get_max_subsys_len() const { return m_max_name_len; } - void add(unsigned subsys, std::string name, int log, int gather); - void set_log_level(unsigned subsys, int log); - void set_gather_level(unsigned subsys, int gather); - int get_log_level(unsigned subsys) const { - if (subsys >= m_subsys.size()) + if (subsys >= get_num()) subsys = 0; return m_subsys[subsys].log_level; } int get_gather_level(unsigned subsys) const { - if (subsys >= m_subsys.size()) + if (subsys >= get_num()) subsys = 0; return m_subsys[subsys].gather_level; } - const std::string& get_name(unsigned subsys) const { - if (subsys >= m_subsys.size()) + // TODO(rzarzynski): move to string_view? + constexpr const char* get_name(unsigned subsys) const { + if (subsys >= get_num()) subsys = 0; - return m_subsys[subsys].name; + return ceph_subsys_get_as_array()[subsys].name; } template bool should_gather(int level) { - static_assert(SubV < ceph_subsys_get_num(), "wrong subsystem ID"); - return level <= m_subsys[SubV].gather_level || - level <= m_subsys[SubV].log_level; + static_assert(SubV < get_num(), "wrong subsystem ID"); + return level <= m_gather_levels[SubV]; } bool should_gather(const unsigned sub, int level) { assert(sub < m_subsys.size()); - return level <= m_subsys[sub].gather_level || - level <= m_subsys[sub].log_level; + return level <= m_gather_levels[sub]; + } + + // TODO(rzarzynski): verify the -1 case + void set_log_level(unsigned subsys, uint8_t log) + { + assert(subsys < m_subsys.size()); + m_subsys[subsys].log_level = log; + m_gather_levels[subsys] = \ + std::max(log, m_subsys[subsys].gather_level); + } + + void set_gather_level(unsigned subsys, uint8_t gather) + { + assert(subsys < m_subsys.size()); + m_subsys[subsys].gather_level = gather; + m_gather_levels[subsys] = \ + std::max(m_subsys[subsys].log_level, gather); } }; diff --git a/src/log/test.cc b/src/log/test.cc index 1f869b110f3..b25ec41e3b7 100644 --- a/src/log/test.cc +++ b/src/log/test.cc @@ -16,10 +16,17 @@ using namespace ceph::logging; TEST(Log, Simple) { SubsystemMap subs; - subs.add(0, "none", 10, 10); - subs.add(1, "foosys", 20, 1); - subs.add(2, "bar", 20, 2); - subs.add(3, "baz", 10, 3); + subs.set_log_level(0, 10); + subs.set_gather_level(0, 10); + + subs.set_log_level(1, 20); + subs.set_gather_level(1, 1); + + subs.set_log_level(2, 20); + subs.set_gather_level(2, 2); + + subs.set_log_level(3, 10); + subs.set_gather_level(3, 3); Log log(&subs); log.start(); @@ -49,7 +56,8 @@ TEST(Log, Simple) TEST(Log, ReuseBad) { SubsystemMap subs; - subs.add(1, "foo", 1, 1); + subs.set_log_level(1, 1); + subs.set_gather_level(1, 1); Log log(&subs); log.start(); log.set_log_file("/tmp/foo"); @@ -80,7 +88,8 @@ int many = 10000; TEST(Log, ManyNoGather) { SubsystemMap subs; - subs.add(1, "foo", 1, 1); + subs.set_log_level(1, 1); + subs.set_gather_level(1, 1); Log log(&subs); log.start(); log.set_log_file("/tmp/big"); @@ -98,7 +107,8 @@ TEST(Log, ManyNoGather) TEST(Log, ManyGatherLog) { SubsystemMap subs; - subs.add(1, "foo", 20, 10); + subs.set_log_level(1, 20); + subs.set_gather_level(1, 10); Log log(&subs); log.start(); log.set_log_file("/tmp/big"); @@ -116,7 +126,8 @@ TEST(Log, ManyGatherLog) TEST(Log, ManyGatherLogStringAssign) { SubsystemMap subs; - subs.add(1, "foo", 20, 10); + subs.set_log_level(1, 20); + subs.set_gather_level(1, 10); Log log(&subs); log.start(); log.set_log_file("/tmp/big"); @@ -137,7 +148,8 @@ TEST(Log, ManyGatherLogStringAssign) TEST(Log, ManyGatherLogStringAssignWithReserve) { SubsystemMap subs; - subs.add(1, "foo", 20, 10); + subs.set_log_level(1, 20); + subs.set_gather_level(1, 10); Log log(&subs); log.start(); log.set_log_file("/tmp/big"); @@ -160,7 +172,8 @@ TEST(Log, ManyGatherLogStringAssignWithReserve) TEST(Log, ManyGatherLogPrebuf) { SubsystemMap subs; - subs.add(1, "foo", 20, 10); + subs.set_log_level(1, 20); + subs.set_gather_level(1, 10); Log log(&subs); log.start(); log.set_log_file("/tmp/big"); @@ -183,7 +196,8 @@ TEST(Log, ManyGatherLogPrebuf) TEST(Log, ManyGatherLogPrebufOverflow) { SubsystemMap subs; - subs.add(1, "foo", 20, 10); + subs.set_log_level(1, 20); + subs.set_gather_level(1, 10); Log log(&subs); log.start(); log.set_log_file("/tmp/big"); @@ -207,7 +221,8 @@ TEST(Log, ManyGatherLogPrebufOverflow) TEST(Log, ManyGather) { SubsystemMap subs; - subs.add(1, "foo", 20, 1); + subs.set_log_level(1, 20); + subs.set_gather_level(1, 1); Log log(&subs); log.start(); log.set_log_file("/tmp/big"); @@ -224,7 +239,8 @@ TEST(Log, ManyGather) void do_segv() { SubsystemMap subs; - subs.add(1, "foo", 20, 1); + subs.set_log_level(1, 20); + subs.set_gather_level(1, 1); Log log(&subs); log.start(); log.set_log_file("/tmp/big"); @@ -249,7 +265,8 @@ TEST(Log, InternalSegv) TEST(Log, LargeLog) { SubsystemMap subs; - subs.add(1, "foo", 20, 10); + subs.set_log_level(1, 20); + subs.set_gather_level(1, 10); Log log(&subs); log.start(); log.set_log_file("/tmp/big"); @@ -269,7 +286,8 @@ TEST(Log, LargeLog) TEST(Log, TimeSwitch) { SubsystemMap subs; - subs.add(1, "foo", 20, 10); + subs.set_log_level(1, 20); + subs.set_gather_level(1, 10); Log log(&subs); log.start(); log.set_log_file("/tmp/time_switch_log");