From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 108824 invoked by alias); 31 Oct 2019 00:23:46 -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 108710 invoked by uid 89); 31 Oct 2019 00:23:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN autolearn=ham version=3.3.1 spammy=finishing, minsym_hash, sk:minsym_, function_view X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 31 Oct 2019 00:23:43 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 1FF4120250; Wed, 30 Oct 2019 20:23:41 -0400 (EDT) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id C8450204A6; Wed, 30 Oct 2019 20:23:37 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 7F1B32A7DA; Wed, 30 Oct 2019 20:23:37 -0400 (EDT) X-Gerrit-PatchSet: 1 Date: Thu, 31 Oct 2019 00:23:00 -0000 From: "Christian Biesinger (Code Review)" To: gdb-patches@sourceware.org Cc: Christian Biesinger Message-ID: Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange Subject: [review] [RFC] Don't block on finishing demangling msymbols X-Gerrit-Change-Id: I9d871917459ece0b41d31670b3c56600757aea66 X-Gerrit-Change-Number: 463 X-Gerrit-ChangeURL: X-Gerrit-Commit: 8c71b191dbb8b87b8942fe7c4014757e3cb37831 References: Reply-To: cbiesinger@google.com, cbiesinger@google.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/3.0.3-75-g9005159e5d Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2019-10/txt/msg01143.txt.bz2 Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/463 ...................................................................... [RFC] Don't block on finishing demangling msymbols Instead, do it all on a background thread and only block when we actually need the result. Note, this is a speedup but not quite as much as I was expecting; still looking into what causes the waits. However, let me know if you have thoughts on the concept! Change-Id: I9d871917459ece0b41d31670b3c56600757aea66 --- M gdb/minsyms.c M gdb/objfiles.h 2 files changed, 97 insertions(+), 60 deletions(-) diff --git a/gdb/minsyms.c b/gdb/minsyms.c index be52923..253e75d 100644 --- a/gdb/minsyms.c +++ b/gdb/minsyms.c @@ -157,15 +157,15 @@ TABLE. */ static void add_minsym_to_demangled_hash_table (struct minimal_symbol *sym, - struct objfile *objfile, + struct objfile_per_bfd_storage *per_bfd, unsigned int hash_value) { if (sym->demangled_hash_next == NULL) { - objfile->per_bfd->demangled_hash_languages.set (MSYMBOL_LANGUAGE (sym)); + per_bfd->demangled_hash_languages.set (MSYMBOL_LANGUAGE (sym)); struct minimal_symbol **table - = objfile->per_bfd->msymbol_demangled_hash; + = per_bfd->msymbol_demangled_hash; unsigned int hash_index = hash_value % MINIMAL_SYMBOL_HASH_SIZE; sym->demangled_hash_next = table[hash_index]; table[hash_index] = sym; @@ -341,6 +341,7 @@ objfile_debug_name (objfile)); } + objfile->per_bfd->wait_for_msymbols (); /* Do two passes: the first over the ordinary hash table, and the second over the demangled hash table. */ lookup_minimal_symbol_mangled (name, sfile, objfile, @@ -477,6 +478,7 @@ (struct objfile *objf, const lookup_name_info &lookup_name, gdb::function_view callback) { + objf->per_bfd->wait_for_msymbols (); /* The first pass is over the ordinary hash table. */ { const char *name = linkage_name_str (lookup_name); @@ -529,6 +531,7 @@ for (objfile *objfile : objf->separate_debug_objfiles ()) { + objfile->per_bfd->wait_for_msymbols (); for (minimal_symbol *msymbol = objfile->per_bfd->msymbol_hash[hash]; msymbol != NULL; msymbol = msymbol->hash_next) @@ -562,6 +565,7 @@ if (objf == NULL || objf == objfile || objf == objfile->separate_debug_objfile_backlink) { + objfile->per_bfd->wait_for_msymbols (); for (msymbol = objfile->per_bfd->msymbol_hash[hash]; msymbol != NULL && found_symbol.minsym == NULL; msymbol = msymbol->hash_next) @@ -609,6 +613,7 @@ if (objf == NULL || objf == objfile || objf == objfile->separate_debug_objfile_backlink) { + objfile->per_bfd->wait_for_msymbols (); for (msymbol = objfile->per_bfd->msymbol_hash[hash]; msymbol != NULL; msymbol = msymbol->hash_next) @@ -720,6 +725,7 @@ /* If this objfile has a minimal symbol table, go search it using a binary search. */ + objfile->per_bfd->wait_for_msymbols (); if (objfile->per_bfd->minimal_symbol_count > 0) { int best_zero_sized = -1; @@ -1270,27 +1276,27 @@ static void build_minimal_symbol_hash_tables - (struct objfile *objfile, + (struct objfile_per_bfd_storage *per_bfd, const std::vector& hash_values) { int i; struct minimal_symbol *msym; /* (Re)insert the actual entries. */ - int mcount = objfile->per_bfd->minimal_symbol_count; + int mcount = per_bfd->minimal_symbol_count; for ((i = 0, - msym = objfile->per_bfd->msymbols.get ()); + msym = per_bfd->msymbols.get ()); i < mcount; i++, msym++) { msym->hash_next = 0; - add_minsym_to_hash_table (msym, objfile->per_bfd->msymbol_hash, + add_minsym_to_hash_table (msym, per_bfd->msymbol_hash, hash_values[i].minsym_hash); msym->demangled_hash_next = 0; if (MSYMBOL_SEARCH_NAME (msym) != MSYMBOL_LINKAGE_NAME (msym)) add_minsym_to_demangled_hash_table - (msym, objfile, hash_values[i].minsym_demangled_hash); + (msym, per_bfd, hash_values[i].minsym_demangled_hash); } } @@ -1311,8 +1317,11 @@ if (m_objfile->per_bfd->minsyms_read) return; + if (m_msym_count > 0) { + m_objfile->per_bfd->wait_for_msymbols (); + if (symtab_create_debug) { fprintf_unfiltered (gdb_stdlog, @@ -1375,63 +1384,71 @@ m_objfile->per_bfd->minimal_symbol_count = mcount; m_objfile->per_bfd->msymbols = std::move (msym_holder); -#if CXX_STD_THREAD - /* Mutex that is used when modifying or accessing the demangled - hash table. */ - std::mutex demangled_mutex; -#endif - - std::vector hash_values (mcount); - msymbols = m_objfile->per_bfd->msymbols.get (); - gdb::parallel_for_each - (&msymbols[0], &msymbols[mcount], - [&] (minimal_symbol *start, minimal_symbol *end) - { - for (minimal_symbol *msym = start; msym < end; ++msym) - { - size_t idx = msym - msymbols; - hash_values[idx].name_length = strlen (msym->name); - if (!msym->name_set) - { - /* This will be freed later, by symbol_set_names. */ - char *demangled_name - = symbol_find_demangled_name (msym, msym->name); - symbol_set_demangled_name - (msym, demangled_name, - &m_objfile->per_bfd->storage_obstack); - msym->name_set = 1; + objfile_per_bfd_storage* per_bfd = m_objfile->per_bfd; - hash_values[idx].mangled_name_hash - = fast_hash (msym->name, hash_values[idx].name_length); - } - hash_values[idx].minsym_hash - = msymbol_hash (MSYMBOL_LINKAGE_NAME (msym)); - hash_values[idx].minsym_demangled_hash - = search_name_hash (MSYMBOL_LANGUAGE (msym), - MSYMBOL_SEARCH_NAME (msym)); - } - { - /* To limit how long we hold the lock, we only acquire it here - and not while we demangle the names above. */ + m_objfile->per_bfd->m_minsym_future + = gdb::thread_pool::g_thread_pool->post_task ([msymbols, mcount, + per_bfd] () + { #if CXX_STD_THREAD - std::lock_guard guard (demangled_mutex); + /* Mutex that is used when modifying or accessing the demangled + hash table. */ + std::mutex demangled_mutex; #endif - for (minimal_symbol *msym = start; msym < end; ++msym) - { - size_t idx = msym - msymbols; - symbol_set_names - (msym, - gdb::string_view(msym->name, - hash_values[idx].name_length), - false, - m_objfile->per_bfd, - hash_values[idx].mangled_name_hash); - } - } - }); - build_minimal_symbol_hash_tables (m_objfile, hash_values); + std::vector hash_values (mcount); + + gdb::parallel_for_each + (&msymbols[0], &msymbols[mcount], + [&] (minimal_symbol *start, minimal_symbol *end) + { + for (minimal_symbol *msym = start; msym < end; ++msym) + { + size_t idx = msym - msymbols; + hash_values[idx].name_length = strlen (msym->name); + if (!msym->name_set) + { + /* This will be freed later, by symbol_set_names. */ + char *demangled_name + = symbol_find_demangled_name (msym, msym->name); + symbol_set_demangled_name + (msym, demangled_name, + &per_bfd->storage_obstack); + msym->name_set = 1; + + hash_values[idx].mangled_name_hash + = fast_hash (msym->name, hash_values[idx].name_length); + } + hash_values[idx].minsym_hash + = msymbol_hash (MSYMBOL_LINKAGE_NAME (msym)); + hash_values[idx].minsym_demangled_hash + = search_name_hash (MSYMBOL_LANGUAGE (msym), + MSYMBOL_SEARCH_NAME (msym)); + } + { + /* To limit how long we hold the lock, we only acquire it here + and not while we demangle the names above. */ +#if CXX_STD_THREAD + std::lock_guard guard (demangled_mutex); +#endif + for (minimal_symbol *msym = start; msym < end; ++msym) + { + size_t idx = msym - msymbols; + symbol_set_names + (msym, + gdb::string_view(msym->name, + hash_values[idx].name_length), + false, + per_bfd, + hash_values[idx].mangled_name_hash); + } + } + }); + + build_minimal_symbol_hash_tables (per_bfd, hash_values); + } + ); } } @@ -1518,6 +1535,8 @@ other sections, to find the next symbol in this section with a different address. */ + minsym.objfile->per_bfd->wait_for_msymbols (); + struct minimal_symbol *past_the_end = (minsym.objfile->per_bfd->msymbols.get () + minsym.objfile->per_bfd->minimal_symbol_count); diff --git a/gdb/objfiles.h b/gdb/objfiles.h index 0c04458..1c126c1 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -30,6 +30,9 @@ #include "psymtab.h" #include #include +#if CXX_STD_THREAD +#include +#endif #include "gdbsupport/next-iterator.h" #include "gdbsupport/safe-iterator.h" #include "bcache.h" @@ -319,6 +322,19 @@ /* All the different languages of symbols found in the demangled hash table. */ std::bitset demangled_hash_languages; + + void wait_for_msymbols() { +#if CXX_STD_THREAD + if (m_minsym_future.valid ()) + { + m_minsym_future.wait (); + m_minsym_future = std::future (); + } +#endif + } +#if CXX_STD_THREAD + std::future m_minsym_future; +#endif }; /* An iterator that first returns a parent objfile, and then each @@ -439,11 +455,13 @@ minimal_symbol_iterator begin () const { + m_objfile->per_bfd->wait_for_msymbols (); return minimal_symbol_iterator (m_objfile->per_bfd->msymbols.get ()); } minimal_symbol_iterator end () const { + m_objfile->per_bfd->wait_for_msymbols (); return minimal_symbol_iterator (m_objfile->per_bfd->msymbols.get () + m_objfile->per_bfd->minimal_symbol_count); -- Gerrit-Project: binutils-gdb Gerrit-Branch: master Gerrit-Change-Id: I9d871917459ece0b41d31670b3c56600757aea66 Gerrit-Change-Number: 463 Gerrit-PatchSet: 1 Gerrit-Owner: Christian Biesinger Gerrit-MessageType: newchange