Re: [patch] Convert frame_stash to a hash table

[Phil Muldoon <pmuldoon@redhat.com>]

On 16/05/13 14:42, Pedro Alves wrote:
> On 05/16/2013 02:09 PM, Phil Muldoon wrote:
> 
> When doing a backtrace, you'll end up linearly walking the frame
> chain, and normally you don't go back to newer frames -- unwind a
> frame (frame.prev()), print info about it, unwind the next, print it,
> on and on.  As such, a single frame stashed in the frame stash should be
> sufficient.  But it's not.  frapy_older does:

When using frame filters, in the case of eliding frames this may not
be the case.  In fact we cannot predict how frame filters will
navigate the stack.

 
>   TRY_CATCH (except, RETURN_MASK_ALL)
>     {
> 
>       /* Try to get the previous frame, to determine if this is the last frame
> 	 in a corrupt stack.  If so, we need to store the frame_id of the next
> 	 frame and not of this one (which is possibly invalid).  */
>       if (get_prev_frame (frame) == NULL
> 	  && get_frame_unwind_stop_reason (frame) != UNWIND_NO_REASON
> 	  && get_next_frame (frame) != NULL)
> 	{
> 	  frame_obj->frame_id = get_frame_id (get_next_frame (frame));
> 	  frame_obj->frame_id_is_next = 1;
> 	}


Yes, this is bogus.  But even if you remove this, the performance hits
still register as significant.

> and given the present frame stash can only hold one frame,
> these get_prev_frame/get_next_frame calls constantly invalidate it. 
> Now, I don't get this "detect corrupt stack" code at all.

Me either, it should be removed.  Hiding the corrupt stack from a
Python consumer seems all kinds of wrong.  I am going to fix this
next.  I decided not to include it in this patch, as I wanted the
focus to be on frame_stash issues where Python scripts can randomly
access frame from all over the stack.

Take this example

f = gdb.newest_frame()

do some other inferior operations happen, stop.

g = gdb.newest_frame()

Now is I access f, say f.type(), that will not be in the frame_stash,
it was from awhile ago.  These kinds of patterns do crop up in frame
filters, because we are filtering, eliding frames.


> To be clear, I'm not against the hash stash idea at all.  It's likely to
> speed up use cases, even if frame_info_to_frame_object was changed
> to not do that dance.

It will be changed, very soon.

Thanks for the comments,

Cheers

Phil