From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15137 invoked by alias); 11 Oct 2011 15:53:29 -0000 Received: (qmail 15094 invoked by uid 22791); 11 Oct 2011 15:53:24 -0000 X-SWARE-Spam-Status: No, hits=-1.0 required=5.0 tests=AWL,BAYES_00,SARE_SUB_IMPROVE,SPF_FAIL X-Spam-Check-By: sourceware.org Received: from gbenson.demon.co.uk (HELO gbenson.demon.co.uk) (80.177.220.214) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 11 Oct 2011 15:53:04 +0000 Date: Tue, 11 Oct 2011 15:53:00 -0000 From: Gary Benson To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: [RFA take 2] Improve performance with lots of shared libraries Message-ID: <20111011155300.GB3411@redhat.com> Mail-Followup-To: Pedro Alves , gdb-patches@sourceware.org References: <20110922171206.GB5874@redhat.com> <20111004102517.GA2679@redhat.com> <201110071601.57025.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="zYM0uCDKw75PZbzx" Content-Disposition: inline In-Reply-To: <201110071601.57025.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2011-10/txt/msg00307.txt.bz2 --zYM0uCDKw75PZbzx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 2056 Hi Pedro, Pedro Alves wrote: > On Tuesday 04 October 2011 11:25:18, Gary Benson wrote: > > 2011-09-22 Gary Benson > > > > * infrun.c (stopped_at_solib_event_breakpoint): New function. > > (stopped_at_solib_event_breakpoint_helper): Likewise. > > (handle_inferior_event): Avoid calling skip_inline_frames > > when at the solib event breakpoint. > > As I mentioned before, better would for this to be a > property/function of the breakpoint itself (and its locations) -- > meaning "I am / am not an inlined location". There's no reason to > limit this just to the solib event breakpoint. An obvious example > without even that generalization is the thread event breakpoint. > > Even if you don't do that, please add this new function (that checks > whether we stopped at an address that can't be inlined) to > breakpoint.c instead, and call it `stopped_at_non_inline_function' > or something along those lines. We can later rework its internals > keeping its interface. ... > You need to check that the breakpoint is enabled and installed. ... > When working in the direction of matching from an event back to our > tables and symbolic info, you start from the address_space instead. > Get at the address space of the current PC using > `get_regcache_aspace (get_thread_regcache (ecs->ptid))'. But better > yet, since you'd end up rewritting bkpt_breakpoint_hit, it's better > instead to reuse bpstat_check_location (despite the name, it doesn't > work with a bpstat). See > bpstop_status->bpstat_check_location->bkpt_breakpoint_hit, and > factor out the necessary bits. Further to my mail yesterday with questions/explanations, attached is a patch which does what I think you are asking. Is this what you had in mind? Note that I haven't extended it to recognise anything other than the shared library event breakpoint as a non-inline location. If possible I'd like to handle other cases in a separate patch as it would require further discussion. Thanks, Gary -- http://gbenson.net/ --zYM0uCDKw75PZbzx Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=patch Content-length: 3404 2011-10-11 Gary Benson * breakpoint.h (stopped_at_non_inline_function): Declare. * breakpoint.c (is_non_inline_function, stopped_at_non_inline_function): New functions. * infrun.c (handle_inferior_event): Don't call skip_inline_frames if the stop is at a location where functions cannot be inlined. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 08ff69b..5782d3d 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -13325,6 +13325,45 @@ iterate_over_breakpoints (int (*callback) (struct breakpoint *, void *), return NULL; } +/* Zero if any of the breakpoint's locations could be a location where + functions have been inlined, nonzero otherwise. */ + +static int +is_non_inline_function (struct breakpoint *b) +{ + /* The shared library event breakpoint is set on the address of a + non-inline function. */ + if (b->type == bp_shlib_event) + return 1; + + return 0; +} + +/* Nonzero if the specified PC cannot be a location where functions + have been inlined. */ + +int +stopped_at_non_inline_function (struct address_space *aspace, CORE_ADDR pc) +{ + struct breakpoint *b; + struct bp_location *bl; + + ALL_BREAKPOINTS (b) + { + if (!is_non_inline_function (b)) + continue; + + for (bl = b->loc; bl != NULL; bl = bl->next) + { + if (!bl->shlib_disabled + && bpstat_check_location (bl, aspace, pc)) + return 1; + } + } + + return 0; +} + void initialize_breakpoint_ops (void) { diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 5e5d1b9..a4e5a4f 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1357,6 +1357,12 @@ extern void end_rbreak_breakpoints (void); extern struct breakpoint *iterate_over_breakpoints (int (*) (struct breakpoint *, void *), void *); +/* Nonzero if the specified PC cannot be a location where functions + have been inlined. */ + +extern int stopped_at_non_inline_function (struct address_space *aspace, + CORE_ADDR pc); + extern int user_breakpoint_p (struct breakpoint *); #endif /* !defined (BREAKPOINT_H) */ diff --git a/gdb/infrun.c b/gdb/infrun.c index cc2e29b..11ba700 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4044,7 +4044,23 @@ handle_inferior_event (struct execution_control_state *ecs) nexti. After stepi and nexti, always show the innermost frame (not any inline function call sites). */ if (ecs->event_thread->control.step_range_end != 1) - skip_inline_frames (ecs->ptid); + { + struct address_space *aspace = + get_regcache_aspace (get_thread_regcache (ecs->ptid)); + + /* skip_inline_frames is expensive, so we avoid it if we can + determine that the address is one where functions cannot have + been inlined. This improves performance with inferiors that + load a lot of shared libraries, because the solib event + breakpoint is defined as the address of a function (i.e. not + inline). Note that we have to check the previous PC as well + as the current one to catch cases when we have just + single-stepped over a breakpoint prior to reinstating it. */ + if (!stopped_at_non_inline_function (aspace, stop_pc) + && !stopped_at_non_inline_function (aspace, + ecs->event_thread->prev_pc)) + skip_inline_frames (ecs->ptid); + } if (ecs->event_thread->suspend.stop_signal == TARGET_SIGNAL_TRAP && ecs->event_thread->control.trap_expected --zYM0uCDKw75PZbzx--