diff mbox

Use find_thread_in_random in select_event_lwp

Message ID 20190409164600.29369-1-tromey@adacore.com
State New, archived
Headers

Commit Message

Tom Tromey April 9, 2019, 4:46 p.m. UTC 
  I noticed that find_thread_in_random duplicates the code in
find_thread_in_random, so this patch changes the latter to use the
former.

There are two other spots in gdb that do this, but to unify all of
them would require switching some code from using the "iterate over"
idiom to using iterators.

Another possible improvement is that find_thread_in_random could be
made single-pass using reservoir sampling.

Tested by the buildbot.

gdb/gdbserver/ChangeLog
2019-04-09  Tom Tromey  <tromey@adacore.com>

	* linux-low.c (select_event_lwp): Use find_thread_in_random.
---
 gdb/gdbserver/ChangeLog   |  4 ++++
 gdb/gdbserver/linux-low.c | 35 ++++-------------------------------
 2 files changed, 8 insertions(+), 31 deletions(-)

Comments

Pedro Alves April 9, 2019, 5:58 p.m. UTC | #1 
On 4/9/19 5:46 PM, Tom Tromey wrote:
> I noticed that find_thread_in_random duplicates the code in
> find_thread_in_random, so this patch changes the latter to use the
> former.
> 
> There are two other spots in gdb that do this, but to unify all of
> them would require switching some code from using the "iterate over"
> idiom to using iterators.

Plus I'm touching that code a lot in the multi-target work and adding
more instances of "pick one at random".  :-D

> 
> Another possible improvement is that find_thread_in_random could be
> made single-pass using reservoir sampling.
> 

Or by simply keeping/maintaining a count of threads.

> Tested by the buildbot.
> 
> gdb/gdbserver/ChangeLog
> 2019-04-09  Tom Tromey  <tromey@adacore.com>
> 
> 	* linux-low.c (select_event_lwp): Use find_thread_in_random.

OK.

Thanks,
Pedro Alves
John Baldwin April 22, 2019, 10:39 p.m. UTC | #2 
On 4/9/19 9:46 AM, Tom Tromey wrote:
> I noticed that find_thread_in_random duplicates the code in
> find_thread_in_random, so this patch changes the latter to use the
> former.

(I don't think I saw this get merged yet): One nit is that I think
the second 'find_thread_in_random' here should be 'select_event_lwp'
in the log?
Tom Tromey April 23, 2019, 2:46 p.m. UTC | #3 
>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:

John> On 4/9/19 9:46 AM, Tom Tromey wrote:
>> I noticed that find_thread_in_random duplicates the code in
>> find_thread_in_random, so this patch changes the latter to use the
>> former.

John> (I don't think I saw this get merged yet): One nit is that I think
John> the second 'find_thread_in_random' here should be 'select_event_lwp'
John> in the log?

Yeah, should have been -- sorry about that.
It did go in a while ago so I don't think it can be fixed.

Tom
diff mbox

Patch

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 94af240a5c2..168f4b2abc2 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2828,7 +2828,6 @@  linux_wait_for_event (ptid_t ptid, int *wstatp, int options)
 static void
 select_event_lwp (struct lwp_info **orig_lp)
 {
-  int random_selector;
   struct thread_info *event_thread = NULL;
 
   /* In all-stop, give preference to the LWP that is being
@@ -2862,39 +2861,13 @@  select_event_lwp (struct lwp_info **orig_lp)
       /* No single-stepping LWP.  Select one at random, out of those
          which have had events.  */
 
-      /* First see how many events we have.  */
-      int num_events = 0;
-      for_each_thread ([&] (thread_info *thread)
-	{
-	  lwp_info *lp = get_thread_lwp (thread);
-
-	  /* Count only resumed LWPs that have an event pending. */
-	  if (thread->last_status.kind == TARGET_WAITKIND_IGNORE
-	      && lp->status_pending_p)
-	    num_events++;
-	});
-      gdb_assert (num_events > 0);
-
-      /* Now randomly pick a LWP out of those that have had
-	 events.  */
-      random_selector = (int)
-	((num_events * (double) rand ()) / (RAND_MAX + 1.0));
-
-      if (debug_threads && num_events > 1)
-	debug_printf ("SEL: Found %d SIGTRAP events, selecting #%d\n",
-		      num_events, random_selector);
-
-      event_thread = find_thread ([&] (thread_info *thread)
+      event_thread = find_thread_in_random ([&] (thread_info *thread)
 	{
 	  lwp_info *lp = get_thread_lwp (thread);
 
-	  /* Select only resumed LWPs that have an event pending.  */
-	  if (thread->last_status.kind == TARGET_WAITKIND_IGNORE
-	      && lp->status_pending_p)
-	    if (random_selector-- == 0)
-	      return true;
-
-	  return false;
+	  /* Only resumed LWPs that have an event pending. */
+	  return (thread->last_status.kind == TARGET_WAITKIND_IGNORE
+		  && lp->status_pending_p);
 	});
     }