Re: [patch] Convert frame_stash to a hash table

[Phil Muldoon <pmuldoon@redhat.com>]

On 16/05/13 15:23, Pedro Alves wrote:
> On 05/16/2013 02:50 PM, Phil Muldoon wrote:
>> 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.
> 
> For sure.  However, I think in your backtrace example, the frame
> filter actually did nothing, correct?

Nope, the frame filter did some operations on each frame function
name, and also elided frames.

>> Yes, this is bogus.  But even if you remove this, the performance hits
>> still register as significant.
> 
> I'd expected that a simple filter (like I imagine yours was)
> you'd not see any performance hit.

It did, but ...

>>> 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.
> 
> OK.  Again, I'm not questioning the merit of the patch, but the
> example/rationale.  :-)  Personally, I'd rather that was fixed first,
> and then the new frame hash stash justified/explained with
> with an example where gdb's inefficiencies are exposed even when
> gdb's python code is sane.  :-)

I'll fix this and rerun the performance tests.

>> 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.
> 
> I'm confused.  :-)  If you do other inferior operations
> that resume the inferior, then the new hash stash won't help either.
> Resuming the inferior always invalidates all frames, along with the
> stash.

Yes my example was messed up, the second newest_frame should be some
other frame, and delete the whole line about inferior operations.

Cheers

Phil