Optimize by moving 'delete' out of locks.

2026-10-29 03:27:04 +08:00 · 2023-12-06 18:08:04 +08:00
parent 30c6a27546
commit 908a2deaa2
1 changed files with 101 additions and 40 deletions
@@ -89,14 +89,16 @@ struct __NamedObjectList
 		return list_empty(&this->head);
 	}

-	void del(T *node, rb_root *root)
+	bool del(T *node, rb_root *root)
 	{
 		list_del(&node->list);
 		if (this->empty())
 		{
 			rb_erase(&this->rb, root);
-			delete this;
+			return true;
 		}
+		else
+			return false;
 	}

 	struct rb_node rb;
@@ -196,9 +198,15 @@ public:
 	{
 		if (node_.task)
 		{
-			std::lock_guard<std::mutex> lock(__timer_map.mutex_);
+			bool erased = false;
+
+			__timer_map.mutex_.lock();
 			if (node_.task)
-				timers_->del(&node_, &__timer_map.root_);
+				erased = timers_->del(&node_, &__timer_map.root_);
+
+			__timer_map.mutex_.unlock();
+			if (erased)
+				delete timers_;
 		}
 	}

@@ -232,12 +240,18 @@ void __WFNamedTimerTask::handle(int state, int error)
 {
 	if (node_.task)
 	{
-		std::lock_guard<std::mutex> lock(__timer_map.mutex_);
+		bool erased = false;
+
+		__timer_map.mutex_.lock();
 		if (node_.task)
 		{
-			timers_->del(&node_, &__timer_map.root_);
+			erased = timers_->del(&node_, &__timer_map.root_);
 			node_.task = NULL;
 		}
+
+		__timer_map.mutex_.unlock();
+		if (erased)
+			delete timers_;
 	}

 	mutex_.lock();
@@ -263,14 +277,17 @@ void __NamedTimerMap::cancel(const std::string& name, size_t max)
 	struct __timer_node *node;
 	TimerList *timers;

-	std::lock_guard<std::mutex> lock(mutex_);
+	mutex_.lock();
 	timers = __get_object_list<TimerList>(name, &root_, false);
 	if (timers)
 	{
-		do
+		while (1)
 		{
 			if (max == 0)
-				return;
+			{
+				timers = NULL;
+				break;
+			}

 			node = list_entry(timers->head.next, struct __timer_node, list);
 			list_del(&node->list);
@@ -279,11 +296,16 @@ void __NamedTimerMap::cancel(const std::string& name, size_t max)

 			node->task = NULL;
 			max--;
-		} while (!timers->empty());
-
-		rb_erase(&timers->rb, &root_);
-		delete timers;
+			if (timers->empty())
+			{
+				rb_erase(&timers->rb, &root_);
+				break;
+			}
+		}
 	}
+
+	mutex_.unlock();
+	delete timers;
 }

 WFTimerTask *WFTaskFactory::create_timer_task(const std::string& name,
@@ -325,13 +347,17 @@ public:

 	void remove(CounterList *counters, struct __counter_node *node)
 	{
+		bool erased;
+
 		mutex_.lock();
-		counters->del(node, &root_);
+		erased = counters->del(node, &root_);
 		mutex_.unlock();
+		if (erased)
+			delete counters;
 	}

 private:
-	void count_n_locked(CounterList *counters, unsigned int n,
+	bool count_n_locked(CounterList *counters, unsigned int n,
 						struct list_head *task_list);
 	struct rb_root root_;
 	std::mutex mutex_;
@@ -389,32 +415,33 @@ WFCounterTask *__NamedCounterMap::create(const std::string& name,
 	return task;
 }

-void __NamedCounterMap::count_n_locked(CounterList *counters, unsigned int n,
+bool __NamedCounterMap::count_n_locked(CounterList *counters, unsigned int n,
 									   struct list_head *task_list)
 {
 	struct __counter_node *node;

-	do
+	while (n > 0)
 	{
-		if (n == 0)
-			return;
-
 		node = list_entry(counters->head.next, struct __counter_node, list);
 		if (n >= node->target_value)
 		{
 			n -= node->target_value;
 			node->target_value = 0;
 			list_move_tail(&node->list, task_list);
+			if (counters->empty())
+			{
+				rb_erase(&counters->rb, &root_);
+				return true;
+			}
 		}
 		else
 		{
 			node->target_value -= n;
-			n = 0;
+			break;
 		}
-	} while (!counters->empty());
+	}

-	rb_erase(&counters->rb, &root_);
-	delete counters;
+	return false;
 }

 void __NamedCounterMap::count_n(const std::string& name, unsigned int n)
@@ -422,13 +449,17 @@ void __NamedCounterMap::count_n(const std::string& name, unsigned int n)
 	LIST_HEAD(task_list);
 	struct __counter_node *node;
 	CounterList *counters;
+	bool erased = false;

 	mutex_.lock();
 	counters = __get_object_list<CounterList>(name, &root_, false);
 	if (counters)
-		count_n_locked(counters, n, &task_list);
+		erased = count_n_locked(counters, n, &task_list);

 	mutex_.unlock();
+	if (erased)
+		delete counters;
+
 	while (!list_empty(&task_list))
 	{
 		node = list_entry(task_list.next, struct __counter_node, list);
@@ -441,15 +472,19 @@ void __NamedCounterMap::count(CounterList *counters,
 							  struct __counter_node *node)
 {
 	__WFNamedCounterTask *task = NULL;
+	bool erased = false;

 	mutex_.lock();
 	if (--node->target_value == 0)
 	{
 		task = node->task;
-		counters->del(node, &root_);
+		erased = counters->del(node, &root_);
 	}

 	mutex_.unlock();
+	if (erased)
+		delete counters;
+
 	if (task)
 		task->WFCounterTask::count();
 }
@@ -491,13 +526,17 @@ public:

 	void remove(MailboxList *mailboxes, struct __mailbox_node *node)
 	{
+		bool erased;
+
 		mutex_.lock();
-		mailboxes->del(node, &root_);
+		erased = mailboxes->del(node, &root_);
 		mutex_.unlock();
+		if (erased)
+			delete mailboxes;
 	}

 private:
-	void send_max_locked(MailboxList *mailboxes, void *msg, size_t max,
+	bool send_max_locked(MailboxList *mailboxes, void *msg, size_t max,
 						 struct list_head *task_list);
 	struct rb_root root_;
 	std::mutex mutex_;
@@ -567,7 +606,7 @@ WFMailboxTask *__NamedMailboxMap::create(const std::string& name,
 	return task;
 }

-void __NamedMailboxMap::send_max_locked(MailboxList *mailboxes,
+bool __NamedMailboxMap::send_max_locked(MailboxList *mailboxes,
 										void *msg, size_t max,
 										struct list_head *task_list)
 {
@@ -578,7 +617,7 @@ void __NamedMailboxMap::send_max_locked(MailboxList *mailboxes,
 		do
 		{
 			if (max == 0)
-				return;
+				return false;

 			list_move_tail(mailboxes->head.next, task_list);
 			max--;
@@ -586,7 +625,7 @@ void __NamedMailboxMap::send_max_locked(MailboxList *mailboxes,
 	}

 	rb_erase(&mailboxes->rb, &root_);
-	delete mailboxes;
+	return true;
 }

 void __NamedMailboxMap::send(const std::string& name, void *msg, size_t max)
@@ -594,13 +633,17 @@ void __NamedMailboxMap::send(const std::string& name, void *msg, size_t max)
 	LIST_HEAD(task_list);
 	struct __mailbox_node *node;
 	MailboxList *mailboxes;
+	bool erased = false;

 	mutex_.lock();
 	mailboxes = __get_object_list<MailboxList>(name, &root_, false);
 	if (mailboxes)
-		send_max_locked(mailboxes, msg, max, &task_list);
+		erased = send_max_locked(mailboxes, msg, max, &task_list);

 	mutex_.unlock();
+	if (erased)
+		delete mailboxes;
+
 	while (!list_empty(&task_list))
 	{
 		node = list_entry(task_list.next, struct __mailbox_node, list);
@@ -613,9 +656,14 @@ void __NamedMailboxMap::send(MailboxList *mailboxes,
 							 struct __mailbox_node *node,
 							 void *msg)
 {
+	bool erased;
+
 	mutex_.lock();
-	mailboxes->del(node, &root_);
+	erased = mailboxes->del(node, &root_);
 	mutex_.unlock();
+	if (erased)
+		delete mailboxes;
+
 	node->task->WFMailboxTask::send(msg);
 }

@@ -663,13 +711,17 @@ public:

 	void remove(ConditionalList *conds, struct __conditional_node *node)
 	{
+		bool erased;
+
 		mutex_.lock();
-		conds->del(node, &root_);
+		erased = conds->del(node, &root_);
 		mutex_.unlock();
+		if (erased)
+			delete conds;
 	}

 private:
-	void signal_max_locked(ConditionalList *conds, void *msg, size_t max,
+	bool signal_max_locked(ConditionalList *conds, void *msg, size_t max,
 						   struct list_head *cond_list);
 	struct rb_root root_;
 	std::mutex mutex_;
@@ -738,7 +790,7 @@ WFConditional *__NamedConditionalMap::create(const std::string& name,
 	return cond;
 }

-void __NamedConditionalMap::signal_max_locked(ConditionalList *conds,
+bool __NamedConditionalMap::signal_max_locked(ConditionalList *conds,
 											  void *msg, size_t max,
 											  struct list_head *cond_list)
 {
@@ -749,7 +801,7 @@ void __NamedConditionalMap::signal_max_locked(ConditionalList *conds,
 		do
 		{
 			if (max == 0)
-				return;
+				return false;

 			list_move_tail(conds->head.next, cond_list);
 			max--;
@@ -757,7 +809,7 @@ void __NamedConditionalMap::signal_max_locked(ConditionalList *conds,
 	}

 	rb_erase(&conds->rb, &root_);
-	delete conds;
+	return true;
 }

 void __NamedConditionalMap::signal(const std::string& name, void *msg, size_t max)
@@ -765,13 +817,17 @@ void __NamedConditionalMap::signal(const std::string& name, void *msg, size_t ma
 	LIST_HEAD(cond_list);
 	struct __conditional_node *node;
 	ConditionalList *conds;
+	bool erased = false;

 	mutex_.lock();
 	conds = __get_object_list<ConditionalList>(name, &root_, false);
 	if (conds)
-		signal_max_locked(conds, msg, max, &cond_list);
+		erased = signal_max_locked(conds, msg, max, &cond_list);

 	mutex_.unlock();
+	if (erased)
+		delete conds;
+
 	while (!list_empty(&cond_list))
 	{
 		node = list_entry(cond_list.next, struct __conditional_node, list);
@@ -784,9 +840,14 @@ void __NamedConditionalMap::signal(ConditionalList *conds,
 								   struct __conditional_node *node,
 								   void *msg)
 {
+	bool erased;
+
 	mutex_.lock();
-	conds->del(node, &root_);
+	erased = conds->del(node, &root_);
 	mutex_.unlock();
+	if (erased)
+		delete conds;
+
 	node->cond->WFConditional::signal(msg);
 }