[GitHub] [tvm] tkonolige commented on a diff in pull request #11448: [Runtime][Thread pool]

[GitBox <gi...@apache.org>]

tkonolige commented on code in PR #11448:
URL: https://github.com/apache/tvm/pull/11448#discussion_r893899782


##########
src/runtime/thread_pool.cc:
##########
@@ -116,9 +115,16 @@ class ParallelLauncher {
     num_pending_.fetch_sub(1);
     par_errors_[task_id] = TVMGetLastError();
     has_error_.store(true);
+    std::unique_lock<std::mutex> lock(finish_mutex_);
+    finish_cv_.notify_all();
   }
   // Signal that one job has finished.
-  void SignalJobFinish() { num_pending_.fetch_sub(1); }
+  void SignalJobFinish() {
+    if (num_pending_.fetch_sub(1) <= 1) {

Review Comment:
   If I understand this correctly, you are adding a mutex `finish_cv_` that guards `num_pending_`. The mutex serves as a wait to have threads wait until `num_pending_` hits zero. Given that `num_pending_` is now guarded, can we turn it into just a raw `int32_t` to avoid additional overhead?
   
   It seems like c++20 would allow a cleaner solution by allowing us to wait on an atomic https://en.cppreference.com/w/cpp/atomic/atomic/wait.



-- 
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-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org