From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2550 invoked by alias); 16 May 2013 14:27:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 2540 invoked by uid 89); 16 May 2013 14:27:13 -0000 X-Spam-SWARE-Status: No, score=-7.3 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 16 May 2013 14:27:13 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4GERBuK022298 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 16 May 2013 10:27:12 -0400 Received: from localhost.localdomain (ovpn-112-16.ams2.redhat.com [10.36.112.16]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r4GERABS014085; Thu, 16 May 2013 10:27:10 -0400 Message-ID: <5194ECBD.6060601@redhat.com> Date: Thu, 16 May 2013 14:27:00 -0000 From: Phil Muldoon MIME-Version: 1.0 To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: Re: [patch] Convert frame_stash to a hash table References: <5194DA88.6020705@redhat.com> <5194E257.4010807@redhat.com> <5194E424.9090605@redhat.com> <5194EBEF.9040209@redhat.com> In-Reply-To: <5194EBEF.9040209@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00622.txt.bz2 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