From 1c4f5d3359e5d2604ff04acc25e5289ded86505c Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 17 Aug 2018 21:57:07 +0300 Subject: [PATCH] Miscellaneous #2482 --- dbms/src/Common/CurrentThread.h | 2 +- dbms/src/Common/ProfileEvents.h | 3 ++- dbms/src/Common/TaskStatsInfoGetter.cpp | 15 ++++++++------- dbms/src/Common/TaskStatsInfoGetter.h | 6 ++---- dbms/src/Interpreters/tests/internal_iotop.cpp | 10 +--------- 5 files changed, 14 insertions(+), 22 deletions(-) diff --git a/dbms/src/Common/CurrentThread.h b/dbms/src/Common/CurrentThread.h index 0443a01be16..afde7ad8bf2 100644 --- a/dbms/src/Common/CurrentThread.h +++ b/dbms/src/Common/CurrentThread.h @@ -56,7 +56,7 @@ public: static void attachQueryContext(Context & query_context); /// You must call one of these methods when create a query child thread: - /// Add current thread to a group associated with thr thread group + /// Add current thread to a group associated with the thread group static void attachTo(const ThreadGroupStatusPtr & thread_group); /// Is useful for a ThreadPool tasks static void attachToIfDetached(const ThreadGroupStatusPtr & thread_group); diff --git a/dbms/src/Common/ProfileEvents.h b/dbms/src/Common/ProfileEvents.h index 7e79a500867..38d8a9df7b9 100644 --- a/dbms/src/Common/ProfileEvents.h +++ b/dbms/src/Common/ProfileEvents.h @@ -59,9 +59,10 @@ namespace ProfileEvents } while (current != nullptr); } + /// Every single value is fetched atomically, but not all values as a whole. Counters getPartiallyAtomicSnapshot() const; - /// Reset metrics and parent + /// Reset all counters to zero and reset parent. void reset(); /// Get parent (thread unsafe) diff --git a/dbms/src/Common/TaskStatsInfoGetter.cpp b/dbms/src/Common/TaskStatsInfoGetter.cpp index 9c7f2a224b5..ed42fdffb5d 100644 --- a/dbms/src/Common/TaskStatsInfoGetter.cpp +++ b/dbms/src/Common/TaskStatsInfoGetter.cpp @@ -174,7 +174,7 @@ bool TaskStatsInfoGetter::getStatImpl(int tid, ::taskstats & out_stats, bool thr { ::nlmsgerr * err = static_cast<::nlmsgerr *>(NLMSG_DATA(&msg)); if (throw_on_error) - throw Exception("Can't get Netlink response, error=" + std::to_string(err->error), ErrorCodes::NETLINK_ERROR); + throw Exception("Can't get Netlink response, error: " + std::to_string(err->error), ErrorCodes::NETLINK_ERROR); else return false; } @@ -198,16 +198,16 @@ bool TaskStatsInfoGetter::getStatImpl(int tid, ::taskstats & out_stats, bool thr { if (na->nla_type == TASKSTATS_TYPE_STATS) { - ::taskstats *ts = static_cast<::taskstats *>(NLA_DATA(na)); + ::taskstats * ts = static_cast<::taskstats *>(NLA_DATA(na)); out_stats = *ts; } len2 += NLA_ALIGN(na->nla_len); - na = reinterpret_cast<::nlattr *>((char *) na + len2); + na = reinterpret_cast<::nlattr *>(reinterpret_cast(na) + len2); } } - na = reinterpret_cast<::nlattr *>((char *) GENLMSG_DATA(&msg) + len); + na = reinterpret_cast<::nlattr *>(reinterpret_cast(GENLMSG_DATA(&msg)) + len); } return true; @@ -215,13 +215,13 @@ bool TaskStatsInfoGetter::getStatImpl(int tid, ::taskstats & out_stats, bool thr void TaskStatsInfoGetter::getStat(::taskstats & stat, int tid) { - tid = tid < 0 ? getDefaultTid() : tid; + tid = tid < 0 ? getDefaultTID() : tid; getStatImpl(tid, stat, true); } bool TaskStatsInfoGetter::tryGetStat(::taskstats & stat, int tid) { - tid = tid < 0 ? getDefaultTid() : tid; + tid = tid < 0 ? getDefaultTID() : tid; return getStatImpl(tid, stat, false); } @@ -233,10 +233,11 @@ TaskStatsInfoGetter::~TaskStatsInfoGetter() int TaskStatsInfoGetter::getCurrentTID() { + /// This call is always successful. - man gettid return static_cast(syscall(SYS_gettid)); } -int TaskStatsInfoGetter::getDefaultTid() +int TaskStatsInfoGetter::getDefaultTID() { if (default_tid < 0) default_tid = getCurrentTID(); diff --git a/dbms/src/Common/TaskStatsInfoGetter.h b/dbms/src/Common/TaskStatsInfoGetter.h index 7b0decd7571..c89194cf88a 100644 --- a/dbms/src/Common/TaskStatsInfoGetter.h +++ b/dbms/src/Common/TaskStatsInfoGetter.h @@ -10,11 +10,10 @@ namespace DB class Exception; -/// Get taskstat infor from OS kernel via Netlink protocol +/// Get taskstat info from OS kernel via Netlink protocol. class TaskStatsInfoGetter { public: - TaskStatsInfoGetter(); TaskStatsInfoGetter(const TaskStatsInfoGetter &) = delete; @@ -30,9 +29,8 @@ public: static bool checkProcessHasRequiredPermissions(); private: - /// Caches current thread tid to avoid extra sys calls - int getDefaultTid(); + int getDefaultTID(); int default_tid = -1; bool getStatImpl(int tid, ::taskstats & out_stats, bool throw_on_error = false); diff --git a/dbms/src/Interpreters/tests/internal_iotop.cpp b/dbms/src/Interpreters/tests/internal_iotop.cpp index 0a6785cb6ed..867f3845e8d 100644 --- a/dbms/src/Interpreters/tests/internal_iotop.cpp +++ b/dbms/src/Interpreters/tests/internal_iotop.cpp @@ -99,7 +99,7 @@ void do_io(size_t id) get_info.getStat(stat, tid); { std::lock_guard lock(mutex); - std::cerr << "#" << id << ", tid " << tid << ", step2\n" << stat << "\n"; + std::cerr << "#" << id << ", tid " << tid << ", step3\n" << stat << "\n"; } Poco::File(path_dst).remove(false); @@ -113,16 +113,12 @@ void test_perf() TaskStatsInfoGetter get_info; rusage rusage; - timespec ts; constexpr size_t num_samples = 1000000; { Stopwatch watch; for (size_t i = 0; i < num_samples; ++i) - { getrusage(RUSAGE_THREAD, &rusage); - clock_gettime(CLOCK_MONOTONIC, &ts); - } auto ms = watch.elapsedMilliseconds(); if (ms > 0) @@ -151,13 +147,9 @@ try ThreadPool pool(num_threads); for (size_t i = 0; i < num_threads; ++i) pool.schedule([i]() { do_io(i); }); - pool.wait(); - test_perf(); - - return 0; } catch (...)