From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 107159 invoked by alias); 10 May 2017 04:14:43 -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 106140 invoked by uid 89); 10 May 2017 04:14:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=hits X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 10 May 2017 04:14:39 +0000 Received: by simark.ca (Postfix, from userid 33) id 6F5F91E4A4; Wed, 10 May 2017 00:14:40 -0400 (EDT) To: Tim Wiederhake Subject: Re: [PATCH v3 10/12] btrace: Replace struct btrace_thread_info::segment. X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 10 May 2017 04:14:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org, markus.t.metzger@intel.com In-Reply-To: <1494312929-22749-11-git-send-email-tim.wiederhake@intel.com> References: <1494312929-22749-1-git-send-email-tim.wiederhake@intel.com> <1494312929-22749-11-git-send-email-tim.wiederhake@intel.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00240.txt.bz2 This title too should say btrace_function. On 2017-05-09 02:55, Tim Wiederhake wrote: > This used to hold a pair of pointers to the previous and next function > segment > that belong to this function call. Replace with a pair of indices into > the > vector of function segments. > > 2017-05-09 Tim Wiederhake > > gdb/ChangeLog: > > * btrace.c (ftrace_fixup_caller, ftrace_new_return, > ftrace_connect_bfun, > ftrace_bridge_gap): Replace references to btrace_thread_info::segment > with btrace_thread_info::next_segment and > btrace_thread_info::prev_segment. > * btrace.h: Remove struct btrace_func_link. > (struct btrace_function): Replace pair of function segment pointers > with pair of indices. > * python/py-record-btrace.c (btpy_call_prev_sibling, > btpy_call_next_sibling): Replace references to > btrace_thread_info::segment with btrace_thread_info::next_segment and > btrace_thread_info::prev_segment. > * record-btrace.c (record_btrace_frame_this_id): Same. > > --- > gdb/btrace.c | 47 > ++++++++++++++++++++++++++----------------- > gdb/btrace.h | 17 ++++++---------- > gdb/python/py-record-btrace.c | 8 ++++---- > gdb/record-btrace.c | 4 ++-- > 4 files changed, 40 insertions(+), 36 deletions(-) > > diff --git a/gdb/btrace.c b/gdb/btrace.c > index f57bbf9..921cb64 100644 > --- a/gdb/btrace.c > +++ b/gdb/btrace.c > @@ -271,20 +271,29 @@ ftrace_update_caller (struct btrace_function > *bfun, > /* Fix up the caller for all segments of a function. */ > > static void > -ftrace_fixup_caller (struct btrace_function *bfun, > +ftrace_fixup_caller (struct btrace_thread_info *btinfo, > + struct btrace_function *bfun, > struct btrace_function *caller, > enum btrace_function_flag flags) > { > - struct btrace_function *prev, *next; > + unsigned int prev, next; > > + prev = bfun->prev; > + next = bfun->next; > ftrace_update_caller (bfun, caller, flags); > > /* Update all function segments belonging to the same function. */ > - for (prev = bfun->segment.prev; prev != NULL; prev = > prev->segment.prev) > - ftrace_update_caller (prev, caller, flags); > + for (; prev != 0; prev = bfun->prev) > + { > + bfun = ftrace_find_call_by_number (btinfo, prev); > + ftrace_update_caller (bfun, caller, flags); > + } > > - for (next = bfun->segment.next; next != NULL; next = > next->segment.next) > - ftrace_update_caller (next, caller, flags); > + for (; next != 0; next = bfun->next) > + { > + bfun = ftrace_find_call_by_number (btinfo, next); > + ftrace_update_caller (bfun, caller, flags); > + } Could you define the loop variables in the for like this? for (unsigned int prev = bfun->prev; prev != 0; prev = bfun->prev) { bfun = ftrace_find_call_by_number (btinfo, prev); ftrace_update_caller (bfun, caller, flags); } Unless is it important to capture the value of bfun->prev/next before calling ftrace_update_caller? This way their scope is limited to where they are used. Btw, this is another thing that could be rewritten nicely if btrace_function had a backlink to btrace_thread_info, something like: for (btrace_function *it = bfun; it != NULL; it = it->next_segment ()) ftrace_update_caller (it, caller, flags); Btw #2, I thing this function could be more efficient (or maybe I don't understand as well as I think). If bfun at function entry is in the middle of a long list of segments, it will start from there and iterate backwards until it hits the first segment. But because the same bfun variable is reused, it will iterate forward from the start until the end, whereas it only needed to iterate from the original bfun. Using a temporary loop variable to avoid modifying bfun would correct that. > @@ -948,7 +957,7 @@ ftrace_bridge_gap (struct btrace_thread_info > *btinfo, > static void > btrace_bridge_gaps (struct thread_info *tp, VEC (bfun_s) **gaps) > { > - struct btrace_thread_info *btinfo; > + struct btrace_thread_info *btinfo = &tp->btrace; btinfo is now assigned twice in this function. Thanks, Simon