Avoid submitting empty tasks in parallel_for_each
authorTom Tromey <tromey@adacore.com>
Tue, 13 Dec 2022 19:03:34 +0000 (12:03 -0700)
committerTom Tromey <tromey@adacore.com>
Tue, 17 Jan 2023 14:03:26 +0000 (07:03 -0700)
I found that parallel_for_each would submit empty tasks to the thread
pool.  For example, this can happen if the number of tasks is smaller
than the number of available threads.  In the DWARF reader, this
resulted in the cooked index containing empty sub-indices.  This patch
arranges to instead shrink the result vector and process the trailing
entries in the calling thread.

gdb/unittests/parallel-for-selftests.c
gdbsupport/parallel-for.h

index 3162db18df137fd47828f83382089f11f80e08e6..15a095ae62bdc8f8f1ecc494f6dd5be48cdb8f0f 100644 (file)
@@ -149,6 +149,45 @@ TEST (int n_threads)
   SELF_CHECK (counter == NUMBER);
 
 #undef NUMBER
+
+  /* Check that if there are fewer tasks than threads, then we won't
+     end up with a null result.  */
+  std::vector<std::unique_ptr<int>> intresults;
+  std::atomic<bool> any_empty_tasks (false);
+
+  FOR_EACH (1, 0, 1,
+           [&] (int start, int end)
+             {
+               if (start == end)
+                 any_empty_tasks = true;
+               return std::unique_ptr<int> (new int (end - start));
+             });
+  SELF_CHECK (!any_empty_tasks);
+  SELF_CHECK (std::all_of (intresults.begin (),
+                          intresults.end (),
+                          [] (const std::unique_ptr<int> &entry)
+                            {
+                              return entry != nullptr;
+                            }));
+
+  /* The same but using the task size parameter.  */
+  intresults.clear ();
+  any_empty_tasks = false;
+  FOR_EACH (1, 0, 1,
+           [&] (int start, int end)
+             {
+               if (start == end)
+                 any_empty_tasks = true;
+               return std::unique_ptr<int> (new int (end - start));
+             },
+           task_size_one);
+  SELF_CHECK (!any_empty_tasks);
+  SELF_CHECK (std::all_of (intresults.begin (),
+                          intresults.end (),
+                          [] (const std::unique_ptr<int> &entry)
+                            {
+                              return entry != nullptr;
+                            }));
 }
 
 #endif /* FOR_EACH */
index b565676a0d050380853ed26094a815b22911c6b5..de9ebb15746d6fc26b0ba81ec308e5395bd93921 100644 (file)
@@ -70,6 +70,12 @@ public:
     return result;
   }
 
+  /* Resize the results to N.  */
+  void resize (size_t n)
+  {
+    m_futures.resize (n);
+  }
+
 private:
   
   /* A vector of futures coming from the tasks run in the
@@ -108,6 +114,12 @@ public:
       }
   }
 
+  /* Resize the results to N.  */
+  void resize (size_t n)
+  {
+    m_futures.resize (n);
+  }
+
 private:
 
   std::vector<gdb::future<void>> m_futures;
@@ -232,6 +244,24 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
          end = j;
          remaining_size -= chunk_size;
        }
+
+      /* This case means we don't have enough elements to really
+        distribute them.  Rather than ever submit a task that does
+        nothing, we short-circuit here.  */
+      if (first == end)
+       end = last;
+
+      if (end == last)
+       {
+         /* We're about to dispatch the last batch of elements, which
+            we normally process in the main thread.  So just truncate
+            the result list here.  This avoids submitting empty tasks
+            to the thread pool.  */
+         count = i;
+         results.resize (count);
+         break;
+       }
+
       if (parallel_for_each_debug)
        {
          debug_printf (_("Parallel for: elements on worker thread %i\t: %zu"),