From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32407 invoked by alias); 9 Sep 2011 14:51:34 -0000 Received: (qmail 32393 invoked by uid 22791); 9 Sep 2011 14:51:32 -0000 X-SWARE-Spam-Status: No, hits=-5.5 required=5.0 tests=AWL,BAYES_20,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SARE_SUB_IMPROVE,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 09 Sep 2011 14:51:14 +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 p89EpEKe011225 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 9 Sep 2011 10:51:14 -0400 Received: from host1.jankratochvil.net (ovpn-116-38.ams2.redhat.com [10.36.116.38]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p89EpBE0009173 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 9 Sep 2011 10:51:13 -0400 Received: from host1.jankratochvil.net (localhost [127.0.0.1]) by host1.jankratochvil.net (8.14.4/8.14.4) with ESMTP id p89EpBPq013170 for ; Fri, 9 Sep 2011 16:51:11 +0200 Received: (from jkratoch@localhost) by host1.jankratochvil.net (8.14.4/8.14.4/Submit) id p89EpAKg013162 for gdb-patches@sourceware.org; Fri, 9 Sep 2011 16:51:10 +0200 Date: Fri, 09 Sep 2011 14:53:00 -0000 From: Jan Kratochvil To: gdb-patches@sourceware.org Subject: Re: [RFA] Improve performance with lots of shared libraries Message-ID: <20110909145110.GB12299@host1.jankratochvil.net> References: <20110909123156.GA1503@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110909123156.GA1503@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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-09/txt/msg00160.txt.bz2 On Fri, 09 Sep 2011 14:31:56 +0200, Gary Benson wrote: > There are two things I'm not sure about with it as it stands. One is > to do with program spaces: I noticed that breakpoints have a program > space, but breakpoint locations also have a program space. Is the way > I have used these correct? Tom is working on removing the program space from the breakpoint itself. Or at least Tom was discussing its removal. > --- /dev/null > +++ b/gdb/solib-bp-disable.c [...] > +/* Nonzero if multi-threaded inferior support is present. */ > + > +static int > +multi_thread_support_availabie (void) This should be in target.c (and probably renamed). [...] > +/* Enable or disable a single solib event breakpoint as appropriate. */ > + > +static int > +update_solib_breakpoint (struct breakpoint *b, void *arg) > +{ > + int enable = *(int *) arg; > + struct bp_location *loc; > + > + if (b->pspace != current_program_space) > + return 0; > + > + if (b->type != bp_shlib_event) > + return 0; > + > + for (loc = b->loc; loc; loc = loc->next) > + if (loc->pspace == current_program_space) > + { > + if (enable && b->enable_state == bp_disabled) > + b->enable_state = bp_enabled; > + else if (!enable && b->enable_state == bp_enabled) > + b->enable_state = bp_disabled; > + } After modifying any ENABLE_STATE you have to always call update_global_location_list. For some events it is already done, it should be probably per-event (sometimes still duplicating the call but that is probably OK). > + > + return 0; > +} > + > +/* Enable or disable solib event breakpoints as appropriate. */ > + > +void > +update_solib_breakpoints () > +{ > + int enable = stop_on_solib_events > + || !multi_thread_support_availabie () > + || pending_breakpoints_exist (); This formatting is not compliant with Emacs-driven GNU Coding Style, it should be AFAIK: int enable = (stop_on_solib_events || !multi_thread_support_availabie () || pending_breakpoints_exist ()); [...] > +void > +_initialize_solib_bp_disable (void) > +{ > + /* Observe breakpoint operations so we can enable the shared > + library event breakpoint if there are breakpoints pending > + on shared library loads. */ > + observer_attach_breakpoint_created (breakpoint_event); > + observer_attach_breakpoint_modified (breakpoint_event); > + observer_attach_breakpoint_deleted (breakpoint_event); > + > + /* We also need to watch for inferior creation, because the > + creation of the shared library event breakpoint does not > + cause a breakpoint_created notification so inferior_created > + is the next best thing. */ Isn't more bug the missing notification? > + observer_attach_inferior_created (inferior_event); > + > + /* Observe shared libraries being loaded and unloaded so we > + can disable the shared library event breakpoint once a > + thread debugging library has been loaded. */ > + observer_attach_solib_loaded (solib_event); > + observer_attach_solib_unloaded (solib_event); > +} There is open the problem of ambiguous breakpoints but otherwise it looks great to me, thanks. Jan