Re: [PATCH] Use an std::vector for inline_states

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2018-04-07 15:28, Pedro Alves wrote:
> On 04/07/2018 03:42 PM, Simon Marchi wrote:
> 
>>  /* Locate saved inlined frame state for PTID, if it exists
>>     and is valid.  */
>> @@ -66,43 +70,29 @@ static VEC(inline_state_s) *inline_states;
>>  static struct inline_state *
>>  find_inline_frame_state (ptid_t ptid)
>>  {
> 
>> -	  if (current_pc != state->saved_pc)
>> -	    {
>> -	      /* PC has changed - this context is invalid.  Use the
>> -		 default behavior.  */
>> -	      VEC_unordered_remove (inline_state_s, inline_states, ix);
>> -	      return NULL;
> 
> ...
> 
>> -  state = VEC_safe_push (inline_state_s, inline_states, NULL);
>> -  memset (state, 0, sizeof (*state));
>> -  state->ptid = ptid;
>> +      inline_states.erase (state_it);
> 
> The patch looks good, though it made me realize that when we're
> replacing VEC_unordered_remove with std::vector::erase, we're 
> introducing
> a pessimization, which makes me ponder about having an 
> utility/replacement
> for VEC_unordered_remove that works with std::vector and alikes.
> I.e., a function that removes an element from a vector simply by moving
> the last element to the now-vacant position.  That's more efficient
> than erase, because it avoids having to copy/move the remaining
> elements, making it O(1).
> 
> Something like:
> 
>  template<typename Vector>
>  void
>  unordered_erase (Vector &v, typename Vector::const_iterator pos)
>  {
>      *pos = std::move (v.back ());
>      v.pop_back ();
>  }

We actually have almost that already in common/gdb_vecs.h.  And I have 
no excuse not to know about it, since I added it.

I'll review my latest patches to see if there are more opportunities to 
use that.

Thanks,

Simon