From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9680 invoked by alias); 16 May 2013 13:50:36 -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 9648 invoked by uid 89); 16 May 2013 13:50:32 -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 13:50:31 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4GDoUbr008162 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 16 May 2013 09:50:30 -0400 Received: from localhost.localdomain (ovpn-112-16.ams2.redhat.com [10.36.112.16]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r4GDoSh8012888; Thu, 16 May 2013 09:50:29 -0400 Message-ID: <5194E424.9090605@redhat.com> Date: Thu, 16 May 2013 13:50: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> In-Reply-To: <5194E257.4010807@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00618.txt.bz2 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