From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 56246 invoked by alias); 18 Feb 2020 12:26:23 -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 56238 invoked by uid 89); 18 Feb 2020 12:26:23 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=browser X-HELO: mail-qk1-f194.google.com Received: from mail-qk1-f194.google.com (HELO mail-qk1-f194.google.com) (209.85.222.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Feb 2020 12:26:20 +0000 Received: by mail-qk1-f194.google.com with SMTP id j8so2928641qka.11 for ; Tue, 18 Feb 2020 04:26:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=iL4PZ2E5pUJ92OwIEvKgsoytIgJih9cRm0Y1Til8OJ8=; b=MYKlj+Wl2ghRhdG3sShOkMrM07mZObCgPaesbST6uAMZJ1WLMs7cDpWIKP7iAuGpCA 7T8v0uDITKzqicaZ5kaOQPBCK1BZ1v1ojInVrFeQhNTBf6I0n71dnjSpaGX76TSE0aYV bkPGSqV63QJ+d2zRh66cLuDoauFlHasrEdh8NyBL9gs0LZmjIjrGcl/R9b0cqG7peDr9 dHEYSB1geBo57BAINzSqMQszSOOnphQzZkOBjG2wI7+jthMOvofuBuQ3Um1rj+sgdjis kZEFYx07x35CdR8z0zvsillxmAykkR7waOTM04AcYNVMCL8SX/ind/4uWsGXXBdySCwb 4kOQ== Return-Path: Received: from [192.168.0.185] ([179.177.236.155]) by smtp.gmail.com with ESMTPSA id h9sm1693826qtq.61.2020.02.18.04.26.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 18 Feb 2020 04:26:18 -0800 (PST) Subject: Re: [PATCH 14/14] Share DWARF partial symtabs To: Tom Tromey , gdb-patches@sourceware.org References: <20200215165444.32653-1-tom@tromey.com> <20200215165444.32653-15-tom@tromey.com> From: Luis Machado Message-ID: <19439d2f-9524-0dea-4260-d1e03d762e65@linaro.org> Date: Tue, 18 Feb 2020 12:26:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200215165444.32653-15-tom@tromey.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00714.txt.bz2 On 2/15/20 1:54 PM, Tom Tromey wrote: > This changes the DWARF reader to share partial symtabs (or indices if > they are available) across objfiles. This has a few parts. > > * The dwarf2_unshareable object is created per-objfile and is stored > on the objfile using a registry key. dwarf2_enter_objfile is > modified to save and restore this pointer in the dwarf2_per_objfile. > > * objfile::partial_symtabs is changed to be a shared_ptr again. This > lets us stash a second reference in dwarf2_per_objfile; if the DWARF > data is being shared, we can simply copy this value to the new > objfile. > > * The dwarf2_per_objfile itself may be shared via the BFD -- using a > new per-BFD registry key -- or not. This depends both on whether > some other symbol reader has found symbols, and whether any BFD > sections require relocation. > > 2020-02-15 Tom Tromey > > * objfiles.h (struct objfile) : Now a > shared_ptr. > * dwarf2/read.h (struct dwarf2_per_objfile) : New > member. > (class dwarf2_enter_objfile): Constructor no longer inline. > : New member. > * dwarf2/read.c (dwarf2_unshared_data_key, dwarf2_bfd_data_key): > New globals. > (dwarf2_objfile_data_key): Add comment. > (get_dwarf2_per_objfile): Rewrite. > (dwarf2_enter_objfile::dwarf2_enter_objfile): Move from read.h. > Initialize m_restore_unshared. > (dwarf2_per_objfile::dwarf2_per_objfile): Add "unshared" > parameter. Don't initialize objfile member. > (dwarf2_per_objfile::~dwarf2_per_objfile): Don't deleted > "unshareable". "Don't delete..." > (dwarf2_has_info): Check dwarf2_bfd_data_key and > dwarf2_unshared_data_key. > (dwarf2_get_section_info): Use get_dwarf2_per_objfile. > (dwarf2_build_psymtabs): Set objfile::partial_symtabs and > short-circuit when sharing. > (dwarf2_build_psymtabs): Set dwarf2_per_objfile::partial_symtabs. > (dwarf2_psymtab::expand_psymtab): Use free_cached_comp_units. > --- > gdb/ChangeLog | 26 ++++++++++++++++++ > gdb/dwarf2/read.c | 70 +++++++++++++++++++++++++++++++++++++---------- > gdb/dwarf2/read.h | 12 ++++---- > gdb/objfiles.h | 2 +- > 4 files changed, 89 insertions(+), 21 deletions(-) > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index 15813b72005..7be57661296 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -100,7 +100,15 @@ static bool check_physname = false; > /* When true, do not reject deprecated .gdb_index sections. */ > static bool use_deprecated_index_sections = false; > > -static const struct objfile_key dwarf2_objfile_data_key; > +/* This is used to store the data that is always per objfile. */ > +static const objfile_key dwarf2_unshared_data_key; > + > +/* These are used to store the dwarf2_per_objfile; either on the > + objfile or on the BFD. */ > +static const struct objfile_key > + dwarf2_objfile_data_key; > +static const struct bfd_key > + dwarf2_bfd_data_key; > > /* The "aclass" indices for various kinds of computed DWARF symbols. */ > > @@ -274,7 +282,18 @@ struct mapped_debug_names final : public mapped_index_base > dwarf2_per_objfile * > get_dwarf2_per_objfile (struct objfile *objfile) > { > - return dwarf2_objfile_data_key.get (objfile); > + dwarf2_per_objfile *result = dwarf2_bfd_data_key.get (objfile->obfd); > + if (result == nullptr) > + result = dwarf2_objfile_data_key.get (objfile); > + return result; > +} > + > +dwarf2_enter_objfile::dwarf2_enter_objfile (struct objfile *objfile) > + : m_per_objfile (get_dwarf2_per_objfile (objfile)), > + m_restore_objfile (&m_per_objfile->objfile, objfile), > + m_restore_unshared (&m_per_objfile->unshareable, > + dwarf2_unshared_data_key.get (objfile)) > +{ > } > > /* Default names of the debugging sections. */ > @@ -1754,16 +1773,12 @@ line_header_eq_voidp (const void *item_lhs, const void *item_rhs) > dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_, > const dwarf2_debug_sections *names, > bool can_copy_) > - : objfile (objfile_), > - can_copy (can_copy_), > - /* Temporarily just allocate one per objfile -- we don't have > - sharing yet. */ > - unshareable (new dwarf2_unshareable) > + : can_copy (can_copy_) > { > if (names == NULL) > names = &dwarf2_elf_names; > > - bfd *obfd = objfile->obfd; > + bfd *obfd = objfile_->obfd; > > for (asection *sec = obfd->sections; sec != NULL; sec = sec->next) > locate_sections (obfd, sec, *names); > @@ -1780,8 +1795,6 @@ dwarf2_per_objfile::~dwarf2_per_objfile () > for (signatured_type *sig_type : all_type_units) > sig_type->per_cu.imported_symtabs_free (); > > - delete unshareable; > - > /* Everything else should be on the objfile obstack. */ > } > > @@ -1841,13 +1854,26 @@ dwarf2_has_info (struct objfile *objfile, > if (objfile->flags & OBJF_READNEVER) > return 0; > > + struct dwarf2_unshareable *unshared = dwarf2_unshared_data_key.get (objfile); > + if (unshared == nullptr) > + unshared = dwarf2_unshared_data_key.emplace (objfile); > + > struct dwarf2_per_objfile *dwarf2_per_objfile > = get_dwarf2_per_objfile (objfile); > > if (dwarf2_per_objfile == NULL) > - dwarf2_per_objfile = dwarf2_objfile_data_key.emplace (objfile, objfile, > - names, > - can_copy); > + { > + dwarf2_per_objfile = new ::dwarf2_per_objfile (objfile, names, > + can_copy); > + /* We can share this if the objfile doesn't require relocations > + and if there aren't partial symbols from some other > + reader. */ > + if (!objfile_has_partial_symbols (objfile) > + && !gdb_bfd_requires_relocations (objfile->obfd)) > + dwarf2_bfd_data_key.set (objfile->obfd, dwarf2_per_objfile); > + else > + dwarf2_objfile_data_key.set (objfile, dwarf2_per_objfile); > + } > > return (!dwarf2_per_objfile->info.is_virtual > && dwarf2_per_objfile->info.s.section != NULL > @@ -2006,7 +2032,7 @@ dwarf2_get_section_info (struct objfile *objfile, > asection **sectp, const gdb_byte **bufp, > bfd_size_type *sizep) > { > - struct dwarf2_per_objfile *data = dwarf2_objfile_data_key.get (objfile); > + struct dwarf2_per_objfile *data = get_dwarf2_per_objfile (objfile); > struct dwarf2_section_info *info; > > /* We may see an objfile without any DWARF, in which case we just > @@ -5862,6 +5888,15 @@ dwarf2_build_psymtabs (struct objfile *objfile) > struct dwarf2_per_objfile *dwarf2_per_objfile > = get_dwarf2_per_objfile (objfile); > > + if (dwarf2_per_objfile->partial_symtabs != nullptr) > + { > + /* Partial symbols were already read, so now we can simply > + attach them. */ > + objfile->partial_symtabs = dwarf2_per_objfile->partial_symtabs; > + dwarf2_resize_unshareable (dwarf2_per_objfile); > + return; > + } > + > init_psymbol_list (objfile, 1024); > > try > @@ -5882,6 +5917,11 @@ dwarf2_build_psymtabs (struct objfile *objfile) > { > exception_print (gdb_stderr, except); > } > + > + /* Finish by setting the local reference to partial symtabs, so that > + we don't try to read again. If we can't in fact share, this > + won't make a difference anyway. */ > + dwarf2_per_objfile->partial_symtabs = objfile->partial_symtabs; > } > > /* Find the base address of the compilation unit for range lists and > @@ -8915,8 +8955,8 @@ dwarf2_psymtab::expand_psymtab (struct objfile *objfile) > if (symtab.has_value ()) > return; > > + free_cached_comp_units freer (dwarf2_per_objfile); > read_dependencies (objfile); > - > dw2_do_instantiate_symtab (per_cu_data, false); > } > Spurious line removal, but it doesn't matter much. > diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h > index 241f3c8e71e..891a844a673 100644 > --- a/gdb/dwarf2/read.h > +++ b/gdb/dwarf2/read.h > @@ -299,6 +299,11 @@ public: > /* CUs that are queued to be read. */ > std::queue queue; > > + /* We keep a separate reference to the partial symtabs, in case we > + are sharing them between objfiles. This is only set after > + partial symbols have been read the first time. */ > + std::shared_ptr partial_symtabs; > + > /* The total number of per_cu and signatured_type objects that have > been created for this reader. */ > size_t num_psymtabs = 0; > @@ -321,11 +326,7 @@ class dwarf2_enter_objfile > { > public: > > - dwarf2_enter_objfile (struct objfile *objfile) > - : m_per_objfile (get_dwarf2_per_objfile (objfile)), > - m_restore_objfile (&m_per_objfile->objfile, objfile) > - { > - } > + dwarf2_enter_objfile (struct objfile *objfile); > > ~dwarf2_enter_objfile () = default; > > @@ -335,6 +336,7 @@ private: > > dwarf2_per_objfile *m_per_objfile; > scoped_restore_tmpl m_restore_objfile; > + scoped_restore_tmpl m_restore_unshared; > }; > > /* A partial symtab specialized for DWARF. */ > diff --git a/gdb/objfiles.h b/gdb/objfiles.h > index b71a8a9edb8..12892f80f71 100644 > --- a/gdb/objfiles.h > +++ b/gdb/objfiles.h > @@ -558,7 +558,7 @@ public: > > /* The partial symbol tables. */ > > - std::unique_ptr partial_symtabs; > + std::shared_ptr partial_symtabs; > > /* The object file's BFD. Can be null if the objfile contains only > minimal symbols, e.g. the run time common symbols for SunOS4. */ > I've gone through the series and didn't see anything obviously wrong. I suppose this will get more thoroughly exercised when running in multi-process/multi-target mode with quite a few objfiles and libraries. Is it currently possible to exercise such a scenario automatically? I suppose manually loading something like a web browser will be a good test as well.