From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32152 invoked by alias); 22 Jul 2009 18:35:16 -0000 Received: (qmail 31995 invoked by uid 22791); 22 Jul 2009 18:34:53 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 22 Jul 2009 18:34:46 +0000 Received: (qmail 3752 invoked from network); 22 Jul 2009 18:34:43 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 22 Jul 2009 18:34:43 -0000 From: Pedro Alves To: Paul Pluzhnikov Subject: Re: [patch] Speed up find_pc_section Date: Wed, 22 Jul 2009 19:19:00 -0000 User-Agent: KMail/1.9.10 Cc: tromey@redhat.com, gdb-patches@sourceware.org References: <8ac60eac0907161724v40e5bd8bg7877d8901b8d7b6e@mail.gmail.com> <8ac60eac0907221016j410f890ald54ac66c7c3291f9@mail.gmail.com> <8ac60eac0907221110v6ac9bc32u4cb17765e2cb170e@mail.gmail.com> In-Reply-To: <8ac60eac0907221110v6ac9bc32u4cb17765e2cb170e@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200907221934.34211.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: 2009-07/txt/msg00564.txt.bz2 On Wednesday 22 July 2009 19:10:48, Paul Pluzhnikov wrote: > On Wed, Jul 22, 2009 at 10:16 AM, Paul Pluzhnikov= wrote: >=20 > > On Wed, Jul 22, 2009 at 9:32 AM, Pedro Alves wr= ote: > > > >> In the OBJF_USERLOADED case, you rebuild the map when you don't > >> really need to. > > > > I think it's pretty clear that what I really need is a new 'remove_objf= ile' > > kind of observer. >=20 > I am not thinking clearly today :-( :-) > I don't need a new observer: free_objfile is in the same source, so I just > need to set the flag there as well. Bingo. > I feel going in circles here. The exec_changed observer was to address > reread_symbols, which doesn't create a new objfile. I believe that's > still necessary. Ah! I see. That, is pretty ugly/nasty. I think that objfiles.c would be a better home for reread_symbols, which would also remove that requirement. A new function objfiles.c:objfiles_changed that is called from reread_symbols, would still be better, that abusing that observer, IMHO. Do you feel like adding it? If not, I won't insist; the observer is fine for now, *if* you do something for me please: Could you please add a comment in _initialize_objfiles explaining that why it is needed, and spell out "reread_symbols" and "objfile_updated_p" in the comment, so that grepping finds it. > The solib_load/unload observers don't appear to be needed though: the load > case will create a new objfile, the unload case (when !OBJF_USERLOADED) > will do free_objfile). Exactly. >=20 > On Wed, Jul 22, 2009 at 10:40 AM, Pedro Alves wro= te: >=20 > >> I think it's pretty clear that what I really need is a new 'remove_obj= file' > >> kind of observer. > > > > Would setting objfiles_updated_p from e.g., unlink_objfile work? >=20 > Exactly. Though I think free_objfile is a more logical place for it. I was thinking that when you look for a section, you loop over linked in objfiles, while free_objfile could in principle also be called even if the objfile hadn't been linked in. But that's a minor detail; free_objfile is fine. > > I think that the need for the solib load/unload observer > > would go away too if the map is flushed on objfile > > removal/freeing/unlinking as well? >=20 > Yes. Great. >=20 > > Come to look at it deeper, what is happening with > > symbol_file_add_with_addrs_or_offsets, if the objfile has only > > minimal symbols (but still has sections)? =A0allocate_objfile is called, > > which builds the section table, but, there's a path that does an > > early return before calling the new_objfile observer, if there are > > no symbols. >=20 > That sounds like a bug: we created a new_objfile, but didn't notify > observers. Indeed. >=20 > Eeasy enough to work around though: I'll set the flag in allocate_objfile > as well. >=20 > > It would be cleaner and easier to review, easier for you (I think), > > and better for the archives in the future, IMHO. =A0But I don't mind > > much if a new patch is cooked on top. >=20 > Let's try for one more fix before reverting ... >=20 > Re-tested on Linux/x86_64 with no new failures. Looks good to me now (minus missing comment). Thank you! --=20 Pedro Alves