From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 367 invoked by alias); 16 May 2013 14:23:47 -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 358 invoked by uid 89); 16 May 2013 14:23:46 -0000 X-Spam-SWARE-Status: No, score=-8.0 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:23:46 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4GENjuY026735 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 16 May 2013 10:23:45 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r4GENhRk016261; Thu, 16 May 2013 10:23:44 -0400 Message-ID: <5194EBEF.9040209@redhat.com> Date: Thu, 16 May 2013 14:23:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Phil Muldoon 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> In-Reply-To: <5194E424.9090605@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00621.txt.bz2 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? > > >> 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. I'd expected that a simple filter (like I imagine yours was) you'd not see any performance hit. > >> 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. :-) > 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. 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. -- Pedro Alves