Re: [PATCH] Use std::vector for displaced_step_inferior_states

[Pedro Alves <palves@redhat.com>]

On 11/22/2018 03:12 AM, Simon Marchi wrote:
> Commit
> 
>   39a36629f68e ("Use std::forward_list for displaced_step_inferior_states")
> 
> changed a hand-made linked list to use std::forward_list of pointers.
> As suggested by David Blaikie, we might as well use values instead of
> pointers.  And instead of a list, we might as well use a vector.  The
> size of this list will always be at most the number of inferiors,
> typically very small.  And in any case the operation we do in the
> hottest path (doing a displaced step) is iterate, and iterating on a
> vector is always faster than a linked list.
> 
> A consequence of using a vector is that objects can be moved, when the
> vector is resized.  I don't think this is a problem, because we don't
> save the address of the objects.  In displaced_step_prepare_throw, we
> save a pointer to the step_saved_copy field in a cleanup, but it is ran
> or discarded immediately after.

Another alternative would be to put the displaced_step_inferior_state
object in struct inferior directly instead of keeping the objects
on the side.  In practice, on x86 GNU/Linux at least, you end
up with an object per inferior anyway, assuming we actually
run the inferiors, which sounds like a good assumption.  It didn't
use to be the case originally, since back then displaced stepping
was a new thing that wasn't on by default.

> @@ -1484,36 +1484,40 @@ displaced_step_closure::~displaced_step_closure () = default;
>  /* Per-inferior displaced stepping state.  */
>  struct displaced_step_inferior_state
>  {
> +  displaced_step_inferior_state (inferior *inf)
> +    : inf (inf)
> +  {}

explicit.

> +
> +  if (it != displaced_step_inferior_states.end ())
> +    displaced_step_inferior_states.erase (it);

I think this could be unordered_remove.

Thanks,
Pedro Alves