From a95cc0927cfaf073fef23e8215a3df812fc5a609 Mon Sep 17 00:00:00 2001
From: wjunLu <wjunlu217@gmail.com>
Date: Tue, 22 Jul 2025 11:39:44 +0800
Subject: [PATCH] optimize thread counter.

---
 sql/event_scheduler.cc             |  2 -
 sql/mysqld_thd_manager.cc          | 25 +++++++++++
 sql/mysqld_thd_manager.h           | 15 +++----
 storage/perfschema/pfs_variable.cc | 69 ++++++++++++++++++++++++++++++
 unittest/gunit/thd_manager-t.cc    |  8 ----
 5 files changed, 99 insertions(+), 20 deletions(-)

diff --git a/sql/event_scheduler.cc b/sql/event_scheduler.cc
index 837fad5a0936..e58e95a665c5 100644
--- a/sql/event_scheduler.cc
+++ b/sql/event_scheduler.cc
@@ -172,7 +172,6 @@ bool post_init_event_thread(THD *thd) {
 
   Global_THD_manager *thd_manager = Global_THD_manager::get_instance();
   thd_manager->add_thd(thd);
-  thd_manager->inc_thread_running();
   return false;
 }
 
@@ -192,7 +191,6 @@ void deinit_event_thread(THD *thd) {
   DBUG_PRINT("exit", ("Event thread finishing"));
   thd->release_resources();
   thd_manager->remove_thd(thd);
-  thd_manager->dec_thread_running();
   delete thd;
 }
 
diff --git a/sql/mysqld_thd_manager.cc b/sql/mysqld_thd_manager.cc
index 62865f1b79b4..582fde033f70 100644
--- a/sql/mysqld_thd_manager.cc
+++ b/sql/mysqld_thd_manager.cc
@@ -364,6 +364,31 @@ THD_ptr Global_THD_manager::find_thd(Find_thd_with_id *func) {
   return THD_ptr{nullptr};
 }
 
+/**
+  This class implements callback for do_for_all_thd().
+  It counts the total number of running threads from global thread list.
+*/
+class Count_thread_running : public Do_THD_Impl {
+  public:
+    Count_thread_running() : m_count(0) {}
+    ~Count_thread_running() {}
+  virtual void operator()(THD *thd) {
+    if (thd->get_command() != COM_SLEEP) {
+      m_count++;
+    }
+  }
+  int get_count() { return m_count; }
+
+  private:
+    int m_count;
+};
+
+void Global_THD_manager::count_num_thread_running() {
+  Count_thread_running count_thread_running;
+  do_for_all_thd(&count_thread_running);
+  atomic_num_thread_running = count_thread_running.get_count();
+}
+
 void inc_thread_created() {
   Global_THD_manager::get_instance()->inc_thread_created();
 }
diff --git a/sql/mysqld_thd_manager.h b/sql/mysqld_thd_manager.h
index 7beab1788ce4..d913df60181d 100644
--- a/sql/mysqld_thd_manager.h
+++ b/sql/mysqld_thd_manager.h
@@ -254,20 +254,15 @@ class Global_THD_manager {
   void remove_thd(THD *thd);
 
   /**
-    Retrieves thread running statistic variable.
-    @return int Returns the total number of threads currently running
+    Count thread running statistic variable.
   */
-  int get_num_thread_running() const { return atomic_num_thread_running; }
+  void count_num_thread_running();
 
   /**
-    Increments thread running statistic variable.
-  */
-  void inc_thread_running() { atomic_num_thread_running++; }
-
-  /**
-    Decrements thread running statistic variable.
+    Retrieves thread running statistic variable.
+    @return int Returns the total number of threads currently running
   */
-  void dec_thread_running() { atomic_num_thread_running--; }
+  int get_num_thread_running() const { return atomic_num_thread_running; }
 
   /**
     Retrieves thread created statistic variable.
diff --git a/storage/perfschema/pfs_variable.cc b/storage/perfschema/pfs_variable.cc
index 64b8ad137c38..eafcf9b4d296 100644
--- a/storage/perfschema/pfs_variable.cc
+++ b/storage/perfschema/pfs_variable.cc
@@ -1188,6 +1188,16 @@ int PFS_status_variable_cache::do_materialize_global() {
                                         false, /* threads */
                                         true,  /* THDs */
                                         &visitor);
+  /*
+    Because of the reason described in 
+    PFS_status_variable_cache::do_materialize_all(THD *unsafe_thd),
+    PFS_status_variable_cache::do_materialize_session(THD *unsafe_thd) and
+    PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread),
+    count_num_thread_running() cannot put together with
+    get_num_thread_running(), so count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
   /*
     Build the status variable cache using the SHOW_VAR array as a reference.
     Use the status totals collected from all threads.
@@ -1233,6 +1243,22 @@ int PFS_status_variable_cache::do_materialize_all(THD *unsafe_thd) {
     init_show_var_array(OPT_SESSION, false);
   }
 
+  /*
+    count_num_thread_running() counts the total number of running threads
+    from global thread list, using LOCK_thd_list to protect sharded
+    global thread list. In lock_order_dependencies.txt, the lock order
+    is that LOCK_thd_list must be locked before LOCK_thd_data. In this
+    function, LOCK_thd_data is already locked in get_THD(), Then manifest()
+    will call get_num_thread_running(). If get_num_thread_running() counts
+    and returns the num, the lock order will be incorrect, which may
+    lead to dead lock. To prevent this situation, get_num_thread_running()
+    is split into two part, one is still called get_num_thread_running()
+    which returns the num, the other is called count_num_thread_running()
+    which counts the num and should be called before get_THD() and
+    get_num_thread_running(). So count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
   /* Get and lock a validated THD from the thread manager. */
   THD_ptr thd_ptr = get_THD(unsafe_thd);
   m_safe_thd = thd_ptr.get();
@@ -1280,6 +1306,22 @@ int PFS_status_variable_cache::do_materialize_session(THD *unsafe_thd) {
     init_show_var_array(OPT_SESSION, true);
   }
 
+  /*
+    count_num_thread_running() counts the total number of running threads
+    from global thread list, using LOCK_thd_list to protect sharded
+    global thread list. In lock_order_dependencies.txt, the lock order
+    is that LOCK_thd_list must be locked before LOCK_thd_data. In this
+    function, LOCK_thd_data is already locked in get_THD(), Then manifest()
+    will call get_num_thread_running(). If get_num_thread_running() counts
+    and returns the num, the lock order will be incorrect, which may
+    lead to dead lock. To prevent this situation, get_num_thread_running()
+    is split into two part, one is still called get_num_thread_running()
+    which returns the num, the other is called count_num_thread_running()
+    which counts the num and should be called before get_THD() and
+    get_num_thread_running(). So count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
   /* Get and lock a validated THD from the thread manager. */
   THD_ptr thd_ptr = get_THD(unsafe_thd);
   m_safe_thd = thd_ptr.get();
@@ -1321,6 +1363,22 @@ int PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread) {
   /* The SHOW_VAR array must be initialized externally. */
   assert(m_initialized);
 
+  /*
+    count_num_thread_running() counts the total number of running threads
+    from global thread list, using LOCK_thd_list to protect sharded
+    global thread list. In lock_order_dependencies.txt, the lock order
+    is that LOCK_thd_list must be locked before LOCK_thd_data. In this
+    function, LOCK_thd_data is already locked in get_THD(), Then manifest()
+    will call get_num_thread_running(). If get_num_thread_running() counts
+    and returns the num, the lock order will be incorrect, which may
+    lead to dead lock. To prevent this situation, get_num_thread_running()
+    is split into two part, one is still called get_num_thread_running()
+    which returns the num, the other is called count_num_thread_running()
+    which counts the num and should be called before get_THD() and
+    get_num_thread_running(). So count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
   /* Get and lock a validated THD from the thread manager. */
   THD_ptr thd_ptr = get_THD(pfs_thread);
   m_safe_thd = thd_ptr.get();
@@ -1368,6 +1426,17 @@ int PFS_status_variable_cache::do_materialize_client(PFS_client *pfs_client) {
   */
   m_sum_client_status(pfs_client, &status_totals);
 
+  /*
+    Because of the reason described in 
+    PFS_status_variable_cache::do_materialize_all(THD *unsafe_thd),
+    PFS_status_variable_cache::do_materialize_session(THD *unsafe_thd) and
+    PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread),
+    count_num_thread_running() cannot put together with
+    get_num_thread_running(), so count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
+
   /*
     Build the status variable cache using the SHOW_VAR array as a reference and
     the status totals collected from threads associated with this client.
diff --git a/unittest/gunit/thd_manager-t.cc b/unittest/gunit/thd_manager-t.cc
index 8277f2902b94..8efc569a5ebc 100644
--- a/unittest/gunit/thd_manager-t.cc
+++ b/unittest/gunit/thd_manager-t.cc
@@ -84,14 +84,6 @@ TEST_F(ThreadManagerTest, AddRemoveTHDWithGuard) {
   EXPECT_EQ(0U, thd_manager->get_thd_count());
 }
 
-TEST_F(ThreadManagerTest, IncDecThreadRunning) {
-  EXPECT_EQ(0, thd_manager->get_num_thread_running());
-  thd_manager->inc_thread_running();
-  EXPECT_EQ(1, thd_manager->get_num_thread_running());
-  thd_manager->dec_thread_running();
-  EXPECT_EQ(0, thd_manager->get_num_thread_running());
-}
-
 TEST_F(ThreadManagerTest, IncThreadCreated) {
   EXPECT_EQ(0U, thd_manager->get_num_thread_created());
   thd_manager->inc_thread_created();
