From: Tom Tromey Date: Tue, 25 Mar 2025 13:17:38 +0000 (-0600) Subject: Remove cooked_index_worker::result_type X-Git-Tag: binutils-2_45~997 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2dc660bd2b23265574c8779a3aed5272c1829b5c;p=thirdparty%2Fbinutils-gdb.git Remove cooked_index_worker::result_type cooked_index_worker::result_type is an ad hoc tuple type used for transferring data between phases of the indexer. It's a bit unwieldy and another patch I'm working on would be somewhat nicer without it. This patch removes the type. Now cooked_index_ephemeral objects are transferred instead, which is handy because they already hold the needed state. Approved-By: Simon Marchi --- diff --git a/gdb/dwarf2/abbrev-table-cache.h b/gdb/dwarf2/abbrev-table-cache.h index 84699487657..d99fb8df8a7 100644 --- a/gdb/dwarf2/abbrev-table-cache.h +++ b/gdb/dwarf2/abbrev-table-cache.h @@ -30,6 +30,9 @@ public: abbrev_table_cache () = default; DISABLE_COPY_AND_ASSIGN (abbrev_table_cache); + abbrev_table_cache (abbrev_table_cache &&) = default; + abbrev_table_cache &operator= (abbrev_table_cache &&) = default; + /* Find an abbrev table coming from the abbrev section SECTION at offset OFFSET. Return the table, or nullptr if it has not yet been registered. */ diff --git a/gdb/dwarf2/cooked-index-worker.c b/gdb/dwarf2/cooked-index-worker.c index 8073a672cb9..bde2055f0f1 100644 --- a/gdb/dwarf2/cooked-index-worker.c +++ b/gdb/dwarf2/cooked-index-worker.c @@ -90,6 +90,22 @@ bool cooked_index_worker_result::cutu_reader_eq::operator() /* See cooked-index-worker.h. */ +void +cooked_index_worker_result::emit_complaints_and_exceptions + (gdb::unordered_set &seen_exceptions) +{ + gdb_assert (is_main_thread ()); + + re_emit_complaints (m_complaints); + + /* Only show a given exception a single time. */ + for (auto &one_exc : m_exceptions) + if (seen_exceptions.insert (one_exc).second) + exception_print (gdb_stderr, one_exc); +} + +/* See cooked-index-worker.h. */ + void cooked_index_worker::start () { @@ -167,12 +183,7 @@ cooked_index_worker::wait (cooked_state desired_state, bool allow_quit) /* Only show a given exception a single time. */ gdb::unordered_set seen_exceptions; for (auto &one_result : m_results) - { - re_emit_complaints (std::get<1> (one_result)); - for (auto &one_exc : std::get<2> (one_result)) - if (seen_exceptions.insert (one_exc).second) - exception_print (gdb_stderr, one_exc); - } + one_result.emit_complaints_and_exceptions (seen_exceptions); print_stats (); diff --git a/gdb/dwarf2/cooked-index-worker.h b/gdb/dwarf2/cooked-index-worker.h index 514487b80d6..4950a56e031 100644 --- a/gdb/dwarf2/cooked-index-worker.h +++ b/gdb/dwarf2/cooked-index-worker.h @@ -47,6 +47,10 @@ public: cooked_index_worker_result (); DISABLE_COPY_AND_ASSIGN (cooked_index_worker_result); + cooked_index_worker_result (cooked_index_worker_result &&) = default; + cooked_index_worker_result &operator= (cooked_index_worker_result &&) + = default; + /* Return the current abbrev table_cache. */ const abbrev_table_cache &get_abbrev_table_cache () const { return m_abbrev_table_cache; } @@ -70,11 +74,24 @@ public: name, parent_entry, per_cu); } + /* Overload that allows the language to be specified. */ + cooked_index_entry *add (sect_offset die_offset, enum dwarf_tag tag, + cooked_index_flag flags, enum language lang, + const char *name, + cooked_index_entry_ref parent_entry, + dwarf2_per_cu *per_cu) + { + return m_shard->add (die_offset, tag, flags, lang, + name, parent_entry, per_cu); + } + /* Install the current addrmap into the shard being constructed, then transfer ownership of the index to the caller. */ - cooked_index_shard_up release () + cooked_index_shard_up release_shard () { m_shard->install_addrmap (&m_addrmap); + /* This isn't needed any more. */ + m_addrmap.clear (); return std::move (m_shard); } @@ -90,13 +107,29 @@ public: return &m_parent_map; } - /* Return the parent_map that is currently being created. Ownership - is passed to the caller. */ - parent_map release_parent_map () + /* Add an exception to the list of exceptions caught while reading. + These are passed forward and printed by the main thread. */ + void note_error (gdb_exception &&except) { - return std::move (m_parent_map); + m_exceptions.push_back (std::move (except)); } + /* Called when the thread using this object is done with its work. + This stores any complaints for later emission, and it clears some + data that won't be needed again. */ + void done_reading (complaint_collection &&complaints) + { + /* Hang on to the complaints. */ + m_complaints = std::move (complaints); + /* Discard things that are no longer needed. */ + m_reader_hash.clear (); + } + + /* Called to emit any stored complaints or exceptions. This can + only be called on the main thread. */ + void emit_complaints_and_exceptions + (gdb::unordered_set &seen_exceptions); + private: /* The abbrev table cache used by this indexer. */ abbrev_table_cache m_abbrev_table_cache; @@ -134,6 +167,13 @@ private: /* A writeable addrmap being constructed by this scanner. */ addrmap_mutable m_addrmap; + + /* The issued complaints. Only set after done_reading is + called. */ + complaint_collection m_complaints; + + /* Exceptions that we're storing to emit later. */ + std::vector m_exceptions; }; /* The possible states of the index. See the explanatory comment @@ -205,28 +245,15 @@ protected: virtual void print_stats () { } - /* Each thread returns a tuple holding a cooked index, any collected - complaints, a vector of errors that should be printed, and a - parent map. - - The errors are retained because GDB's I/O system is not - thread-safe. run_on_main_thread could be used, but that would - mean the messages are printed after the prompt, which looks - weird. */ - using result_type = std::tuple, - parent_map>; - /* The per-objfile object. */ dwarf2_per_objfile *m_per_objfile; /* Result of each worker task. */ - std::vector m_results; - /* Any warnings emitted. This is not in 'result_type' because (for - the time being at least), it's only needed in do_reading, not in - every worker. Note that deferred_warnings uses gdb_stderr in its - constructor, and this should only be done from the main thread. - This is enforced in the cooked_index_worker constructor. */ + std::vector m_results; + /* Any warnings emitted. For the time being at least, this only + needed in do_reading, not in every worker. Note that + deferred_warnings uses gdb_stderr in its constructor, and this + should only be done from the main thread. This is enforced in + the cooked_index_worker constructor. */ deferred_warnings m_warnings; /* A map of all parent maps. Used during finalization to fix up diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c index edac713f183..99c8c015900 100644 --- a/gdb/dwarf2/read-debug-names.c +++ b/gdb/dwarf2/read-debug-names.c @@ -114,11 +114,12 @@ struct mapped_debug_names_reader gdb::unordered_map abbrev_map; - /* Even though the scanning of .debug_names and creation of the cooked index - entries is done serially, we create multiple shards so that the - finalization step can be parallelized. The shards are filled in a round - robin fashion. */ - std::vector shards; + /* Even though the scanning of .debug_names and creation of the + cooked index entries is done serially, we create multiple shards + so that the finalization step can be parallelized. The shards + are filled in a round robin fashion. It's convenient to use a + result object rather than an actual shard. */ + std::vector indices; /* Next shard to insert an entry in. */ int next_shard = 0; @@ -290,11 +291,11 @@ mapped_debug_names_reader::scan_one_entry (const char *name, if (per_cu != nullptr) { *result - = shards[next_shard]->add (die_offset, (dwarf_tag) indexval.dwarf_tag, + = indices[next_shard].add (die_offset, (dwarf_tag) indexval.dwarf_tag, flags, lang, name, nullptr, per_cu); ++next_shard; - if (next_shard == shards.size ()) + if (next_shard == indices.size ()) next_shard = 0; entry_pool_offsets_to_entries.emplace (offset_in_entry_pool, *result); @@ -414,20 +415,33 @@ void cooked_index_worker_debug_names::do_reading () { complaint_interceptor complaint_handler; - std::vector exceptions; + try { m_map.scan_all_names (); } - catch (const gdb_exception &exc) + catch (gdb_exception &exc) { - exceptions.push_back (std::move (exc)); + /* Arbitrarily put all exceptions into the first result. */ + m_map.indices[0].note_error (std::move (exc)); } - m_results.emplace_back (nullptr, - complaint_handler.release (), - std::move (exceptions), - parent_map ()); + std::vector shards; + bool first = true; + for (auto &iter : m_map.indices) + { + if (first) + { + iter.done_reading (complaint_handler.release ()); + first = false; + } + else + iter.done_reading ({}); + shards.push_back (iter.release_shard ()); + m_all_parents_map.add_map (*iter.get_parent_map ()); + } + + m_results = std::move (m_map.indices); dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd; cooked_index *table @@ -436,7 +450,7 @@ cooked_index_worker_debug_names::do_reading () /* Note that this code never uses IS_PARENT_DEFERRED, so it is safe to pass nullptr here. */ - table->set_contents (std::move (m_map.shards), &m_warnings, nullptr); + table->set_contents (std::move (shards), &m_warnings, nullptr); bfd_thread_cleanup (); } @@ -838,24 +852,26 @@ do_dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile) } per_bfd->debug_aranges.read (per_objfile->objfile); - addrmap_mutable addrmap; + + /* There is a single address map for the whole index (coming from + .debug_aranges). We only need to install it into a single shard + for it to get searched by cooked_index. So, we make the first + result object here, so we can store the addrmap, then move it + into place later. */ + cooked_index_worker_result first; deferred_warnings warnings; read_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges, - &addrmap, &warnings); + first.get_addrmap (), &warnings); warnings.emit (); const auto n_workers = std::max (gdb::thread_pool::g_thread_pool->thread_count (), 1); - /* Create as many index shard as there are worker threads. */ - for (int i = 0; i < n_workers; ++i) - map.shards.emplace_back (std::make_unique ()); - - /* There is a single address map for the whole index (coming from - .debug_aranges). We only need to install it into a single shard for it to - get searched by cooked_index. */ - map.shards[0]->install_addrmap (&addrmap); + /* Create as many index shard as there are worker threads, + preserving the first one. */ + map.indices.push_back (std::move (first)); + map.indices.resize (n_workers); auto cidn = (std::make_unique (per_objfile, std::move (map))); diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index fc079cd1520..179289ae699 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -3571,8 +3571,9 @@ private: }; void -cooked_index_worker_debug_info::process_cus (size_t task_number, unit_iterator first, - unit_iterator end) +cooked_index_worker_debug_info::process_cus (size_t task_number, + unit_iterator first, + unit_iterator end) { SCOPE_EXIT { bfd_thread_cleanup (); }; @@ -3591,14 +3592,12 @@ cooked_index_worker_debug_info::process_cus (size_t task_number, unit_iterator f } catch (gdb_exception &except) { - errors.push_back (std::move (except)); + thread_storage.note_error (std::move (except)); } } - m_results[task_number] = result_type (thread_storage.release (), - complaint_handler.release (), - std::move (errors), - thread_storage.release_parent_map ()); + thread_storage.done_reading (complaint_handler.release ()); + m_results[task_number] = std::move (thread_storage); } void @@ -3610,17 +3609,17 @@ cooked_index_worker_debug_info::done_reading () for (auto &one_result : m_results) { - shards.push_back (std::move (std::get<0> (one_result))); - m_all_parents_map.add_map (std::get<3> (one_result)); + shards.push_back (one_result.release_shard ()); + m_all_parents_map.add_map (*one_result.get_parent_map ()); } /* This has to wait until we read the CUs, we need the list of DWOs. */ process_skeletonless_type_units (m_per_objfile, &m_index_storage); - shards.push_back (m_index_storage.release ()); + shards.push_back (m_index_storage.release_shard ()); shards.shrink_to_fit (); - m_all_parents_map.add_map (m_index_storage.release_parent_map ()); + m_all_parents_map.add_map (*m_index_storage.get_parent_map ()); dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd; cooked_index *table