Re: [RFA] Use std::vector for moribund_locations

[Tom Tromey <tom@tromey.com>]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Just a note that this changes an "unordered remove" to an "ordered remove".
Simon> If we don't need to keep the relative order of the remaining elements, it
Simon> might be good performance-wise to keep the original behavior.  This could
Simon> be done by keeping the original code structure (iterating by index) and
Simon> calling undordered_remove.

How's this?

Tom

commit ccb7af8e831bd703fda0fa87ea5bca951965faf0
Author: Tom Tromey <tom@tromey.com>
Date:   Tue Jun 5 10:35:38 2018 -0600

    Use std::vector for moribund_locations
    
    This changes moribund_locations to use a std::vector rather than VEC.
    I also noticed that moribund_locations is only used in breakpoint.c,
    so now it is static as well.
    
    It might be possible to make this code a bit simpler by using a
    ref_ptr in moribund_locations; however, I have not done this.
    
    Tested by the buildbot.
    
    gdb/ChangeLog
    2018-07-09  Tom Tromey  <tom@tromey.com>
    
            * breakpoint.c (moribund_locations): Now static and a
            std::vector.
            (breakpoint_init_inferior, moribund_breakpoint_here_p)
            (build_bpstat_chain, update_global_location_list)
            (breakpoint_retire_moribund): Update.
            * breakpoint.h (bp_location_p): Remove typedef.  Don't declare
            VEC.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 70b4d443d0d..fbe2f2be92b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2018-07-09  Tom Tromey  <tom@tromey.com>
 
+	* breakpoint.c (moribund_locations): Now static and a
+	std::vector.
+	(breakpoint_init_inferior, moribund_breakpoint_here_p)
+	(build_bpstat_chain, update_global_location_list)
+	(breakpoint_retire_moribund): Update.
+	* breakpoint.h (bp_location_p): Remove typedef.  Don't declare
+	VEC.
+
+2018-07-09  Tom Tromey  <tom@tromey.com>
+
 	* Makefile.in (CDEPS): Don't mention XM_CDEPS.
 	(CLIBS): Don't mention NAT_CLIBS.
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 77c4638df83..b135db3db95 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -560,7 +560,7 @@ static CORE_ADDR bp_locations_shadow_len_after_address_max;
 /* The locations that no longer correspond to any breakpoint, unlinked
    from the bp_locations array, but for which a hit may still be
    reported by a target.  */
-VEC(bp_location_p) *moribund_locations = NULL;
+static std::vector<bp_location *> moribund_locations;
 
 /* Number of last breakpoint made.  */
 
@@ -3860,8 +3860,6 @@ void
 breakpoint_init_inferior (enum inf_context context)
 {
   struct breakpoint *b, *b_tmp;
-  struct bp_location *bl;
-  int ix;
   struct program_space *pspace = current_program_space;
 
   /* If breakpoint locations are shared across processes, then there's
@@ -3951,9 +3949,9 @@ breakpoint_init_inferior (enum inf_context context)
   }
 
   /* Get rid of the moribund locations.  */
-  for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, bl); ++ix)
+  for (bp_location *bl : moribund_locations)
     decref_bp_location (&bl);
-  VEC_free (bp_location_p, moribund_locations);
+  moribund_locations.clear ();
 }
 
 /* These functions concern about actual breakpoints inserted in the
@@ -4041,10 +4039,7 @@ breakpoint_in_range_p (const address_space *aspace,
 int
 moribund_breakpoint_here_p (const address_space *aspace, CORE_ADDR pc)
 {
-  struct bp_location *loc;
-  int ix;
-
-  for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
+  for (bp_location *loc : moribund_locations)
     if (breakpoint_location_address_match (loc, aspace, pc))
       return 1;
 
@@ -5372,10 +5367,7 @@ build_bpstat_chain (const address_space *aspace, CORE_ADDR bp_addr,
   if (!target_supports_stopped_by_sw_breakpoint ()
       || !target_supports_stopped_by_hw_breakpoint ())
     {
-      bp_location *loc;
-
-      for (int ix = 0;
-	   VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
+      for (bp_location *loc : moribund_locations)
 	{
 	  if (breakpoint_location_address_match (loc, aspace, bp_addr)
 	      && need_moribund_for_location_type (loc))
@@ -12012,7 +12004,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
 	      old_loc->events_till_retirement = 3 * (thread_count () + 1);
 	      old_loc->owner = NULL;
 
-	      VEC_safe_push (bp_location_p, moribund_locations, old_loc);
+	      moribund_locations.push_back (old_loc);
 	    }
 	  else
 	    {
@@ -12115,16 +12107,16 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
 void
 breakpoint_retire_moribund (void)
 {
-  struct bp_location *loc;
-  int ix;
-
-  for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
-    if (--(loc->events_till_retirement) == 0)
-      {
-	decref_bp_location (&loc);
-	VEC_unordered_remove (bp_location_p, moribund_locations, ix);
-	--ix;
-      }
+  for (int ix = 0; ix < moribund_locations.size (); ++ix)
+    {
+      struct bp_location *loc = moribund_locations[ix];
+      if (--(loc->events_till_retirement) == 0)
+	{
+	  decref_bp_location (&loc);
+	  unordered_remove (moribund_locations, ix);
+	  --ix;
+	}
+    }
 }
 
 static void
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 7da9452fd15..63cb5370c16 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -667,9 +667,6 @@ enum watchpoint_triggered
   watch_triggered_yes  
 };
 
-typedef struct bp_location *bp_location_p;
-DEF_VEC_P(bp_location_p);
-
 /* Some targets (e.g., embedded PowerPC) need two debug registers to set
    a watchpoint over a memory region.  If this flag is true, GDB will use
    only one register per watchpoint, thus assuming that all acesses that