public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tromey@adacore.com>
Subject: [PATCH v2 1/4] Avoid submitting empty tasks in parallel_for_each
Date: Tue, 10 Jan 2023 11:33:35 -0700	[thread overview]
Message-ID: <20230110183338.2623088-2-tromey@adacore.com> (raw)
In-Reply-To: <20230110183338.2623088-1-tromey@adacore.com>

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 | 39 ++++++++++++++++++++++++++
 gdbsupport/parallel-for.h              | 30 ++++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/gdb/unittests/parallel-for-selftests.c b/gdb/unittests/parallel-for-selftests.c
index 3162db18df1..15a095ae62b 100644
--- a/gdb/unittests/parallel-for-selftests.c
+++ b/gdb/unittests/parallel-for-selftests.c
@@ -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 */
diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h
index b565676a0d0..de9ebb15746 100644
--- a/gdbsupport/parallel-for.h
+++ b/gdbsupport/parallel-for.h
@@ -70,6 +70,12 @@ struct par_for_accumulator
     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 @@ struct par_for_accumulator<void>
       }
   }
 
+  /* 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"),
-- 
2.38.1


  reply	other threads:[~2023-01-10 18:33 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-10 18:33 [PATCH v2 0/4] Fix regression in new DWARF reader Tom Tromey
2023-01-10 18:33 ` Tom Tromey [this message]
2023-01-14  6:03   ` [PATCH v2 1/4] Avoid submitting empty tasks in parallel_for_each Joel Brobecker
2023-01-10 18:33 ` [PATCH v2 2/4] Don't erase empty indices in DWARF reader Tom Tromey
2023-01-14  6:05   ` Joel Brobecker
2023-01-17 13:53     ` Tom Tromey
2023-01-10 18:33 ` [PATCH v2 3/4] Move hash_entry and eq_entry into cooked_index::do_finalize Tom Tromey
2023-01-14  6:06   ` Joel Brobecker
2023-01-10 18:33 ` [PATCH v2 4/4] Fix parameter-less template regression in new DWARF reader Tom Tromey
2023-01-14  6:11   ` Joel Brobecker
2023-01-17 13:54     ` Tom Tromey
2023-01-17 16:44     ` Tom de Vries
2023-01-17 18:46       ` Tom Tromey
2023-01-17 18:09   ` Simon Marchi
2023-01-17 19:39     ` Tom Tromey
2023-01-27  5:47       ` Simon Marchi
2023-01-27 10:15         ` Andrew Burgess
2023-01-27 14:30           ` Tom Tromey
2023-01-27 19:57             ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230110183338.2623088-2-tromey@adacore.com \
    --to=tromey@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).