github-actions[bot] commented on code in PR #38833: URL: https://github.com/apache/doris/pull/38833#discussion_r1703150007
########## be/src/util/thread.h: ########## @@ -291,4 +293,87 @@ class ThreadJoiner { // Registers /threadz with the debug webserver. void register_thread_display_page(WebPageHandler* web_page_handler); + +// A singleton class that tracks all live threads, and groups them together for easy +// auditing. Used only by Thread. +class ThreadMgr { +public: + static ThreadMgr* instance(); + + ThreadMgr() : _threads_started_metric(0), _threads_running_metric(0) {} + ~ThreadMgr() { + std::unique_lock<std::mutex> lock(_lock); + _thread_categories.clear(); + } + + static void set_thread_name(const std::string& name, int64_t tid); + +#ifndef __APPLE__ + static void set_idle_sched(int64_t tid); + + static void set_thread_nice_value(int64_t tid); +#endif + + // not the system TID, since pthread_t is less prone to being recycled. + void add_thread(const pthread_t& pthread_id, const std::string& name, + const std::string& category, int64_t tid); + + // Removes a thread from the supplied category. If the thread has + // already been removed, this is a no-op. + void remove_thread(const pthread_t& pthread_id, const std::string& category); + + void display_thread_callback(const WebPageHandler::ArgumentMap& args, EasyJson* ej) const; + void update_threads_metrics(); + +private: + // Container class for any details we want to capture about a thread + // TODO: Add start-time. + // TODO: Track fragment ID. + class ThreadDescriptor { + public: + ThreadDescriptor() {} Review Comment: warning: use '= default' to define a trivial default constructor [modernize-use-equals-default] ```suggestion ThreadDescriptor() = default; ``` ########## be/src/util/thread.h: ########## @@ -291,4 +293,87 @@ // Registers /threadz with the debug webserver. void register_thread_display_page(WebPageHandler* web_page_handler); + +// A singleton class that tracks all live threads, and groups them together for easy +// auditing. Used only by Thread. +class ThreadMgr { +public: + static ThreadMgr* instance(); + + ThreadMgr() : _threads_started_metric(0), _threads_running_metric(0) {} + ~ThreadMgr() { + std::unique_lock<std::mutex> lock(_lock); + _thread_categories.clear(); + } + + static void set_thread_name(const std::string& name, int64_t tid); + +#ifndef __APPLE__ + static void set_idle_sched(int64_t tid); + + static void set_thread_nice_value(int64_t tid); +#endif + + // not the system TID, since pthread_t is less prone to being recycled. + void add_thread(const pthread_t& pthread_id, const std::string& name, + const std::string& category, int64_t tid); + + // Removes a thread from the supplied category. If the thread has + // already been removed, this is a no-op. + void remove_thread(const pthread_t& pthread_id, const std::string& category); + + void display_thread_callback(const WebPageHandler::ArgumentMap& args, EasyJson* ej) const; + void update_threads_metrics(); + +private: + // Container class for any details we want to capture about a thread + // TODO: Add start-time. + // TODO: Track fragment ID. + class ThreadDescriptor { + public: + ThreadDescriptor() {} + ThreadDescriptor(std::string category, std::string name, int64_t thread_id) + : _name(std::move(name)), _category(std::move(category)), _thread_id(thread_id) {} + + const std::string& name() const { return _name; } + const std::string& category() const { return _category; } + int64_t thread_id() const { return _thread_id; } + + void register_metric(); + void update_metric(); + void deregister_metric(); + + private: + std::string _name; + std::string _category; + int64_t _thread_id; + + std::shared_ptr<MetricEntity> _metric_entity; + DoubleCounter* thread_cpu_total = nullptr; + }; + + void summarize_thread_descriptor(const ThreadDescriptor& desc, EasyJson* ej) const; + + // A ThreadCategory is a set of threads that are logically related. + // TODO: unordered_map is incompatible with pthread_t, but would be more + // efficient here. + typedef std::map<const pthread_t, ThreadDescriptor> ThreadCategory; + + // All thread categories, keyed on the category name. + typedef std::map<std::string, ThreadCategory> ThreadCategoryMap; + + // Protects _thread_categories and thread metrics. + mutable std::mutex _lock; + + // All thread categories that ever contained a thread, even if empty + ThreadCategoryMap _thread_categories; + + // Counters to track all-time total number of threads, and the + // current number of running threads. + uint64_t _threads_started_metric; Review Comment: warning: use default member initializer for '_threads_started_metric' [modernize-use-default-member-init] be/src/util/thread.h:302: ```diff - ThreadMgr() : _threads_started_metric(0), _threads_running_metric(0) {} + ThreadMgr() : , _threads_running_metric(0) {} ``` ```suggestion uint64_t _threads_started_metric{0}; ``` ########## be/src/util/thread.h: ########## @@ -291,4 +293,87 @@ // Registers /threadz with the debug webserver. void register_thread_display_page(WebPageHandler* web_page_handler); + +// A singleton class that tracks all live threads, and groups them together for easy +// auditing. Used only by Thread. +class ThreadMgr { +public: + static ThreadMgr* instance(); + + ThreadMgr() : _threads_started_metric(0), _threads_running_metric(0) {} + ~ThreadMgr() { + std::unique_lock<std::mutex> lock(_lock); + _thread_categories.clear(); + } + + static void set_thread_name(const std::string& name, int64_t tid); + +#ifndef __APPLE__ + static void set_idle_sched(int64_t tid); + + static void set_thread_nice_value(int64_t tid); +#endif + + // not the system TID, since pthread_t is less prone to being recycled. + void add_thread(const pthread_t& pthread_id, const std::string& name, + const std::string& category, int64_t tid); + + // Removes a thread from the supplied category. If the thread has + // already been removed, this is a no-op. + void remove_thread(const pthread_t& pthread_id, const std::string& category); + + void display_thread_callback(const WebPageHandler::ArgumentMap& args, EasyJson* ej) const; + void update_threads_metrics(); + +private: + // Container class for any details we want to capture about a thread + // TODO: Add start-time. + // TODO: Track fragment ID. + class ThreadDescriptor { + public: + ThreadDescriptor() {} + ThreadDescriptor(std::string category, std::string name, int64_t thread_id) + : _name(std::move(name)), _category(std::move(category)), _thread_id(thread_id) {} + + const std::string& name() const { return _name; } + const std::string& category() const { return _category; } + int64_t thread_id() const { return _thread_id; } + + void register_metric(); + void update_metric(); + void deregister_metric(); + + private: + std::string _name; + std::string _category; + int64_t _thread_id; + + std::shared_ptr<MetricEntity> _metric_entity; + DoubleCounter* thread_cpu_total = nullptr; + }; + + void summarize_thread_descriptor(const ThreadDescriptor& desc, EasyJson* ej) const; + + // A ThreadCategory is a set of threads that are logically related. + // TODO: unordered_map is incompatible with pthread_t, but would be more + // efficient here. + typedef std::map<const pthread_t, ThreadDescriptor> ThreadCategory; + + // All thread categories, keyed on the category name. + typedef std::map<std::string, ThreadCategory> ThreadCategoryMap; + + // Protects _thread_categories and thread metrics. + mutable std::mutex _lock; + + // All thread categories that ever contained a thread, even if empty + ThreadCategoryMap _thread_categories; + + // Counters to track all-time total number of threads, and the + // current number of running threads. + uint64_t _threads_started_metric; + uint64_t _threads_running_metric; + + DISALLOW_COPY_AND_ASSIGN(ThreadMgr); Review Comment: warning: prefer deleting copy constructor and assignment operator over using macro 'DISALLOW_COPY_AND_ASSIGN' [modernize-replace-disallow-copy-and-assign-macro] ```suggestion ThreadMgr(const ThreadMgr &) = delete; const ThreadMgr &operator=(const ThreadMgr &) = delete; ``` ########## be/src/util/thread.h: ########## @@ -291,4 +293,87 @@ // Registers /threadz with the debug webserver. void register_thread_display_page(WebPageHandler* web_page_handler); + +// A singleton class that tracks all live threads, and groups them together for easy +// auditing. Used only by Thread. +class ThreadMgr { +public: + static ThreadMgr* instance(); + + ThreadMgr() : _threads_started_metric(0), _threads_running_metric(0) {} + ~ThreadMgr() { + std::unique_lock<std::mutex> lock(_lock); + _thread_categories.clear(); + } + + static void set_thread_name(const std::string& name, int64_t tid); + +#ifndef __APPLE__ + static void set_idle_sched(int64_t tid); + + static void set_thread_nice_value(int64_t tid); +#endif + + // not the system TID, since pthread_t is less prone to being recycled. + void add_thread(const pthread_t& pthread_id, const std::string& name, + const std::string& category, int64_t tid); + + // Removes a thread from the supplied category. If the thread has + // already been removed, this is a no-op. + void remove_thread(const pthread_t& pthread_id, const std::string& category); + + void display_thread_callback(const WebPageHandler::ArgumentMap& args, EasyJson* ej) const; + void update_threads_metrics(); + +private: + // Container class for any details we want to capture about a thread + // TODO: Add start-time. + // TODO: Track fragment ID. + class ThreadDescriptor { + public: + ThreadDescriptor() {} + ThreadDescriptor(std::string category, std::string name, int64_t thread_id) + : _name(std::move(name)), _category(std::move(category)), _thread_id(thread_id) {} + + const std::string& name() const { return _name; } + const std::string& category() const { return _category; } + int64_t thread_id() const { return _thread_id; } + + void register_metric(); + void update_metric(); + void deregister_metric(); + + private: + std::string _name; + std::string _category; + int64_t _thread_id; + + std::shared_ptr<MetricEntity> _metric_entity; + DoubleCounter* thread_cpu_total = nullptr; + }; + + void summarize_thread_descriptor(const ThreadDescriptor& desc, EasyJson* ej) const; + + // A ThreadCategory is a set of threads that are logically related. + // TODO: unordered_map is incompatible with pthread_t, but would be more + // efficient here. + typedef std::map<const pthread_t, ThreadDescriptor> ThreadCategory; + + // All thread categories, keyed on the category name. + typedef std::map<std::string, ThreadCategory> ThreadCategoryMap; + + // Protects _thread_categories and thread metrics. + mutable std::mutex _lock; + + // All thread categories that ever contained a thread, even if empty + ThreadCategoryMap _thread_categories; + + // Counters to track all-time total number of threads, and the + // current number of running threads. + uint64_t _threads_started_metric; + uint64_t _threads_running_metric; Review Comment: warning: use default member initializer for '_threads_running_metric' [modernize-use-default-member-init] be/src/util/thread.h:302: ```diff - ThreadMgr() : _threads_started_metric(0), _threads_running_metric(0) {} + ThreadMgr() : _threads_started_metric(0), {} ``` ```suggestion uint64_t _threads_running_metric{0}; ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org